[Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends

Marc-André Lureau posted 1 patch 5 years, 7 months ago
Failed in applying to current master (apply log)
docs/interop/vhost-user.txt | 109 +++++++++++++++++++++++++++++++++++-
1 file changed, 107 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Marc-André Lureau 5 years, 7 months ago
As discussed during "[PATCH v4 00/29] vhost-user for input & GPU"
review, let's define a common set of backend conventions to help with
management layer implementation, and interoperability.

v2:
 - drop --pidfile
 - add some notes about daemonizing & stdin/out/err

Cc: libvir-list@redhat.com
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Changpeng Liu <changpeng.liu@intel.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Felipe Franciosi <felipe@nutanix.com>
Cc: Gonglei <arei.gonglei@huawei.com>
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Victor Kaplansky <victork@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 docs/interop/vhost-user.txt | 109 +++++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 2 deletions(-)

diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index ba5e37d714..339b335e9c 100644
--- a/docs/interop/vhost-user.txt
+++ b/docs/interop/vhost-user.txt
@@ -17,8 +17,13 @@ The protocol defines 2 sides of the communication, master and slave. Master is
 the application that shares its virtqueues, in our case QEMU. Slave is the
 consumer of the virtqueues.
 
-In the current implementation QEMU is the Master, and the Slave is intended to
-be a software Ethernet switch running in user space, such as Snabbswitch.
+In the current implementation QEMU is the Master, and the Slave is the
+external process consuming the virtio queues, for example a software
+Ethernet switch running in user space, such as Snabbswitch, or a block
+device backend processing read & write to a virtual disk. In order to
+facilitate interoperability between various backend implementations,
+it is recommended to follow the "Backend program conventions"
+described in this document.
 
 Master and slave can be either a client (i.e. connecting) or server (listening)
 in the socket communication.
@@ -859,3 +864,103 @@ resilient for selective requests.
 For the message types that already solicit a reply from the client, the
 presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being set brings
 no behavioural change. (See the 'Communication' section for details.)
+
+Backend program conventions
+---------------------------
+
+vhost-user backends provide various services and they may need to be
+configured manually depending on the use case. However, it is a good
+idea to follow the conventions listed here when possible. Users, QEMU
+or libvirt, can then rely on some common behaviour to avoid
+heterogenous configuration and management of the backend program and
+facilitate interoperability.
+
+In order to be discoverable, default vhost-user backends should be
+located under "/usr/libexec", and be named "vhost-user-$device" where
+"$device" is the device name in lower-case following the name listed
+in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
+backend would be named "vhost-user-rproc-serial").
+
+Mechanisms to list, and to select among alternatives implementations
+or modify the default backend are not described at this point (a
+distribution may use update-alternatives, for example, to list and to
+pick a different default backend).
+
+The backend program must not daemonize itself, but it may be
+daemonized by the management layer. It may also have a restricted
+access to the system.
+
+File descriptors 0, 1 and 2 will exist, and have regular
+stdin/stdout/stderr usage (they may be redirected to /dev/null by the
+management layer, or to a log handler).
+
+The backend program must end (as quickly and cleanly as possible) when
+the SIGTERM signal is received. Eventually, it may be SIGKILL by the
+management layer after a few seconds.
+
+The following command line options have an expected behaviour. They
+are mandatory, unless explicitly said differently:
+
+* --socket-path=PATH
+
+This option specify the location of the vhost-user Unix domain socket.
+It is incompatible with --fd.
+
+* --fd=FDNUM
+
+When this argument is given, the backend program is started with the
+vhost-user socket as file descriptor FDNUM. It is incompatible with
+--socket-path.
+
+* --print-capabilities
+
+Output to stdout a line-seperated list of backend capabilities, and
+then exit successfully. Other options and arguments should be ignored,
+and the backend program should not perform its normal function.
+
+At the time of writing, there are no common capabilities. Some
+device-specific capabilities are listed in the respective sections. By
+convention, device-specific capabilities are prefixed by their device
+name.
+
+vhost-user-input program conventions
+------------------------------------
+
+Capabilities:
+
+input-evdev-path
+
+    The --evdev-path command line option is supported.
+
+input-no-grab
+
+    The --no-grab command line option is supported.
+
+* --evdev-path=PATH (optional)
+
+Specify the linux input device.
+
+* --no-grab (optional)
+
+Do no request exclusive access to the input device.
+
+vhost-user-gpu program conventions
+----------------------------------
+
+Capabilities:
+
+gpu-render-node
+
+    The --render-node command line option is supported.
+
+gpu-virgl
+
+    The --virgl command line option is supported.
+
+* --render-node=PATH (optional)
+
+Specify the GPU DRM render node.
+
+* --virgl (optional)
+
+Enable virgl rendering support.
-- 
2.19.0


Re: [libvirt] [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Ján Tomko 5 years, 6 months ago
On Fri, Sep 14, 2018 at 05:52:30PM +0400, Marc-André Lureau wrote:
>As discussed during "[PATCH v4 00/29] vhost-user for input & GPU"
>review, let's define a common set of backend conventions to help with
>management layer implementation, and interoperability.
>
>v2:
> - drop --pidfile
> - add some notes about daemonizing & stdin/out/err
>
>Cc: libvir-list@redhat.com
>Cc: Gerd Hoffmann <kraxel@redhat.com>
>Cc: Daniel P. Berrangé <berrange@redhat.com>
>Cc: Changpeng Liu <changpeng.liu@intel.com>
>Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>Cc: Felipe Franciosi <felipe@nutanix.com>
>Cc: Gonglei <arei.gonglei@huawei.com>
>Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>Cc: Michael S. Tsirkin <mst@redhat.com>
>Cc: Victor Kaplansky <victork@redhat.com>
>Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>---
> docs/interop/vhost-user.txt | 109 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 107 insertions(+), 2 deletions(-)
>
>diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
>index ba5e37d714..339b335e9c 100644
>--- a/docs/interop/vhost-user.txt
>+++ b/docs/interop/vhost-user.txt
>@@ -17,8 +17,13 @@ The protocol defines 2 sides of the communication, master and slave. Master is
> the application that shares its virtqueues, in our case QEMU. Slave is the
> consumer of the virtqueues.
>
>-In the current implementation QEMU is the Master, and the Slave is intended to
>-be a software Ethernet switch running in user space, such as Snabbswitch.
>+In the current implementation QEMU is the Master, and the Slave is the
>+external process consuming the virtio queues, for example a software
>+Ethernet switch running in user space, such as Snabbswitch, or a block
>+device backend processing read & write to a virtual disk. In order to
>+facilitate interoperability between various backend implementations,
>+it is recommended to follow the "Backend program conventions"
>+described in this document.
>
> Master and slave can be either a client (i.e. connecting) or server (listening)
> in the socket communication.
>@@ -859,3 +864,103 @@ resilient for selective requests.
> For the message types that already solicit a reply from the client, the
> presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being set brings
> no behavioural change. (See the 'Communication' section for details.)
>+
>+Backend program conventions
>+---------------------------
>+
>+vhost-user backends provide various services and they may need to be
>+configured manually depending on the use case. However, it is a good
>+idea to follow the conventions listed here when possible. Users, QEMU
>+or libvirt, can then rely on some common behaviour to avoid
>+heterogenous configuration and management of the backend program and
>+facilitate interoperability.
>+
>+In order to be discoverable, default vhost-user backends should be
>+located under "/usr/libexec", and be named "vhost-user-$device" where
>+"$device" is the device name in lower-case following the name listed
>+in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
>+backend would be named "vhost-user-rproc-serial").
>+
>+Mechanisms to list, and to select among alternatives implementations
>+or modify the default backend are not described at this point (a
>+distribution may use update-alternatives, for example, to list and to
>+pick a different default backend).
>+
>+The backend program must not daemonize itself, but it may be
>+daemonized by the management layer. It may also have a restricted
>+access to the system.
>+
>+File descriptors 0, 1 and 2 will exist, and have regular
>+stdin/stdout/stderr usage (they may be redirected to /dev/null by the
>+management layer, or to a log handler).
>+
>+The backend program must end (as quickly and cleanly as possible) when
>+the SIGTERM signal is received. Eventually, it may be SIGKILL by the
>+management layer after a few seconds.
>+
>+The following command line options have an expected behaviour. They
>+are mandatory, unless explicitly said differently:
>+
>+* --socket-path=PATH
>+
>+This option specify the location of the vhost-user Unix domain socket.

Will the backend program listen on this socket or connect to it?

