Re: [PATCH v5] introduce vfio-user protocol specification

[John G Johnson]

> On Nov 2, 2020, at 3:51 AM, Thanos Makatos <thanos.makatos@nutanix.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Qemu-devel <qemu-devel-
>> bounces+thanos.makatos=nutanix.com@nongnu.org> On Behalf Of John
>> Levon
>> Sent: 02 November 2020 11:41
>> To: Thanos Makatos <thanos.makatos@nutanix.com>
>> Cc: benjamin.walker@intel.com; Elena Ufimtseva
>> <elena.ufimtseva@oracle.com>; jag.raman@oracle.com;
>> james.r.harris@intel.com; Swapnil Ingle <swapnil.ingle@nutanix.com>;
>> john.g.johnson@oracle.com; yuvalkashtan@gmail.com;
>> konrad.wilk@oracle.com; tina.zhang@intel.com; qemu-devel@nongnu.org;
>> dgilbert@redhat.com; Marc-André Lureau
>> <marcandre.lureau@redhat.com>; ismael@linux.com;
>> alex.williamson@redhat.com; Stefan Hajnoczi <stefanha@redhat.com>;
>> Felipe Franciosi <felipe@nutanix.com>; xiuchun.lu@intel.com;
>> tomassetti.andrea@gmail.com; changpeng.liu@intel.com; Raphael Norwitz
>> <raphael.norwitz@nutanix.com>; Kanth.Ghatraju@oracle.com
>> Subject: Re: [PATCH v5] introduce vfio-user protocol specification
>> 
>> On Mon, Nov 02, 2020 at 11:29:23AM +0000, Thanos Makatos wrote:
>> 
>>>> 
>> +==============+========+=================================
>>>> ==================+
>>>>> | version      | object | ``{"major": <number>, "minor": <number>}``
>> |
>>>>> |              |        |                                                 
>>>>>   |
>>>>> |              |        | Version supported by the sender, e.g. "0.1".    
>>>>>   |
>>>> 
>>>> It seems quite unlikely but this should specify it's strings not floating 
>>>> point
>>>> values maybe?
>>>> 
>>>> Definitely applies to max_fds too.
>>> 
>>> major and minor are JSON numbers and specifically integers.
>> 
>> It is debatable as to whether there is such a thing as a JSON integer :)
> 
> AFAIK there isn't.
> 
>> 
>>> The rationale behind this is to simplify parsing. Is specifying that
>>> major/minor/max_fds should be an interger sufficient to clear any
>> vagueness
>>> here?
>> 
>> I suppose that's OK as long as we never want a 0.1.1 or whatever. I'm not
>> sure
>> it simplifies parsing, but maybe it does.
> 
> Now that you mention it, why preclude 0.1.1? IIUC the whole point of not
> stating the version as a float is to simply have this flexibility in the 
> future.
> You're right in your earlier suggestion to explicitly state major/minor as
> strings.
> 

        The idea behind the version IDs is to identify incompatible protocol
changes as major versions, and compatible changes as minor versions.  What
would be the purpose of the third version type?

        The thing that makes parsing the JSON easier is knowing the version
beforehand so the parser knows what keys to expect, so I’d like to promote
major and minor to separate fields in the packet from being embedded at an
arbitrary points in the JSON string.


>> 
>>>>> Versioning and Feature Support
>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>> Upon accepting a connection, the server must send a
>> VFIO_USER_VERSION
>>>> message
>>>>> proposing a protocol version and a set of capabilities. The client
>> compares
>>>>> these with the versions and capabilities it supports and sends a
>>>>> VFIO_USER_VERSION reply according to the following rules.
>>>> 
>>>> I'm curious if there was a specific reason it's this way around, when it
>> seems
>>>> more natural for the client to propose first, and the server to reply?
>>> 
>>> I'm not aware of any specific reason.
>> 
>> So can we switch it now so the initial setup is a send/recv too?
> 
> I'm fine with that but would first like to hear back from John in case he 
> objects.


        I think I write that section, and just switched client and server.  The 
code
is written as client proposes, server responds; this is the better model.

                                                                        JJ