Re: [PATCH RFC server v2 02/11] vfio-user: define vfio-user object

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3863 bytes --]

On Fri, Aug 27, 2021 at 01:53:21PM -0400, Jagannathan Raman wrote:
> Define vfio-user object which is remote process server for QEMU. Setup
> object initialization functions and properties necessary to instantiate
> the object
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  qapi/qom.json             |  20 ++++++-
>  hw/remote/vfio-user-obj.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++
>  MAINTAINERS               |   1 +
>  hw/remote/meson.build     |   1 +
>  hw/remote/trace-events    |   3 +
>  5 files changed, 168 insertions(+), 2 deletions(-)
>  create mode 100644 hw/remote/vfio-user-obj.c
> 
> diff --git a/qapi/qom.json b/qapi/qom.json
> index a25616b..3e941ee 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -689,6 +689,20 @@
>    'data': { 'fd': 'str', 'devid': 'str' } }
>  
>  ##
> +# @VfioUserProperties:
> +#
> +# Properties for vfio-user objects.
> +#
> +# @socket: path to be used as socket by the libvfiouser library
> +#
> +# @devid: the id of the device to be associated with the file descriptor
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'VfioUserProperties',
> +  'data': { 'socket': 'str', 'devid': 'str' } }

Please use 'SocketAddress' for socket instead of 'str'. That way file
descriptor passing is easy to support and additional socket address
families can be supported in the future.

> +
> +##
>  # @RngProperties:
>  #
>  # Properties for objects of classes derived from rng.
> @@ -812,7 +826,8 @@
>      'tls-creds-psk',
>      'tls-creds-x509',
>      'tls-cipher-suites',
> -    'x-remote-object'
> +    'x-remote-object',
> +    'vfio-user'
>    ] }
>  
>  ##
> @@ -868,7 +883,8 @@
>        'tls-creds-psk':              'TlsCredsPskProperties',
>        'tls-creds-x509':             'TlsCredsX509Properties',
>        'tls-cipher-suites':          'TlsCredsProperties',
> -      'x-remote-object':            'RemoteObjectProperties'
> +      'x-remote-object':            'RemoteObjectProperties',
> +      'vfio-user':                  'VfioUserProperties'

"vfio-user" doesn't communicate whether this is a client or server. Is
"vfio-user-server" clearer?

>    } }
>  
>  ##
> diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
> new file mode 100644
> index 0000000..4a1e297
> --- /dev/null
> +++ b/hw/remote/vfio-user-obj.c
> @@ -0,0 +1,145 @@
> +/**
> + * QEMU vfio-user server object
> + *
> + * Copyright © 2021 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL-v2, version 2 or later.
> + *
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +/**
> + * Usage: add options:
> + *     -machine x-remote
> + *     -device <PCI-device>,id=<pci-dev-id>
> + *     -object vfio-user,id=<id>,socket=<socket-path>,devid=<pci-dev-id>

I suggest renaming devid= to device= or pci-device= (similar to drive=
and netdev=) for consistency and to avoid confusion with PCI Device IDs.

> diff --git a/hw/remote/meson.build b/hw/remote/meson.build
> index fb35fb8..cd44dfc 100644
> --- a/hw/remote/meson.build
> +++ b/hw/remote/meson.build
> @@ -6,6 +6,7 @@ remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('message.c'))
>  remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('remote-obj.c'))
>  remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('proxy.c'))
>  remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('iohub.c'))
> +remote_ss.add(when: 'CONFIG_MULTIPROCESS', if_true: files('vfio-user-obj.c'))

If you use CONFIG_VFIO_USER_SERVER then it's easier to separate mpqemu
from vfio-user. Sharing CONFIG_MULTIPROCESS could become messy later.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]