>+It is incompatible with --fd.
>+
>+* --fd=FDNUM
>+
>+When this argument is given, the backend program is started with the
>+vhost-user socket as file descriptor FDNUM. It is incompatible with
>+--socket-path.
>+
>+* --print-capabilities
>+
>+Output to stdout a line-seperated list of backend capabilities, and
>+then exit successfully. Other options and arguments should be ignored,
>+and the backend program should not perform its normal function.
>+
>+At the time of writing, there are no common capabilities. Some
>+device-specific capabilities are listed in the respective sections. By
>+convention, device-specific capabilities are prefixed by their device
>+name.

Ideally, this would not be needed by libvirt.
QEMU capability probing is nice to have when we need to provide a
helpful error message (and avoid the lengthy preparation and cleanup
in case it fails), but absolutely necessary if we need to supply
different command line arguments to maintain the same functionality.

I was really happy to be able to delete qemu-img help parsing when
we dropped support for QEMU < 1.5.0 in upstream libvirt,
see commit e5261d8fe382483a85b86bfad7ba1d28dc939687

So unless the helper's command line arguments get new meaning in new
versions, libvirt can just pass on all the options requested in domain
XML and expect the helper to fail if it cannot satisfy them.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Marc-André Lureau 5 years, 6 months ago
Hi

On Fri, Oct 5, 2018 at 5:51 PM Ján Tomko <jtomko@redhat.com> wrote:
>
> On Fri, Sep 14, 2018 at 05:52:30PM +0400, Marc-André Lureau wrote:
> >As discussed during "[PATCH v4 00/29] vhost-user for input & GPU"
> >review, let's define a common set of backend conventions to help with
> >management layer implementation, and interoperability.
> >
> >v2:
> > - drop --pidfile
> > - add some notes about daemonizing & stdin/out/err
> >
> >Cc: libvir-list@redhat.com
> >Cc: Gerd Hoffmann <kraxel@redhat.com>
> >Cc: Daniel P. Berrangé <berrange@redhat.com>
> >Cc: Changpeng Liu <changpeng.liu@intel.com>
> >Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >Cc: Felipe Franciosi <felipe@nutanix.com>
> >Cc: Gonglei <arei.gonglei@huawei.com>
> >Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> >Cc: Michael S. Tsirkin <mst@redhat.com>
> >Cc: Victor Kaplansky <victork@redhat.com>
> >Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >---
> > docs/interop/vhost-user.txt | 109 +++++++++++++++++++++++++++++++++++-
> > 1 file changed, 107 insertions(+), 2 deletions(-)
> >
> >diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> >index ba5e37d714..339b335e9c 100644
> >--- a/docs/interop/vhost-user.txt
> >+++ b/docs/interop/vhost-user.txt
> >@@ -17,8 +17,13 @@ The protocol defines 2 sides of the communication, master and slave. Master is
> > the application that shares its virtqueues, in our case QEMU. Slave is the
> > consumer of the virtqueues.
> >
> >-In the current implementation QEMU is the Master, and the Slave is intended to
> >-be a software Ethernet switch running in user space, such as Snabbswitch.
> >+In the current implementation QEMU is the Master, and the Slave is the
> >+external process consuming the virtio queues, for example a software
> >+Ethernet switch running in user space, such as Snabbswitch, or a block
> >+device backend processing read & write to a virtual disk. In order to
> >+facilitate interoperability between various backend implementations,
> >+it is recommended to follow the "Backend program conventions"
> >+described in this document.
> >
> > Master and slave can be either a client (i.e. connecting) or server (listening)
> > in the socket communication.
> >@@ -859,3 +864,103 @@ resilient for selective requests.
> > For the message types that already solicit a reply from the client, the
> > presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being set brings
> > no behavioural change. (See the 'Communication' section for details.)
> >+
> >+Backend program conventions
> >+---------------------------
> >+
> >+vhost-user backends provide various services and they may need to be
> >+configured manually depending on the use case. However, it is a good
> >+idea to follow the conventions listed here when possible. Users, QEMU
> >+or libvirt, can then rely on some common behaviour to avoid
> >+heterogenous configuration and management of the backend program and
> >+facilitate interoperability.
> >+
> >+In order to be discoverable, default vhost-user backends should be
> >+located under "/usr/libexec", and be named "vhost-user-$device" where
> >+"$device" is the device name in lower-case following the name listed
> >+in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
> >+backend would be named "vhost-user-rproc-serial").
> >+
> >+Mechanisms to list, and to select among alternatives implementations
> >+or modify the default backend are not described at this point (a
> >+distribution may use update-alternatives, for example, to list and to
> >+pick a different default backend).
> >+
> >+The backend program must not daemonize itself, but it may be
> >+daemonized by the management layer. It may also have a restricted
> >+access to the system.
> >+
> >+File descriptors 0, 1 and 2 will exist, and have regular
> >+stdin/stdout/stderr usage (they may be redirected to /dev/null by the
> >+management layer, or to a log handler).
> >+
> >+The backend program must end (as quickly and cleanly as possible) when
> >+the SIGTERM signal is received. Eventually, it may be SIGKILL by the
> >+management layer after a few seconds.
> >+
> >+The following command line options have an expected behaviour. They
> >+are mandatory, unless explicitly said differently:
> >+
> >+* --socket-path=PATH
> >+
> >+This option specify the location of the vhost-user Unix domain socket.
>
> Will the backend program listen on this socket or connect to it?

Connect (if we ever need to support the listen mode, we can have another option)

>
> >+It is incompatible with --fd.
> >+
> >+* --fd=FDNUM
> >+
> >+When this argument is given, the backend program is started with the
> >+vhost-user socket as file descriptor FDNUM. It is incompatible with
> >+--socket-path.
> >+
> >+* --print-capabilities
> >+
> >+Output to stdout a line-seperated list of backend capabilities, and
> >+then exit successfully. Other options and arguments should be ignored,
> >+and the backend program should not perform its normal function.
> >+
> >+At the time of writing, there are no common capabilities. Some
> >+device-specific capabilities are listed in the respective sections. By
> >+convention, device-specific capabilities are prefixed by their device
> >+name.
>
> Ideally, this would not be needed by libvirt.
> QEMU capability probing is nice to have when we need to provide a
> helpful error message (and avoid the lengthy preparation and cleanup
> in case it fails), but absolutely necessary if we need to supply
> different command line arguments to maintain the same functionality.
>
> I was really happy to be able to delete qemu-img help parsing when
> we dropped support for QEMU < 1.5.0 in upstream libvirt,
> see commit e5261d8fe382483a85b86bfad7ba1d28dc939687
>
> So unless the helper's command line arguments get new meaning in new
> versions, libvirt can just pass on all the options requested in domain
> XML and expect the helper to fail if it cannot satisfy them.

qemu-img is not exchangeable, and provides a fixed/growing set of functions.

Here the goal is to allow vhost-user backend to be implemented by
various parties, with various capabilities.

For example, "virgl" support may not be implemented. This
--print-capabilities is a simple way to check what the backend
implements.

All the capabilities currently have a corresponding program argument,
which should be listed in --help. But it's not necessarily the case
for all future capabilities (for example, migration, etc..)

I agree that libvirt may not need to check the capabilities for now,
and instead rely on a runtime/starting error.

thanks

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Hoffmann, Gerd 5 years, 6 months ago
  Hi,

> For example, "virgl" support may not be implemented. This
> --print-capabilities is a simple way to check what the backend
> implements.

What is the expected behavior in case virgl is implemented by the
backend, but not available (due to lack of 3d hardware in the host for
example) ?

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Marc-André Lureau 5 years, 6 months ago
Hi

On Fri, Oct 5, 2018 at 7:59 PM Hoffmann, Gerd <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > For example, "virgl" support may not be implemented. This
> > --print-capabilities is a simple way to check what the backend
> > implements.
>
> What is the expected behavior in case virgl is implemented by the
> backend, but not available (due to lack of 3d hardware in the host for
> example) ?

I would say either it fails at runtime, or the backend doesn't announce it.

Would you want something else?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Hoffmann, Gerd 5 years, 6 months ago
On Sat, Oct 06, 2018 at 02:32:45AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Oct 5, 2018 at 7:59 PM Hoffmann, Gerd <kraxel@redhat.com> wrote:
> >
> >   Hi,
> >
> > > For example, "virgl" support may not be implemented. This
> > > --print-capabilities is a simple way to check what the backend
> > > implements.
> >
> > What is the expected behavior in case virgl is implemented by the
> > backend, but not available (due to lack of 3d hardware in the host for
> > example) ?
> 
> I would say either it fails at runtime, or the backend doesn't announce it.
> 
> Would you want something else?

Yes, one of the two, but we should clarify which one.

I suspect "fail at runtime" is easier for libvirt, given that it libvirt
must handle backend failures anyway ...

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Daniel P. Berrangé 5 years, 6 months ago
Adding Markus since we're talking about new CLI argument and capability
reporting standards.

On Fri, Sep 14, 2018 at 05:52:30PM +0400, Marc-André Lureau wrote:
> As discussed during "[PATCH v4 00/29] vhost-user for input & GPU"
> review, let's define a common set of backend conventions to help with
> management layer implementation, and interoperability.
> 
> v2:
>  - drop --pidfile
>  - add some notes about daemonizing & stdin/out/err
> 
> Cc: libvir-list@redhat.com
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Changpeng Liu <changpeng.liu@intel.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Felipe Franciosi <felipe@nutanix.com>
> Cc: Gonglei <arei.gonglei@huawei.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Victor Kaplansky <victork@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  docs/interop/vhost-user.txt | 109 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index ba5e37d714..339b335e9c 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -17,8 +17,13 @@ The protocol defines 2 sides of the communication, master and slave. Master is
>  the application that shares its virtqueues, in our case QEMU. Slave is the
>  consumer of the virtqueues.
>  
> -In the current implementation QEMU is the Master, and the Slave is intended to
> -be a software Ethernet switch running in user space, such as Snabbswitch.
> +In the current implementation QEMU is the Master, and the Slave is the
> +external process consuming the virtio queues, for example a software
> +Ethernet switch running in user space, such as Snabbswitch, or a block
> +device backend processing read & write to a virtual disk. In order to
> +facilitate interoperability between various backend implementations,
> +it is recommended to follow the "Backend program conventions"
> +described in this document.
>  
>  Master and slave can be either a client (i.e. connecting) or server (listening)
>  in the socket communication.
> @@ -859,3 +864,103 @@ resilient for selective requests.
>  For the message types that already solicit a reply from the client, the
>  presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being set brings
>  no behavioural change. (See the 'Communication' section for details.)
> +
> +Backend program conventions
> +---------------------------
> +
> +vhost-user backends provide various services and they may need to be
> +configured manually depending on the use case. However, it is a good
> +idea to follow the conventions listed here when possible. Users, QEMU
> +or libvirt, can then rely on some common behaviour to avoid
> +heterogenous configuration and management of the backend program and
> +facilitate interoperability.
> +
> +In order to be discoverable, default vhost-user backends should be
> +located under "/usr/libexec", and be named "vhost-user-$device" where
> +"$device" is the device name in lower-case following the name listed
> +in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
> +backend would be named "vhost-user-rproc-serial").
> +
> +Mechanisms to list, and to select among alternatives implementations
> +or modify the default backend are not described at this point (a
> +distribution may use update-alternatives, for example, to list and to
> +pick a different default backend).

I don't think that update-alternatives is a good thing as it presumes
that each host only needs a single preferred impl at a time.

I think we need to be able to discover all impls for a given device
type.

This feels like the same problem we tackled recently with enumerating
and choosing between multiple firmware impls.

In $git/docs/interop/firmware.json we defined a way to drop config files
into a standard directory, providing info about the firmware in a well
defined QAPI based data format.

Rather than requiring a special file naming convention I think we just
need to register config files in a particular directory, letting the
mgmt app enumerate them.

eg

  /etc/qemu/vhost-user/50-rproc-serial.json  (a default imp from QEMU)
  /etc/qemu/vhost-user/10-my-rproc-serial.json (my replacenment impl)

a file could be something pretty simple like

   {
       "name": "my-rproc-serial",
       "description": "My rproc serial impl doing foo, bar, wizz",
       "device": "rproc-serial",
       "binary": "/usr/libexec/my-awesome-rproc-serial",
   }

Mgmt apps can simply load all files in that directory to learn about
the possible impls. The file load order gives a prioritization if
multiple matches exist, or a specific impl can be requested by
name "my-rproc-serial".

This shouldn't provide full capabilities reporting though, just
enough to identify viable binaries. Capabilities should still be
via the binary itself so it can be dynamically tailored based on
other environmental factors

> +
> +The backend program must not daemonize itself, but it may be
> +daemonized by the management layer. It may also have a restricted
> +access to the system.
> +
> +File descriptors 0, 1 and 2 will exist, and have regular
> +stdin/stdout/stderr usage (they may be redirected to /dev/null by the
> +management layer, or to a log handler).
> +
> +The backend program must end (as quickly and cleanly as possible) when
> +the SIGTERM signal is received. Eventually, it may be SIGKILL by the
> +management layer after a few seconds.
> +
> +The following command line options have an expected behaviour. They
> +are mandatory, unless explicitly said differently:
> +
> +* --socket-path=PATH
> +
> +This option specify the location of the vhost-user Unix domain socket.
> +It is incompatible with --fd.
> +
> +* --fd=FDNUM
> +
> +When this argument is given, the backend program is started with the
> +vhost-user socket as file descriptor FDNUM. It is incompatible with
> +--socket-path.
> +
> +* --print-capabilities
> +
> +Output to stdout a line-seperated list of backend capabilities, and
> +then exit successfully. Other options and arguments should be ignored,
> +and the backend program should not perform its normal function.

This is going to repeat the mistakes we've had with every other
binary in QEMU. A "simple" flag list or args sounds appealing,
but we've always been burnt by it in the medium-long term, which
is why we created QAPI.

If we're doing to have any capabilities reporting, we should
model it in QAPI schema, so any '--print-capabilities' arg
should print a JSON doc following the documented schema.

While talking about QAPI, I think this is an opportunity to
also avoid the problems of CLI arg values becoming more
complex than just scalars. eg

  --socket-path=PATH

may inevitably grow more options  - eg to perhaps say whether
to use it in listen or connect mode. Or to indicate a reconnect
timeout. etc

I know Markus wants to replace QemuOpts with something that
is again driven by QAPI, so that "-arg $VALUE" can handle
$VALUE being complex  non-scalar data following a QAPI
schema with well defined semantics for parsing. Since we
are defining a new standard, I think we should go todo
something better than scalar values right from the start.

> +
> +At the time of writing, there are no common capabilities. Some
> +device-specific capabilities are listed in the respective sections. By
> +convention, device-specific capabilities are prefixed by their device
> +name.
> +
> +vhost-user-input program conventions
> +------------------------------------
> +
> +Capabilities:
> +
> +input-evdev-path
> +
> +    The --evdev-path command line option is supported.
> +
> +input-no-grab
> +
> +    The --no-grab command line option is supported.
> +
> +* --evdev-path=PATH (optional)
> +
> +Specify the linux input device.
> +
> +* --no-grab (optional)
> +
> +Do no request exclusive access to the input device.
> +
> +vhost-user-gpu program conventions
> +----------------------------------
> +
> +Capabilities:
> +
> +gpu-render-node
> +
> +    The --render-node command line option is supported.
> +
> +gpu-virgl
> +
> +    The --virgl command line option is supported.
> +
> +* --render-node=PATH (optional)
> +
> +Specify the GPU DRM render node.
> +
> +* --virgl (optional)
> +
> +Enable virgl rendering support.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt] [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Daniel P. Berrangé 5 years, 6 months ago
On Thu, Oct 11, 2018 at 04:48:34PM +0100, Daniel P. Berrangé wrote:
> Adding Markus since we're talking about new CLI argument and capability
> reporting standards.
> 
> On Fri, Sep 14, 2018 at 05:52:30PM +0400, Marc-André Lureau wrote:
> > As discussed during "[PATCH v4 00/29] vhost-user for input & GPU"
> > review, let's define a common set of backend conventions to help with
> > management layer implementation, and interoperability.
> > 
> > v2:
> >  - drop --pidfile
> >  - add some notes about daemonizing & stdin/out/err
> > 
> > Cc: libvir-list@redhat.com
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Changpeng Liu <changpeng.liu@intel.com>
> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Cc: Felipe Franciosi <felipe@nutanix.com>
> > Cc: Gonglei <arei.gonglei@huawei.com>
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Victor Kaplansky <victork@redhat.com>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  docs/interop/vhost-user.txt | 109 +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 107 insertions(+), 2 deletions(-)
> > 
> > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> > index ba5e37d714..339b335e9c 100644
> > --- a/docs/interop/vhost-user.txt
> > +++ b/docs/interop/vhost-user.txt
> > @@ -17,8 +17,13 @@ The protocol defines 2 sides of the communication, master and slave. Master is
> >  the application that shares its virtqueues, in our case QEMU. Slave is the
> >  consumer of the virtqueues.
> >  
> > -In the current implementation QEMU is the Master, and the Slave is intended to
> > -be a software Ethernet switch running in user space, such as Snabbswitch.
> > +In the current implementation QEMU is the Master, and the Slave is the
> > +external process consuming the virtio queues, for example a software
> > +Ethernet switch running in user space, such as Snabbswitch, or a block
> > +device backend processing read & write to a virtual disk. In order to
> > +facilitate interoperability between various backend implementations,
> > +it is recommended to follow the "Backend program conventions"
> > +described in this document.
> >  
> >  Master and slave can be either a client (i.e. connecting) or server (listening)
> >  in the socket communication.
> > @@ -859,3 +864,103 @@ resilient for selective requests.
> >  For the message types that already solicit a reply from the client, the
> >  presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being set brings
> >  no behavioural change. (See the 'Communication' section for details.)
> > +
> > +Backend program conventions
> > +---------------------------
> > +
> > +vhost-user backends provide various services and they may need to be
> > +configured manually depending on the use case. However, it is a good
> > +idea to follow the conventions listed here when possible. Users, QEMU
> > +or libvirt, can then rely on some common behaviour to avoid
> > +heterogenous configuration and management of the backend program and
> > +facilitate interoperability.
> > +
> > +In order to be discoverable, default vhost-user backends should be
> > +located under "/usr/libexec", and be named "vhost-user-$device" where
> > +"$device" is the device name in lower-case following the name listed
> > +in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
> > +backend would be named "vhost-user-rproc-serial").
> > +
> > +Mechanisms to list, and to select among alternatives implementations
> > +or modify the default backend are not described at this point (a
> > +distribution may use update-alternatives, for example, to list and to
> > +pick a different default backend).
> 
> I don't think that update-alternatives is a good thing as it presumes
> that each host only needs a single preferred impl at a time.
> 
> I think we need to be able to discover all impls for a given device
> type.
> 
> This feels like the same problem we tackled recently with enumerating
> and choosing between multiple firmware impls.
> 
> In $git/docs/interop/firmware.json we defined a way to drop config files
> into a standard directory, providing info about the firmware in a well
> defined QAPI based data format.
> 
> Rather than requiring a special file naming convention I think we just
> need to register config files in a particular directory, letting the
> mgmt app enumerate them.
> 
> eg
> 
>   /etc/qemu/vhost-user/50-rproc-serial.json  (a default imp from QEMU)
>   /etc/qemu/vhost-user/10-my-rproc-serial.json (my replacenment impl)
> 
> a file could be something pretty simple like
> 
>    {
>        "name": "my-rproc-serial",
>        "description": "My rproc serial impl doing foo, bar, wizz",
>        "device": "rproc-serial",
>        "binary": "/usr/libexec/my-awesome-rproc-serial",
>    }
> 
> Mgmt apps can simply load all files in that directory to learn about
> the possible impls. The file load order gives a prioritization if
> multiple matches exist, or a specific impl can be requested by
> name "my-rproc-serial".
> 
> This shouldn't provide full capabilities reporting though, just
> enough to identify viable binaries. Capabilities should still be
> via the binary itself so it can be dynamically tailored based on
> other environmental factors
> 
> > +
> > +The backend program must not daemonize itself, but it may be
> > +daemonized by the management layer. It may also have a restricted
> > +access to the system.
> > +
> > +File descriptors 0, 1 and 2 will exist, and have regular
> > +stdin/stdout/stderr usage (they may be redirected to /dev/null by the
> > +management layer, or to a log handler).
> > +
> > +The backend program must end (as quickly and cleanly as possible) when
> > +the SIGTERM signal is received. Eventually, it may be SIGKILL by the
> > +management layer after a few seconds.
> > +
> > +The following command line options have an expected behaviour. They
> > +are mandatory, unless explicitly said differently:
> > +
> > +* --socket-path=PATH
> > +
> > +This option specify the location of the vhost-user Unix domain socket.
> > +It is incompatible with --fd.
> > +
> > +* --fd=FDNUM
> > +
> > +When this argument is given, the backend program is started with the
> > +vhost-user socket as file descriptor FDNUM. It is incompatible with
> > +--socket-path.
> > +
> > +* --print-capabilities
> > +
> > +Output to stdout a line-seperated list of backend capabilities, and
> > +then exit successfully. Other options and arguments should be ignored,
> > +and the backend program should not perform its normal function.
> 
> This is going to repeat the mistakes we've had with every other
> binary in QEMU. A "simple" flag list or args sounds appealing,
> but we've always been burnt by it in the medium-long term, which
> is why we created QAPI.
> 
> If we're doing to have any capabilities reporting, we should
> model it in QAPI schema, so any '--print-capabilities' arg
> should print a JSON doc following the documented schema.
> 
> While talking about QAPI, I think this is an opportunity to
> also avoid the problems of CLI arg values becoming more
> complex than just scalars. eg
> 
>   --socket-path=PATH
> 
> may inevitably grow more options  - eg to perhaps say whether
> to use it in listen or connect mode. Or to indicate a reconnect
> timeout. etc
> 
> I know Markus wants to replace QemuOpts with something that
> is again driven by QAPI, so that "-arg $VALUE" can handle
> $VALUE being complex  non-scalar data following a QAPI
> schema with well defined semantics for parsing. Since we
> are defining a new standard, I think we should go todo
> something better than scalar values right from the start.
> 
> > +
> > +At the time of writing, there are no common capabilities. Some
> > +device-specific capabilities are listed in the respective sections. By
> > +convention, device-specific capabilities are prefixed by their device
> > +name.
> > +
> > +vhost-user-input program conventions
> > +------------------------------------
> > +
> > +Capabilities:
> > +
> > +input-evdev-path
> > +
> > +    The --evdev-path command line option is supported.
> > +
> > +input-no-grab
> > +
> > +    The --no-grab command line option is supported.
> > +
> > +* --evdev-path=PATH (optional)
> > +
> > +Specify the linux input device.
> > +
> > +* --no-grab (optional)
> > +
> > +Do no request exclusive access to the input device.
> > +
> > +vhost-user-gpu program conventions
> > +----------------------------------
> > +
> > +Capabilities:
> > +
> > +gpu-render-node
> > +
> > +    The --render-node command line option is supported.
> > +
> > +gpu-virgl
> > +
> > +    The --virgl command line option is supported.
> > +
> > +* --render-node=PATH (optional)
> > +
> > +Specify the GPU DRM render node.
> > +
> > +* --virgl (optional)
> > +
> > +Enable virgl rendering support.

As a rough illustration I mocked up a possible QAPI schema that covers
the templates describing the binaries, the format of CLI arguments, and
the data for capabilities.

Note, I can't remember what Markus had proposed for CLI arguments in
QAPI, so I invented something arbitary but plausible.



#
# The type of device the vhost-user backend is for
#
{ 'enum': 'VHostUserBackendType',
  'data': 'input', 'gpu', ... }


#
# @type: the type of backend interface provided
# @name: short name of the impl, unique wrt @type
# @description: a human-readable description of the firmware.
# @binary: fully qualified path to the binary
#
{
    'struct': 'VHostUserBackend',
    'data': {
	'type': 'VHostUserBackendType',
	'name': 'str'
	'description': 'str'
	'binary': 'str'
    }
}



#
# Command line options common to all vhost user backends
#
{
    'optionset': 'VHostUserBackendCommandLineBase',
    'data': [
	{
	    'option': '--print-capabilities',
            'help': 'Print backend capabilities document',
	},
	{
	    'option': '--socket',
	    'data': 'ChardevSocket',
	    'help': 'Socket to communicate with frontend',
	},
    ]
}


#
# Command line options for vhost user "input" backends
#
{
    'optionset': 'VHostUserBackendCommandLineInput',
    'base': 'VHostUserBackendCommandLineBase',
    'data': [
	{
	    'option': '--evdev-path',
	    'data': 'str',
            'help': 'The Linux input device path',
	},
	{
	    'option': '--no-grab',
	    'data': 'str',
	    'help': 'Do not request exclusive access to device',
	},
    ]
}


#
# Command line options for vhost user "gpu" backends
#
{
    'optionset': 'VHostUserBackendCommandLineGPU',
    'base': 'VHostUserBackendCommandLineBase',
    'data': [
	{
	    'option': '--render-node',
	    'data': 'str',
            'help': 'The GPU DRM render node path',
	},
	{
	    'option': '--virgl',
	    'help': 'Enable virgl rendering support',
	},
    ]
}


#
# Command line options for vhost user backends
#
{
    'union': 'VHostUserBackendCommandLine',
    'base': { 'type': 'VHostUserBackendType' },
    'discriminator': 'type',
    'data': {
	'input': 'VHostUserBackendCommandLineInput',
	'gpu': 'VHostUserBackendCommandLineGPU',
    }
}


{
    'enum': 'VHostUserBackendInputFeature',
    'data': { 'evdev-path', 'no-grab', }
}

#
# Capabilities reported by vhost user "input" backends
#
{
    'struct': 'VHostUserBackendCapabilitiesInput',
    'data': {
        'features': [ 'VHostUserBackendInputFeature' ],
    }
}



{
    'enum': 'VHostUserBackendGPUFeature',
    'data': { 'render-node', 'virgl' }
}


#
# Capabilities reported by vhost user "gpu" backends
#
{
    'struct': 'VHostUserBackendCapabilitiesGPU',
    'data': {
        'features': [ 'VHostUserBackendGPUFeature' ],
    }
}


#
# Capabilities reported by vhost user backends
#
{
    'union': 'VHostUserBackendCapabilities',
    'base': { 'type': 'VHostUserBackendType' },
    'discriminator': 'type',
    'data': {
	'input': 'VHostUserBackendCapabilitiesInput',
	'gpu': 'VHostUserBackendCapabilitiesGPU',
    }
}



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Gerd Hoffmann 5 years, 6 months ago
  Hi,

> Note, I can't remember what Markus had proposed for CLI arguments in
> QAPI, so I invented something arbitary but plausible.

Using qapi visitors to parse the command line.  Used by -blockdev and
-display (and maybe others meanwhile).  See parse_display_qapi().

I think Daniels suggestion was to do that in the backends ...

cheers,
  Gerd

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Daniel P. Berrangé 5 years, 6 months ago
On Fri, Oct 12, 2018 at 12:23:11PM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > Note, I can't remember what Markus had proposed for CLI arguments in
> > QAPI, so I invented something arbitary but plausible.
> 
> Using qapi visitors to parse the command line.  Used by -blockdev and
> -display (and maybe others meanwhile).  See parse_display_qapi().
> 
> I think Daniels suggestion was to do that in the backends ...

More than just that - I was thinking of this series:

   http://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00209.html

In that there is no QemuOpts, or getopt(), etc. You end up using QAPI
for the entire argv parsing procss:

    QAPIOption *qopt;

    qopt = qapi_options_parse(argc, argv);

    for (i = 0; !qopt[i].cnt; i++) {
       switch (qopt[i].type) {
       case QAPI_OPTION_KIND_NODEFCONFIG:
           defconfig = false;
           break;
       case QAPI_OPTION_KIND_NO_USER_CONFIG:
           userconfig = false;
           break;
	   ...etc...
    }

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Marc-André Lureau 5 years, 6 months ago
Hi

On Thu, Oct 11, 2018 at 7:49 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Adding Markus since we're talking about new CLI argument and capability
> reporting standards.
>
> On Fri, Sep 14, 2018 at 05:52:30PM +0400, Marc-André Lureau wrote:
> > As discussed during "[PATCH v4 00/29] vhost-user for input & GPU"
> > review, let's define a common set of backend conventions to help with
> > management layer implementation, and interoperability.
> >
> > v2:
> >  - drop --pidfile
> >  - add some notes about daemonizing & stdin/out/err
> >
> > Cc: libvir-list@redhat.com
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Daniel P. Berrangé <berrange@redhat.com>
> > Cc: Changpeng Liu <changpeng.liu@intel.com>
> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Cc: Felipe Franciosi <felipe@nutanix.com>
> > Cc: Gonglei <arei.gonglei@huawei.com>
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Victor Kaplansky <victork@redhat.com>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  docs/interop/vhost-user.txt | 109 +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 107 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> > index ba5e37d714..339b335e9c 100644
> > --- a/docs/interop/vhost-user.txt
> > +++ b/docs/interop/vhost-user.txt
> > @@ -17,8 +17,13 @@ The protocol defines 2 sides of the communication, master and slave. Master is
> >  the application that shares its virtqueues, in our case QEMU. Slave is the
> >  consumer of the virtqueues.
> >
> > -In the current implementation QEMU is the Master, and the Slave is intended to
> > -be a software Ethernet switch running in user space, such as Snabbswitch.
> > +In the current implementation QEMU is the Master, and the Slave is the
> > +external process consuming the virtio queues, for example a software
> > +Ethernet switch running in user space, such as Snabbswitch, or a block
> > +device backend processing read & write to a virtual disk. In order to
> > +facilitate interoperability between various backend implementations,
> > +it is recommended to follow the "Backend program conventions"
> > +described in this document.
> >
> >  Master and slave can be either a client (i.e. connecting) or server (listening)
> >  in the socket communication.
> > @@ -859,3 +864,103 @@ resilient for selective requests.
> >  For the message types that already solicit a reply from the client, the
> >  presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being set brings
> >  no behavioural change. (See the 'Communication' section for details.)
> > +
> > +Backend program conventions
> > +---------------------------
> > +
> > +vhost-user backends provide various services and they may need to be
> > +configured manually depending on the use case. However, it is a good
> > +idea to follow the conventions listed here when possible. Users, QEMU
> > +or libvirt, can then rely on some common behaviour to avoid
> > +heterogenous configuration and management of the backend program and
> > +facilitate interoperability.
> > +
> > +In order to be discoverable, default vhost-user backends should be
> > +located under "/usr/libexec", and be named "vhost-user-$device" where
> > +"$device" is the device name in lower-case following the name listed
> > +in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
> > +backend would be named "vhost-user-rproc-serial").
> > +
> > +Mechanisms to list, and to select among alternatives implementations
> > +or modify the default backend are not described at this point (a
> > +distribution may use update-alternatives, for example, to list and to
> > +pick a different default backend).
>
> I don't think that update-alternatives is a good thing as it presumes
> that each host only needs a single preferred impl at a time.
>
> I think we need to be able to discover all impls for a given device
> type.

That was left for a future improvement. I don't think it's a good idea
to tackle problems we don't have yet.

>
> This feels like the same problem we tackled recently with enumerating
> and choosing between multiple firmware impls.
>
> In $git/docs/interop/firmware.json we defined a way to drop config files
> into a standard directory, providing info about the firmware in a well
> defined QAPI based data format.
>
> Rather than requiring a special file naming convention I think we just
> need to register config files in a particular directory, letting the
> mgmt app enumerate them.
>
> eg
>
>   /etc/qemu/vhost-user/50-rproc-serial.json  (a default imp from QEMU)
>   /etc/qemu/vhost-user/10-my-rproc-serial.json (my replacenment impl)
>
> a file could be something pretty simple like
>
>    {
>        "name": "my-rproc-serial",
>        "description": "My rproc serial impl doing foo, bar, wizz",
>        "device": "rproc-serial",
>        "binary": "/usr/libexec/my-awesome-rproc-serial",
>    }
>
> Mgmt apps can simply load all files in that directory to learn about
> the possible impls. The file load order gives a prioritization if
> multiple matches exist, or a specific impl can be requested by
> name "my-rproc-serial".
>
> This shouldn't provide full capabilities reporting though, just
> enough to identify viable binaries. Capabilities should still be
> via the binary itself so it can be dynamically tailored based on
> other environmental factors
>
> > +
> > +The backend program must not daemonize itself, but it may be
> > +daemonized by the management layer. It may also have a restricted
> > +access to the system.
> > +
> > +File descriptors 0, 1 and 2 will exist, and have regular
> > +stdin/stdout/stderr usage (they may be redirected to /dev/null by the
> > +management layer, or to a log handler).
> > +
> > +The backend program must end (as quickly and cleanly as possible) when
> > +the SIGTERM signal is received. Eventually, it may be SIGKILL by the
> > +management layer after a few seconds.
> > +
> > +The following command line options have an expected behaviour. They
> > +are mandatory, unless explicitly said differently:
> > +
> > +* --socket-path=PATH
> > +
> > +This option specify the location of the vhost-user Unix domain socket.
> > +It is incompatible with --fd.
> > +
> > +* --fd=FDNUM
> > +
> > +When this argument is given, the backend program is started with the
> > +vhost-user socket as file descriptor FDNUM. It is incompatible with
> > +--socket-path.
> > +
> > +* --print-capabilities
> > +
> > +Output to stdout a line-seperated list of backend capabilities, and
> > +then exit successfully. Other options and arguments should be ignored,
> > +and the backend program should not perform its normal function.
>
> This is going to repeat the mistakes we've had with every other
> binary in QEMU. A "simple" flag list or args sounds appealing,
> but we've always been burnt by it in the medium-long term, which
> is why we created QAPI.

isn't libvirt using a list of strings/symbols as well for the
capabilities? To me it sounds fairly easy to extend this way.

>
> If we're doing to have any capabilities reporting, we should
> model it in QAPI schema, so any '--print-capabilities' arg
> should print a JSON doc following the documented schema.

perhaps we could have --print-json-capabilities later, if needed.

>
> While talking about QAPI, I think this is an opportunity to
> also avoid the problems of CLI arg values becoming more
> complex than just scalars. eg
>
>   --socket-path=PATH
>
> may inevitably grow more options  - eg to perhaps say whether
> to use it in listen or connect mode. Or to indicate a reconnect
> timeout. etc

Yes, that would be new capabilities symbols. That doesn't require json to me.

>
> I know Markus wants to replace QemuOpts with something that
> is again driven by QAPI, so that "-arg $VALUE" can handle
> $VALUE being complex  non-scalar data following a QAPI
> schema with well defined semantics for parsing. Since we
> are defining a new standard, I think we should go todo
> something better than scalar values right from the start.
>
> > +
> > +At the time of writing, there are no common capabilities. Some
> > +device-specific capabilities are listed in the respective sections. By
> > +convention, device-specific capabilities are prefixed by their device
> > +name.
> > +
> > +vhost-user-input program conventions
> > +------------------------------------
> > +
> > +Capabilities:
> > +
> > +input-evdev-path
> > +
> > +    The --evdev-path command line option is supported.
> > +
> > +input-no-grab
> > +
> > +    The --no-grab command line option is supported.
> > +
> > +* --evdev-path=PATH (optional)
> > +
> > +Specify the linux input device.
> > +
> > +* --no-grab (optional)
> > +
> > +Do no request exclusive access to the input device.
> > +
> > +vhost-user-gpu program conventions
> > +----------------------------------
> > +
> > +Capabilities:
> > +
> > +gpu-render-node
> > +
> > +    The --render-node command line option is supported.
> > +
> > +gpu-virgl
> > +
> > +    The --virgl command line option is supported.
> > +
> > +* --render-node=PATH (optional)
> > +
> > +Specify the GPU DRM render node.
> > +
> > +* --virgl (optional)
> > +
> > +Enable virgl rendering support.
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Daniel P. Berrangé 5 years, 6 months ago
On Fri, Oct 12, 2018 at 01:43:39PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Oct 11, 2018 at 7:49 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > Adding Markus since we're talking about new CLI argument and capability
> > reporting standards.
> >
> > On Fri, Sep 14, 2018 at 05:52:30PM +0400, Marc-André Lureau wrote:
> > > +Backend program conventions
> > > +---------------------------
> > > +
> > > +vhost-user backends provide various services and they may need to be
> > > +configured manually depending on the use case. However, it is a good
> > > +idea to follow the conventions listed here when possible. Users, QEMU
> > > +or libvirt, can then rely on some common behaviour to avoid
> > > +heterogenous configuration and management of the backend program and
> > > +facilitate interoperability.
> > > +
> > > +In order to be discoverable, default vhost-user backends should be
> > > +located under "/usr/libexec", and be named "vhost-user-$device" where
> > > +"$device" is the device name in lower-case following the name listed
> > > +in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
> > > +backend would be named "vhost-user-rproc-serial").
> > > +
> > > +Mechanisms to list, and to select among alternatives implementations
> > > +or modify the default backend are not described at this point (a
> > > +distribution may use update-alternatives, for example, to list and to
> > > +pick a different default backend).
> >
> > I don't think that update-alternatives is a good thing as it presumes
> > that each host only needs a single preferred impl at a time.
> >
> > I think we need to be able to discover all impls for a given device
> > type.
> 
> That was left for a future improvement. I don't think it's a good idea
> to tackle problems we don't have yet.

I think the need to support multiple alternatives will arrive pretty
much immediately, as it has already been raised in previous threads
about vhost-user. It is counterproductive to implement something we
know is not satisfactory when we can easily provide an approach
which is extensible for no significant extra work upfront. Having
it aligned with the approach we do for firmware makes it even more
compelling, as we get a consistent story across QEMU instead of
two completely different approaches for the same concept.



> > > +The backend program must not daemonize itself, but it may be
> > > +daemonized by the management layer. It may also have a restricted
> > > +access to the system.
> > > +
> > > +File descriptors 0, 1 and 2 will exist, and have regular
> > > +stdin/stdout/stderr usage (they may be redirected to /dev/null by the
> > > +management layer, or to a log handler).
> > > +
> > > +The backend program must end (as quickly and cleanly as possible) when
> > > +the SIGTERM signal is received. Eventually, it may be SIGKILL by the
> > > +management layer after a few seconds.
> > > +
> > > +The following command line options have an expected behaviour. They
> > > +are mandatory, unless explicitly said differently:
> > > +
> > > +* --socket-path=PATH
> > > +
> > > +This option specify the location of the vhost-user Unix domain socket.
> > > +It is incompatible with --fd.
> > > +
> > > +* --fd=FDNUM
> > > +
> > > +When this argument is given, the backend program is started with the
> > > +vhost-user socket as file descriptor FDNUM. It is incompatible with
> > > +--socket-path.
> > > +
> > > +* --print-capabilities
> > > +
> > > +Output to stdout a line-seperated list of backend capabilities, and
> > > +then exit successfully. Other options and arguments should be ignored,
> > > +and the backend program should not perform its normal function.
> >
> > This is going to repeat the mistakes we've had with every other
> > binary in QEMU. A "simple" flag list or args sounds appealing,
> > but we've always been burnt by it in the medium-long term, which
> > is why we created QAPI.
> 
> isn't libvirt using a list of strings/symbols as well for the
> capabilities? To me it sounds fairly easy to extend this way.
>
> > If we're doing to have any capabilities reporting, we should
> > model it in QAPI schema, so any '--print-capabilities' arg
> > should print a JSON doc following the documented schema.
> 
> perhaps we could have --print-json-capabilities later, if needed.

A QAPI schema can still start off as reporting a simple list of
features flags.

The key point is to provide a data format that is extensible
right from the start. For QEMU that means QAPI, instead of
inventing yet another custom data format, that we'd have to
fix later when we find a flat list of strings is insufficient.


> > While talking about QAPI, I think this is an opportunity to
> > also avoid the problems of CLI arg values becoming more
> > complex than just scalars. eg
> >
> >   --socket-path=PATH
> >
> > may inevitably grow more options  - eg to perhaps say whether
> > to use it in listen or connect mode. Or to indicate a reconnect
> > timeout. etc
> 
> Yes, that would be new capabilities symbols. That doesn't require json to me.

It just reinvents the chardev unix socket syntax, but in a
different adhoc manner, which is repeating the mistake we have
made time & again in QEMU. Using QAPI we can directly accept
the ChardevSocket syntax we already know about. Instead of
having --socket-path and --fd and documenting that they are
mutually exclusive ChardevSocket QAPI schema provides that
representation in a well defined format which is discoverable
and QEMU and mgmt apps already understand.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Marc-André Lureau 5 years, 6 months ago
Hi

On Fri, Oct 12, 2018 at 1:56 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Oct 12, 2018 at 01:43:39PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Thu, Oct 11, 2018 at 7:49 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > Adding Markus since we're talking about new CLI argument and capability
> > > reporting standards.
> > >
> > > On Fri, Sep 14, 2018 at 05:52:30PM +0400, Marc-André Lureau wrote:
> > > > +Backend program conventions
> > > > +---------------------------
> > > > +
> > > > +vhost-user backends provide various services and they may need to be
> > > > +configured manually depending on the use case. However, it is a good
> > > > +idea to follow the conventions listed here when possible. Users, QEMU
> > > > +or libvirt, can then rely on some common behaviour to avoid
> > > > +heterogenous configuration and management of the backend program and
> > > > +facilitate interoperability.
> > > > +
> > > > +In order to be discoverable, default vhost-user backends should be
> > > > +located under "/usr/libexec", and be named "vhost-user-$device" where
> > > > +"$device" is the device name in lower-case following the name listed
> > > > +in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
> > > > +backend would be named "vhost-user-rproc-serial").
> > > > +
> > > > +Mechanisms to list, and to select among alternatives implementations
> > > > +or modify the default backend are not described at this point (a
> > > > +distribution may use update-alternatives, for example, to list and to
> > > > +pick a different default backend).
> > >
> > > I don't think that update-alternatives is a good thing as it presumes
> > > that each host only needs a single preferred impl at a time.
> > >
> > > I think we need to be able to discover all impls for a given device
> > > type.
> >
> > That was left for a future improvement. I don't think it's a good idea
> > to tackle problems we don't have yet.
>
> I think the need to support multiple alternatives will arrive pretty
> much immediately, as it has already been raised in previous threads
> about vhost-user. It is counterproductive to implement something we

At this point, there are no alternative implementation of the
vhost-user-gpu or vhost-user-input backends (and no other device has
been implemented to follow this spec).

> know is not satisfactory when we can easily provide an approach
> which is extensible for no significant extra work upfront. Having
> it aligned with the approach we do for firmware makes it even more
> compelling, as we get a consistent story across QEMU instead of
> two completely different approaches for the same concept.

That's not incompatible with doing it as a future improvement.

>
>
> > > > +The backend program must not daemonize itself, but it may be
> > > > +daemonized by the management layer. It may also have a restricted
> > > > +access to the system.
> > > > +
> > > > +File descriptors 0, 1 and 2 will exist, and have regular
> > > > +stdin/stdout/stderr usage (they may be redirected to /dev/null by the
> > > > +management layer, or to a log handler).
> > > > +
> > > > +The backend program must end (as quickly and cleanly as possible) when
> > > > +the SIGTERM signal is received. Eventually, it may be SIGKILL by the
> > > > +management layer after a few seconds.
> > > > +
> > > > +The following command line options have an expected behaviour. They
> > > > +are mandatory, unless explicitly said differently:
> > > > +
> > > > +* --socket-path=PATH
> > > > +
> > > > +This option specify the location of the vhost-user Unix domain socket.
> > > > +It is incompatible with --fd.
> > > > +
> > > > +* --fd=FDNUM
> > > > +
> > > > +When this argument is given, the backend program is started with the
> > > > +vhost-user socket as file descriptor FDNUM. It is incompatible with
> > > > +--socket-path.
> > > > +
> > > > +* --print-capabilities
> > > > +
> > > > +Output to stdout a line-seperated list of backend capabilities, and
> > > > +then exit successfully. Other options and arguments should be ignored,
> > > > +and the backend program should not perform its normal function.
> > >
> > > This is going to repeat the mistakes we've had with every other
> > > binary in QEMU. A "simple" flag list or args sounds appealing,
> > > but we've always been burnt by it in the medium-long term, which
> > > is why we created QAPI.
> >
> > isn't libvirt using a list of strings/symbols as well for the
> > capabilities? To me it sounds fairly easy to extend this way.
> >
> > > If we're doing to have any capabilities reporting, we should
> > > model it in QAPI schema, so any '--print-capabilities' arg
> > > should print a JSON doc following the documented schema.
> >
> > perhaps we could have --print-json-capabilities later, if needed.
>
> A QAPI schema can still start off as reporting a simple list of
> features flags.
> The key point is to provide a data format that is extensible
> right from the start. For QEMU that means QAPI, instead of
> inventing yet another custom data format, that we'd have to
> fix later when we find a flat list of strings is insufficient.


Changing --print-capabilites from:
feature-a
feature-b

to a json form:
{
  "features": [
    "feature-a",
    "feature-b"
  ]
}

Would that be extensible enough?

>
>
> > > While talking about QAPI, I think this is an opportunity to
> > > also avoid the problems of CLI arg values becoming more
> > > complex than just scalars. eg
> > >
> > >   --socket-path=PATH
> > >
> > > may inevitably grow more options  - eg to perhaps say whether
> > > to use it in listen or connect mode. Or to indicate a reconnect
> > > timeout. etc
> >
> > Yes, that would be new capabilities symbols. That doesn't require json to me.
>
> It just reinvents the chardev unix socket syntax, but in a
> different adhoc manner, which is repeating the mistake we have
> made time & again in QEMU. Using QAPI we can directly accept
> the ChardevSocket syntax we already know about. Instead of
> having --socket-path and --fd and documenting that they are
> mutually exclusive ChardevSocket QAPI schema provides that
> representation in a well defined format which is discoverable
> and QEMU and mgmt apps already understand.

That would require external vhost-user backends to implement QAPI/json
parsing. Is this necessary for a vhost-user backend? I doubt it.

>
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Daniel P. Berrangé 5 years, 6 months ago
On Tue, Oct 16, 2018 at 02:47:12PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Oct 12, 2018 at 1:56 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Fri, Oct 12, 2018 at 01:43:39PM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Thu, Oct 11, 2018 at 7:49 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >
> > > > Adding Markus since we're talking about new CLI argument and capability
> > > > reporting standards.
> > > >
> > > > On Fri, Sep 14, 2018 at 05:52:30PM +0400, Marc-André Lureau wrote:
> > > > > +Backend program conventions
> > > > > +---------------------------
> > > > > +
> > > > > +vhost-user backends provide various services and they may need to be
> > > > > +configured manually depending on the use case. However, it is a good
> > > > > +idea to follow the conventions listed here when possible. Users, QEMU
> > > > > +or libvirt, can then rely on some common behaviour to avoid
> > > > > +heterogenous configuration and management of the backend program and
> > > > > +facilitate interoperability.
> > > > > +
> > > > > +In order to be discoverable, default vhost-user backends should be
> > > > > +located under "/usr/libexec", and be named "vhost-user-$device" where
> > > > > +"$device" is the device name in lower-case following the name listed
> > > > > +in the Linux virtio_ids.h header (ex: the VIRTIO_ID_RPROC_SERIAL
> > > > > +backend would be named "vhost-user-rproc-serial").
> > > > > +
> > > > > +Mechanisms to list, and to select among alternatives implementations
> > > > > +or modify the default backend are not described at this point (a
> > > > > +distribution may use update-alternatives, for example, to list and to
> > > > > +pick a different default backend).
> > > >
> > > > I don't think that update-alternatives is a good thing as it presumes
> > > > that each host only needs a single preferred impl at a time.
> > > >
> > > > I think we need to be able to discover all impls for a given device
> > > > type.
> > >
> > > That was left for a future improvement. I don't think it's a good idea
> > > to tackle problems we don't have yet.
> >
> > I think the need to support multiple alternatives will arrive pretty
> > much immediately, as it has already been raised in previous threads
> > about vhost-user. It is counterproductive to implement something we
> 
> At this point, there are no alternative implementation of the
> vhost-user-gpu or vhost-user-input backends (and no other device has
> been implemented to follow this spec).

We can't assume that will remain true. The very notion of the
capabilities reporting for optional features indicates that
we're expecting alternate impls with varying feature sets.

> > know is not satisfactory when we can easily provide an approach
> > which is extensible for no significant extra work upfront. Having
> > it aligned with the approach we do for firmware makes it even more
> > compelling, as we get a consistent story across QEMU instead of
> > two completely different approaches for the same concept.
> 
> That's not incompatible with doing it as a future improvement.

It is different from the POV of libvirt, as it implies two different
ways to identifying which is the preferred / default vhost. I don't
see a compelling reason to provide an initial impl that is intentionally
not flexible enough to cope with future needs.

> > > > > +The backend program must not daemonize itself, but it may be
> > > > > +daemonized by the management layer. It may also have a restricted
> > > > > +access to the system.
> > > > > +
> > > > > +File descriptors 0, 1 and 2 will exist, and have regular
> > > > > +stdin/stdout/stderr usage (they may be redirected to /dev/null by the
> > > > > +management layer, or to a log handler).
> > > > > +
> > > > > +The backend program must end (as quickly and cleanly as possible) when
> > > > > +the SIGTERM signal is received. Eventually, it may be SIGKILL by the
> > > > > +management layer after a few seconds.
> > > > > +
> > > > > +The following command line options have an expected behaviour. They
> > > > > +are mandatory, unless explicitly said differently:
> > > > > +
> > > > > +* --socket-path=PATH
> > > > > +
> > > > > +This option specify the location of the vhost-user Unix domain socket.
> > > > > +It is incompatible with --fd.
> > > > > +
> > > > > +* --fd=FDNUM
> > > > > +
> > > > > +When this argument is given, the backend program is started with the
> > > > > +vhost-user socket as file descriptor FDNUM. It is incompatible with
> > > > > +--socket-path.
> > > > > +
> > > > > +* --print-capabilities
> > > > > +
> > > > > +Output to stdout a line-seperated list of backend capabilities, and
> > > > > +then exit successfully. Other options and arguments should be ignored,
> > > > > +and the backend program should not perform its normal function.
> > > >
> > > > This is going to repeat the mistakes we've had with every other
> > > > binary in QEMU. A "simple" flag list or args sounds appealing,
> > > > but we've always been burnt by it in the medium-long term, which
> > > > is why we created QAPI.
> > >
> > > isn't libvirt using a list of strings/symbols as well for the
> > > capabilities? To me it sounds fairly easy to extend this way.
> > >
> > > > If we're doing to have any capabilities reporting, we should
> > > > model it in QAPI schema, so any '--print-capabilities' arg
> > > > should print a JSON doc following the documented schema.
> > >
> > > perhaps we could have --print-json-capabilities later, if needed.
> >
> > A QAPI schema can still start off as reporting a simple list of
> > features flags.
> > The key point is to provide a data format that is extensible
> > right from the start. For QEMU that means QAPI, instead of
> > inventing yet another custom data format, that we'd have to
> > fix later when we find a flat list of strings is insufficient.
> 
> 
> Changing --print-capabilites from:
> feature-a
> feature-b
> 
> to a json form:
> {
>   "features": [
>     "feature-a",
>     "feature-b"
>   ]
> }
> 
> Would that be extensible enough?

As long as the JSON format is associated with a QAPI schema so we
have a clear specification that we're following as illustrated in
this example

  https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02459.html

> > > > While talking about QAPI, I think this is an opportunity to
> > > > also avoid the problems of CLI arg values becoming more
> > > > complex than just scalars. eg
> > > >
> > > >   --socket-path=PATH
> > > >
> > > > may inevitably grow more options  - eg to perhaps say whether
> > > > to use it in listen or connect mode. Or to indicate a reconnect
> > > > timeout. etc
> > >
> > > Yes, that would be new capabilities symbols. That doesn't require json to me.
> >
> > It just reinvents the chardev unix socket syntax, but in a
> > different adhoc manner, which is repeating the mistake we have
> > made time & again in QEMU. Using QAPI we can directly accept
> > the ChardevSocket syntax we already know about. Instead of
> > having --socket-path and --fd and documenting that they are
> > mutually exclusive ChardevSocket QAPI schema provides that
> > representation in a well defined format which is discoverable
> > and QEMU and mgmt apps already understand.
> 
> That would require external vhost-user backends to implement QAPI/json
> parsing. Is this necessary for a vhost-user backend? I doubt it.

They could, but would not be required, to implement QAPI/json parser.

The QAPI schema defines a standard for how to model & interpret the
non-scalar values for command line arguments. An external impl would
need to ensure that whatever parsing it does for CLI args is semantically
compatible with the parsing rules defined by the QEMU QAPI schema we
define to ensure interoperability of its impl.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Marc-André Lureau 5 years, 5 months ago
Hi

On Tue, Oct 16, 2018 at 4:14 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > It just reinvents the chardev unix socket syntax, but in a
> > > different adhoc manner, which is repeating the mistake we have
> > > made time & again in QEMU. Using QAPI we can directly accept
> > > the ChardevSocket syntax we already know about. Instead of
> > > having --socket-path and --fd and documenting that they are
> > > mutually exclusive ChardevSocket QAPI schema provides that
> > > representation in a well defined format which is discoverable
> > > and QEMU and mgmt apps already understand.
> >
> > That would require external vhost-user backends to implement QAPI/json
> > parsing. Is this necessary for a vhost-user backend? I doubt it.
>
> They could, but would not be required, to implement QAPI/json parser.
>
> The QAPI schema defines a standard for how to model & interpret the
> non-scalar values for command line arguments. An external impl would
> need to ensure that whatever parsing it does for CLI args is semantically
> compatible with the parsing rules defined by the QEMU QAPI schema we
> define to ensure interoperability of its impl.

So you would want to have something like?

--chardev '{ "id" : "bar", "backend" : { "type" : "socket", "data" : {
"addr" : { "type": "unix", "path": "/tmp/foo.sock" }, "server":
"false"  } } }'

instead of:

--socket-path=/tmp/foo.sock

I don't really get what that will help with, for the vhost-user backend case...

>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Daniel P. Berrangé 5 years, 5 months ago
On Wed, Oct 31, 2018 at 07:44:36PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Oct 16, 2018 at 4:14 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > It just reinvents the chardev unix socket syntax, but in a
> > > > different adhoc manner, which is repeating the mistake we have
> > > > made time & again in QEMU. Using QAPI we can directly accept
> > > > the ChardevSocket syntax we already know about. Instead of
> > > > having --socket-path and --fd and documenting that they are
> > > > mutually exclusive ChardevSocket QAPI schema provides that
> > > > representation in a well defined format which is discoverable
> > > > and QEMU and mgmt apps already understand.
> > >
> > > That would require external vhost-user backends to implement QAPI/json
> > > parsing. Is this necessary for a vhost-user backend? I doubt it.
> >
> > They could, but would not be required, to implement QAPI/json parser.
> >
> > The QAPI schema defines a standard for how to model & interpret the
> > non-scalar values for command line arguments. An external impl would
> > need to ensure that whatever parsing it does for CLI args is semantically
> > compatible with the parsing rules defined by the QEMU QAPI schema we
> > define to ensure interoperability of its impl.
> 
> So you would want to have something like?
> 
> --chardev '{ "id" : "bar", "backend" : { "type" : "socket", "data" : {
> "addr" : { "type": "unix", "path": "/tmp/foo.sock" }, "server":
> "false"  } } }'

I wasn't specificially suggesting json syntax. Just the flattened
dot separate syntax, whose valid components are mapped to corresponding
qapi schema defintions. eg closer to what we have already today

  --chardev socket,id=bar,path=/tmp/foo.sock,server

>
> instead of:
> 
> --socket-path=/tmp/foo.sock
> 
> I don't really get what that will help with, for the vhost-user backend case...

It avoids presuming that we forever want to launch the backend with
a single socket path and nothing else. Using the chardev, means we
can choosen between client/server mode. We can choose whether to
pass an FD to the socket, instead of the socket path. Whether the
reconnect flag is set or not, and all the other aspects of a chardev
you can define.

I don't think we should have to add more & more adhoc CLI args (--socket-path,
--fd, --reconnect, etc) and then manually parse them & pack their values
together into a chardev object.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Qemu-devel] [PATCH v2] vhost-user: define conventions for vhost-user backends
Posted by Marc-André Lureau 5 years, 5 months ago
Hi

On Wed, Oct 31, 2018 at 8:05 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Oct 31, 2018 at 07:44:36PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Tue, Oct 16, 2018 at 4:14 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > > It just reinvents the chardev unix socket syntax, but in a
> > > > > different adhoc manner, which is repeating the mistake we have
> > > > > made time & again in QEMU. Using QAPI we can directly accept
> > > > > the ChardevSocket syntax we already know about. Instead of
> > > > > having --socket-path and --fd and documenting that they are
> > > > > mutually exclusive ChardevSocket QAPI schema provides that
> > > > > representation in a well defined format which is discoverable
> > > > > and QEMU and mgmt apps already understand.
> > > >
> > > > That would require external vhost-user backends to implement QAPI/json
> > > > parsing. Is this necessary for a vhost-user backend? I doubt it.
> > >
> > > They could, but would not be required, to implement QAPI/json parser.
> > >
> > > The QAPI schema defines a standard for how to model & interpret the
> > > non-scalar values for command line arguments. An external impl would
> > > need to ensure that whatever parsing it does for CLI args is semantically
> > > compatible with the parsing rules defined by the QEMU QAPI schema we
> > > define to ensure interoperability of its impl.
> >
> > So you would want to have something like?
> >
> > --chardev '{ "id" : "bar", "backend" : { "type" : "socket", "data" : {
> > "addr" : { "type": "unix", "path": "/tmp/foo.sock" }, "server":
> > "false"  } } }'
>
> I wasn't specificially suggesting json syntax. Just the flattened
> dot separate syntax, whose valid components are mapped to corresponding
> qapi schema defintions. eg closer to what we have already today
>
>   --chardev socket,id=bar,path=/tmp/foo.sock,server
>

ok

> >
> > instead of:
> >
> > --socket-path=/tmp/foo.sock
> >
> > I don't really get what that will help with, for the vhost-user backend case...
>
> It avoids presuming that we forever want to launch the backend with
> a single socket path and nothing else. Using the chardev, means we
> can choosen between client/server mode. We can choose whether to
> pass an FD to the socket, instead of the socket path. Whether the
> reconnect flag is set or not, and all the other aspects of a chardev
> you can define.
>

We are trying to define a common specification for vhost-user
backends. Where do we stop defining what --chardev should support?

> I don't think we should have to add more & more adhoc CLI args (--socket-path,
> --fd, --reconnect, etc) and then manually parse them & pack their values
> together into a chardev object.

The backends most likely won't use qemu chardev (nor qapi), and be
limited to unix socket.


>
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|