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.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 MAINTAINERS                  |   1 +
 docs/interop/vhost-user.json | 219 +++++++++++++++++++++++++++++++++++
 docs/interop/vhost-user.txt  | 101 +++++++++++++++-
 3 files changed, 319 insertions(+), 2 deletions(-)
 create mode 100644 docs/interop/vhost-user.json

diff --git a/MAINTAINERS b/MAINTAINERS
index 1032406c56..81e4368a43 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1335,6 +1335,7 @@ vhost
 M: Michael S. Tsirkin <mst@redhat.com>
 S: Supported
 F: hw/*/*vhost*
+F: docs/interop/vhost-user.json
 F: docs/interop/vhost-user.txt
 
 virtio
diff --git a/docs/interop/vhost-user.json b/docs/interop/vhost-user.json
new file mode 100644
index 0000000000..91b5bf499e
--- /dev/null
+++ b/docs/interop/vhost-user.json
@@ -0,0 +1,219 @@
+# -*- Mode: Python -*-
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# Authors:
+#  Marc-André Lureau <marcandre.lureau@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+##
+# = vhost user backend discovery & capabilities
+##
+
+##
+# @VHostUserBackendType:
+#
+# List the various vhost user backend types.
+#
+# @net: virtio net
+# @block: virtio block
+# @console: virtio console
+# @rng: virtio rng
+# @balloon: virtio balloon
+# @rpmsg: virtio remote processor messaging
+# @scsi: virtio scsi
+# @9p: 9p virtio console
+# @rproc-serial: virtio remoteproc serial link
+# @caif: virtio caif
+# @gpu: virtio gpu
+# @input: virtio input
+# @vsock: virtio vsock transport
+# @crypto: virtio crypto
+#
+# Since: 3.2
+##
+{
+  'enum': 'VHostUserBackendType',
+  'data': [ 'net', 'block', 'console', 'rng', 'balloon', 'rpmsg',
+            'scsi', '9p', 'rproc-serial', 'caif', 'gpu', 'input', 'vsock',
+            'crypto' ]
+}
+
+##
+# @VHostUserBackendInputFeature:
+#
+# List of vhost user "input" features.
+#
+# @evdev-path: The --evdev-path command line option is supported.
+# @no-grab: The --no-grab command line option is supported.
+#
+# Since: 3.2
+##
+{
+  'enum': 'VHostUserBackendInputFeature',
+  'data': [ 'evdev-path', 'no-grab' ]
+}
+
+##
+# @VHostUserBackendCapabilitiesInput:
+#
+# Capabilities reported by vhost user "input" backends
+#
+# @features: list of supported features.
+#
+# Since: 3.2
+##
+{
+  'struct': 'VHostUserBackendCapabilitiesInput',
+  'data': {
+    'features': [ 'VHostUserBackendInputFeature' ]
+  }
+}
+
+##
+# @VHostUserBackendGPUFeature:
+#
+# List of vhost user "gpu" features.
+#
+# @render-node: The --render-node command line option is supported.
+# @virgl: The --virgl command line option is supported.
+#
+# Since: 3.2
+##
+{
+  'enum': 'VHostUserBackendGPUFeature',
+  'data': [ 'render-node', 'virgl' ]
+}
+
+##
+# @VHostUserBackendCapabilitiesGPU:
+#
+# Capabilities reported by vhost user "gpu" backends.
+#
+# @features: list of supported features.
+#
+# Since: 3.2
+##
+{
+  'struct': 'VHostUserBackendCapabilitiesGPU',
+  'data': {
+    'features': [ 'VHostUserBackendGPUFeature' ]
+  }
+}
+
+##
+# @VHostUserBackendCapabilities:
+#
+# Capabilities reported by vhost user backends.
+#
+# @type: The vhost user backend type.
+#
+# Since: 3.2
+##
+{
+  'union': 'VHostUserBackendCapabilities',
+  'base': { 'type': 'VHostUserBackendType' },
+  'discriminator': 'type',
+  'data': {
+    'input': 'VHostUserBackendCapabilitiesInput',
+    'gpu': 'VHostUserBackendCapabilitiesGPU'
+  }
+}
+
+##
+# @VhostUserBackend:
+#
+# Describes a vhost user backend to management software.
+#
+# It is possible for multiple @VhostUserBackend elements to match the
+# search criteria of management software. Applications thus need rules
+# to pick one of the many matches, and users need the ability to
+# override distro defaults.
+#
+# It is recommended to create vhost user backend JSON files (each
+# containing a single @VhostUserBackend root element) with a
+# double-digit prefix, for example "50-qemu-gpu.json",
+# "50-crosvm-gpu.json", etc, so they can be sorted in predictable
+# order. The backend JSON files should be searched for in three
+# directories:
+#
+#   - /usr/share/qemu/vhost-user -- populated by distro-provided
+#                                   packages (XDG_DATA_DIRS covers
+#                                   /usr/share by default),
+#
+#   - /etc/qemu/vhost-user -- exclusively for sysadmins' local additions,
+#
+#   - $XDG_CONFIG_HOME/qemu/vhost-user -- exclusively for per-user local
+#                                         additions (XDG_CONFIG_HOME
+#                                         defaults to $HOME/.config).
+#
+# Top-down, the list of directories goes from general to specific.
+#
+# Management software should build a list of files from all three
+# locations, then sort the list by filename (i.e., last pathname
+# component). Management software should choose the first JSON file on
+# the sorted list that matches the search criteria. If a more specific
+# directory has a file with same name as a less specific directory, then
+# the file in the more specific directory takes effect. If the more
+# specific file is zero length, it hides the less specific one.
+#
+# For example, if a distro ships
+#
+#   - /usr/share/qemu/vhost-user/50-qemu-gpu.json
+#
+#   - /usr/share/qemu/vhost-user/50-crosvm-gpu.json
+#
+# then the sysadmin can prevent the default QEMU being used at all with
+#
+#   $ touch /etc/qemu/vhost-user/50-qemu-gpu.json
+#
+# The sysadmin can replace/alter the distro default OVMF with
+#
+#   $ vim /etc/qemu/vhost-user/50-qemu-gpu.json
+#
+# or they can provide a parallel QEMU GPU with higher priority
+#
+#   $ vim /etc/qemu/vhost-user/10-qemu-gpu.json
+#
+# or they can provide a parallel OVMF with lower priority
+#
+#   $ vim /etc/qemu/vhost-user/99-qemu-gpu.json
+#
+# @type: The vhost user backend type.
+#
+# @description: Provides a human-readable description of the backend.
+#               Management software may or may not display @description.
+#
+# @binary: Absolute path to the backend binary.
+#
+# @tags: An optional list of auxiliary strings associated with the
+#        backend for which @description is not appropriate, due to the
+#        latter's possible exposure to the end-user. @tags serves
+#        development and debugging purposes only, and management
+#        software shall explicitly ignore it.
+#
+# Since: 3.2
+#
+# Example:
+#
+# {
+#   "description": "QEMU vhost-user-gpu",
+#   "type": "gpu",
+#   "binary": "/usr/libexec/qemu/vhost-user-gpu",
+#   "tags": [
+#     "CONFIG_OPENGL_DMABUF=y"
+#   ]
+# }
+#
+##
+{
+  'struct' : 'VhostUserBackend',
+  'data'   : {
+    'description': 'str',
+    'type': 'VHostUserBackendType',
+    'binary': 'str',
+    '*tags': [ 'str' ]
+  }
+}
diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index c2194711d9..cefec9ffe1 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.
@@ -835,3 +840,95 @@ 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 can provide various devices & services and 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 programs and facilitate interoperability.
+
+Each backend installed on a host system should come with at least one
+JSON file that conforms to the vhost-user.json schema. Each file
+informs the management applications about the backend type, and binary
+location. In addition, it defines rules for management apps for
+picking the highest priority backend when multiple match the search
+criteria (see @VhostUserBackend documentation in the schema file).
+
+If the backend is not capable of enabling a requested feature on the
+host (such as 3D acceleration with virgl), or the initialization
+failed, the backend should fail to start early and exit with a status
+!= 0. It may also print a message to stderr for further details.
+
+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 have been 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 the backend capabilities in JSON format, and then
+exit successfully. Other options and arguments should be ignored, and
+the backend program should not perform its normal function.  The
+capabilities can be reported dynamically depending on the host
+capabilities.
+
+The JSON output is described in the vhost-user.json schema, by
+@VHostUserBackendCapabilities.  Example:
+{
+  "type": "foo",
+  "features": [
+    "feature-a",
+    "feature-b"
+  ]
+}
+
+vhost-user-input
+----------------
+
+Command line options:
+
+* --evdev-path=PATH (optional)
+
+Specify the linux input device.
+
+* --no-grab (optional)
+
+Do no request exclusive access to the input device.
+
+vhost-user-gpu
+--------------
+
+Command line options:
+
+* --render-node=PATH (optional)
+
+Specify the GPU DRM render node.
+
+* --virgl (optional)
+
+Enable virgl rendering support.
-- 
2.20.0.rc1

On 11/26/18 6:42 AM, 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.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   MAINTAINERS                  |   1 +
>   docs/interop/vhost-user.json | 219 +++++++++++++++++++++++++++++++++++
>   docs/interop/vhost-user.txt  | 101 +++++++++++++++-
>   3 files changed, 319 insertions(+), 2 deletions(-)
>   create mode 100644 docs/interop/vhost-user.json

> +++ b/docs/interop/vhost-user.json

> +# @VHostUserBackendType:
> +#
> +# List the various vhost user backend types.
> +#
> +# @net: virtio net
> +# @block: virtio block
> +# @console: virtio console
> +# @rng: virtio rng
> +# @balloon: virtio balloon
> +# @rpmsg: virtio remote processor messaging
> +# @scsi: virtio scsi
> +# @9p: 9p virtio console
> +# @rproc-serial: virtio remoteproc serial link
> +# @caif: virtio caif
> +# @gpu: virtio gpu
> +# @input: virtio input
> +# @vsock: virtio vsock transport
> +# @crypto: virtio crypto
> +#
> +# Since: 3.2

The next release will be 4.0, not 3.2. (We'll probably have to do a 
global search-and-replace in January, as you're not the only one 
proposing a patch with this version number).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Mon, Nov 26, 2018 at 04:42:40PM +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.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

btw: have you seen the idea to use a vfio-style interface for
communication between qemu and external device emulation processes?

cheers,
  Gerd

Hi

On Mon, Dec 10, 2018 at 6:30 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Mon, Nov 26, 2018 at 04:42:40PM +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.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>
> btw: have you seen the idea to use a vfio-style interface for
> communication between qemu and external device emulation processes?

I heard there was some discussion during KVM forum.

Fwiwi, this is also an idea I proposed last year (and quickly
discussed during my talk about multi-process qemu).
I also experimented with the idea, and wrote a vfio-user backend, with
a PCI serial device running in a seperate process:
the qemu tree: https://github.com/elmarco/qemu/tree/wip/vfio-user (dirty tree)
and the serial device:
https://github.com/elmarco/qemu/blob/wip/vfio-user/contrib/libvfio-user/vfio-user-serial.c



>
> cheers,
>   Gerd
>

On Mon, Dec 10, 2018 at 10:36:29PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Dec 10, 2018 at 6:30 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Mon, Nov 26, 2018 at 04:42:40PM +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.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> >
> > Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > btw: have you seen the idea to use a vfio-style interface for
> > communication between qemu and external device emulation processes?
> 
> I heard there was some discussion during KVM forum.
> 
> Fwiwi, this is also an idea I proposed last year (and quickly
> discussed during my talk about multi-process qemu).
> I also experimented with the idea, and wrote a vfio-user backend, with
> a PCI serial device running in a seperate process:
> the qemu tree: https://github.com/elmarco/qemu/tree/wip/vfio-user (dirty tree)
> and the serial device:
> https://github.com/elmarco/qemu/blob/wip/vfio-user/contrib/libvfio-user/vfio-user-serial.c

Right. The main issue is that we need to make sure only
in-tree devices are supported. vhost-user by design
is for out of tree users. It needn't be hard,
maybe it's enough to just make qemu launch these processes
as opposed to connecting to them on command line.

> 
> 
> >
> > cheers,
> >   Gerd
> >

  Hi,

> Right. The main issue is that we need to make sure only
> in-tree devices are supported.

Well, that is under debate right now, see:
https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html

> vhost-user by design
> is for out of tree users. It needn't be hard,
> maybe it's enough to just make qemu launch these processes
> as opposed to connecting to them on command line.

Not sure this is a good idea, with security being one of the motivating
factors to move device emulation to other processes.  When libvirt
launches the processes it can place them in separate sandboxes ...

cheers,
  Gerd

On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
>   Hi,
> 
> > Right. The main issue is that we need to make sure only
> > in-tree devices are supported.
> 
> Well, that is under debate right now, see:
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html

I've previously been against the idea of external plugins for QEMU,
however, that was when the plugin was something that would be dlopen'd
by QEMU. That would cause our internal ABI to be exposed to 3rd parties
which is highly undesirable, even if they were open source to comply
with the license needs.

When the plugin is a completely isolated process communicating with a
well defined protocol, it is not placing a significant burden on the
QEMU developers' ongoing maintainence, nor has problems with license
compliance. The main problem would come from debugging the combined
system as the external process is essentially a black box from QEMU's
POV. Downstream OS vendors are free to place restrictions on which
backend processes they'd be willing to support with QEMU, and upstream
is under no obligation to debug stuff beyond the QEMU boundary.

We have already accepted that tradeoff with networking by supporting
vhost-user and have externals impls like DPDK, so I don't see a
compelling reason to try to restrict it for other vhost-user backends.

> > vhost-user by design
> > is for out of tree users. It needn't be hard,
> > maybe it's enough to just make qemu launch these processes
> > as opposed to connecting to them on command line.
> 
> Not sure this is a good idea, with security being one of the motivating
> factors to move device emulation to other processes.  When libvirt
> launches the processes it can place them in separate sandboxes ...

Yep, libvirt already turns on seccomp policies which forbid QEMU from
forking/execing anything, and we have no desire to go backwards here.
Any external processes have to be launched by libvirt ahead of time.


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 :|

On Tue, Dec 11, 2018 at 09:29:44AM +0000, Daniel P. Berrangé wrote:
> On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
> >   Hi,
> > 
> > > Right. The main issue is that we need to make sure only
> > > in-tree devices are supported.
> > 
> > Well, that is under debate right now, see:
> > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html
> 
> I've previously been against the idea of external plugins for QEMU,
> however, that was when the plugin was something that would be dlopen'd
> by QEMU. That would cause our internal ABI to be exposed to 3rd parties
> which is highly undesirable, even if they were open source to comply
> with the license needs.
> 
> When the plugin is a completely isolated process communicating with a
> well defined protocol, it is not placing a significant burden on the
> QEMU developers' ongoing maintainence, nor has problems with license
> compliance. The main problem would come from debugging the combined
> system as the external process is essentially a black box from QEMU's
> POV. Downstream OS vendors are free to place restrictions on which
> backend processes they'd be willing to support with QEMU, and upstream
> is under no obligation to debug stuff beyond the QEMU boundary.
> 
> We have already accepted that tradeoff with networking by supporting
> vhost-user and have externals impls like DPDK, so I don't see a
> compelling reason to try to restrict it for other vhost-user backends.

OK seems to be more or less a rough concensus then.

I wonder what's the approach wrt migration though.

Even the compatibility story about vhost-user isn't
great, I would like to see something solid before
we allow that.

Are we happy to just block live migration?
For sure that's already the case with VFIO.


> > > vhost-user by design
> > > is for out of tree users. It needn't be hard,
> > > maybe it's enough to just make qemu launch these processes
> > > as opposed to connecting to them on command line.
> > 
> > Not sure this is a good idea, with security being one of the motivating
> > factors to move device emulation to other processes.  When libvirt
> > launches the processes it can place them in separate sandboxes ...
> 
> Yep, libvirt already turns on seccomp policies which forbid QEMU from
> forking/execing anything, and we have no desire to go backwards here.
> Any external processes have to be launched by libvirt ahead of time.
> 
> 
> 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 :|

Hi

On Tue, Dec 11, 2018 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Dec 11, 2018 at 09:29:44AM +0000, Daniel P. Berrangé wrote:
> > On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
> > >   Hi,
> > >
> > > > Right. The main issue is that we need to make sure only
> > > > in-tree devices are supported.
> > >
> > > Well, that is under debate right now, see:
> > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html
> >
> > I've previously been against the idea of external plugins for QEMU,
> > however, that was when the plugin was something that would be dlopen'd
> > by QEMU. That would cause our internal ABI to be exposed to 3rd parties
> > which is highly undesirable, even if they were open source to comply
> > with the license needs.
> >
> > When the plugin is a completely isolated process communicating with a
> > well defined protocol, it is not placing a significant burden on the
> > QEMU developers' ongoing maintainence, nor has problems with license
> > compliance. The main problem would come from debugging the combined
> > system as the external process is essentially a black box from QEMU's
> > POV. Downstream OS vendors are free to place restrictions on which
> > backend processes they'd be willing to support with QEMU, and upstream
> > is under no obligation to debug stuff beyond the QEMU boundary.
> >
> > We have already accepted that tradeoff with networking by supporting
> > vhost-user and have externals impls like DPDK, so I don't see a
> > compelling reason to try to restrict it for other vhost-user backends.
>
> OK seems to be more or less a rough concensus then.
>
> I wonder what's the approach wrt migration though.

The series doesn't take care of migration.

>
> Even the compatibility story about vhost-user isn't
> great, I would like to see something solid before
> we allow that.

To allow migration? vhost-user has partial support for migration
(dirty memory tracking), and there is also "[PATCH v2 for-4.0 0/7]
vhost-user-blk: Add support for backend reconnecting" - allowing the
backend to store some state, if I understand correctly, which could be
leveraged I guess...

But I don't think we should block this series because migration isn't
tackled here.

thanks


.

>
> Are we happy to just block live migration?
> For sure that's already the case with VFIO.
>
>
> > > > vhost-user by design
> > > > is for out of tree users. It needn't be hard,
> > > > maybe it's enough to just make qemu launch these processes
> > > > as opposed to connecting to them on command line.
> > >
> > > Not sure this is a good idea, with security being one of the motivating
> > > factors to move device emulation to other processes.  When libvirt
> > > launches the processes it can place them in separate sandboxes ...
> >
> > Yep, libvirt already turns on seccomp policies which forbid QEMU from
> > forking/execing anything, and we have no desire to go backwards here.
> > Any external processes have to be launched by libvirt ahead of time.
> >
> >
> > 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 :|
>


-- 
Marc-André Lureau

On Tue, Dec 18, 2018 at 10:35:05PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Dec 11, 2018 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Dec 11, 2018 at 09:29:44AM +0000, Daniel P. Berrangé wrote:
> > > On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
> > > >   Hi,
> > > >
> > > > > Right. The main issue is that we need to make sure only
> > > > > in-tree devices are supported.
> > > >
> > > > Well, that is under debate right now, see:
> > > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html
> > >
> > > I've previously been against the idea of external plugins for QEMU,
> > > however, that was when the plugin was something that would be dlopen'd
> > > by QEMU. That would cause our internal ABI to be exposed to 3rd parties
> > > which is highly undesirable, even if they were open source to comply
> > > with the license needs.
> > >
> > > When the plugin is a completely isolated process communicating with a
> > > well defined protocol, it is not placing a significant burden on the
> > > QEMU developers' ongoing maintainence, nor has problems with license
> > > compliance. The main problem would come from debugging the combined
> > > system as the external process is essentially a black box from QEMU's
> > > POV. Downstream OS vendors are free to place restrictions on which
> > > backend processes they'd be willing to support with QEMU, and upstream
> > > is under no obligation to debug stuff beyond the QEMU boundary.
> > >
> > > We have already accepted that tradeoff with networking by supporting
> > > vhost-user and have externals impls like DPDK, so I don't see a
> > > compelling reason to try to restrict it for other vhost-user backends.
> >
> > OK seems to be more or less a rough concensus then.
> >
> > I wonder what's the approach wrt migration though.
> 
> The series doesn't take care of migration.
> 
> >
> > Even the compatibility story about vhost-user isn't
> > great, I would like to see something solid before
> > we allow that.
> 
> To allow migration? vhost-user has partial support for migration
> (dirty memory tracking), and there is also "[PATCH v2 for-4.0 0/7]
> vhost-user-blk: Add support for backend reconnecting" - allowing the
> backend to store some state, if I understand correctly, which could be
> leveraged I guess...
> 
> But I don't think we should block this series because migration isn't
> tackled here.
> 
> thanks
> 
> 
> .

How about blocking migration for now then?

We need someone to work on a solution for cross version/device
compatibility, vhost net/blk without that is bad enough but at least we
have a feature bits, for random devices it would be very very bad.


> >
> > Are we happy to just block live migration?
> > For sure that's already the case with VFIO.
> >
> >
> > > > > vhost-user by design
> > > > > is for out of tree users. It needn't be hard,
> > > > > maybe it's enough to just make qemu launch these processes
> > > > > as opposed to connecting to them on command line.
> > > >
> > > > Not sure this is a good idea, with security being one of the motivating
> > > > factors to move device emulation to other processes.  When libvirt
> > > > launches the processes it can place them in separate sandboxes ...
> > >
> > > Yep, libvirt already turns on seccomp policies which forbid QEMU from
> > > forking/execing anything, and we have no desire to go backwards here.
> > > Any external processes have to be launched by libvirt ahead of time.
> > >
> > >
> > > 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 :|
> >
> 
> 
> -- 
> Marc-André Lureau

Hi

On Wed, Dec 19, 2018 at 3:20 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Dec 18, 2018 at 10:35:05PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Tue, Dec 11, 2018 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Tue, Dec 11, 2018 at 09:29:44AM +0000, Daniel P. Berrangé wrote:
> > > > On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
> > > > >   Hi,
> > > > >
> > > > > > Right. The main issue is that we need to make sure only
> > > > > > in-tree devices are supported.
> > > > >
> > > > > Well, that is under debate right now, see:
> > > > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html
> > > >
> > > > I've previously been against the idea of external plugins for QEMU,
> > > > however, that was when the plugin was something that would be dlopen'd
> > > > by QEMU. That would cause our internal ABI to be exposed to 3rd parties
> > > > which is highly undesirable, even if they were open source to comply
> > > > with the license needs.
> > > >
> > > > When the plugin is a completely isolated process communicating with a
> > > > well defined protocol, it is not placing a significant burden on the
> > > > QEMU developers' ongoing maintainence, nor has problems with license
> > > > compliance. The main problem would come from debugging the combined
> > > > system as the external process is essentially a black box from QEMU's
> > > > POV. Downstream OS vendors are free to place restrictions on which
> > > > backend processes they'd be willing to support with QEMU, and upstream
> > > > is under no obligation to debug stuff beyond the QEMU boundary.
> > > >
> > > > We have already accepted that tradeoff with networking by supporting
> > > > vhost-user and have externals impls like DPDK, so I don't see a
> > > > compelling reason to try to restrict it for other vhost-user backends.
> > >
> > > OK seems to be more or less a rough concensus then.
> > >
> > > I wonder what's the approach wrt migration though.
> >
> > The series doesn't take care of migration.
> >
> > >
> > > Even the compatibility story about vhost-user isn't
> > > great, I would like to see something solid before
> > > we allow that.
> >
> > To allow migration? vhost-user has partial support for migration
> > (dirty memory tracking), and there is also "[PATCH v2 for-4.0 0/7]
> > vhost-user-blk: Add support for backend reconnecting" - allowing the
> > backend to store some state, if I understand correctly, which could be
> > leveraged I guess...
> >
> > But I don't think we should block this series because migration isn't
> > tackled here.
> >
> > thanks
> >
> >
> > .
>
> How about blocking migration for now then?

The device here (vhost-user-input) blocks migration (unmigratable = 1)

>
> We need someone to work on a solution for cross version/device
> compatibility, vhost net/blk without that is bad enough but at least we
> have a feature bits, for random devices it would be very very bad.

For now, if migration is somehow supported, it must be handled mostly
by the qemu device, and the vhost-user backend must track dirty memory
and be "stateless": *able to reconnect & resume where it left off).

A backend shouldn't declare VHOST_USER_PROTOCOL_F_LOG_SHMFD if it can't do that.

When a backend will have a state to migrate, we will have to implement
a new feature. I assume that's what you are asking.

>
>
> > >
> > > Are we happy to just block live migration?
> > > For sure that's already the case with VFIO.
> > >
> > >
> > > > > > vhost-user by design
> > > > > > is for out of tree users. It needn't be hard,
> > > > > > maybe it's enough to just make qemu launch these processes
> > > > > > as opposed to connecting to them on command line.
> > > > >
> > > > > Not sure this is a good idea, with security being one of the motivating
> > > > > factors to move device emulation to other processes.  When libvirt
> > > > > launches the processes it can place them in separate sandboxes ...
> > > >
> > > > Yep, libvirt already turns on seccomp policies which forbid QEMU from
> > > > forking/execing anything, and we have no desire to go backwards here.
> > > > Any external processes have to be launched by libvirt ahead of time.
> > > >
> > > >
> > > > 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 :|
> > >
> >
> >
> > --
> > Marc-André Lureau



-- 
Marc-André Lureau

On Wed, Dec 19, 2018 at 12:01:59PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Dec 19, 2018 at 3:20 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Dec 18, 2018 at 10:35:05PM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Tue, Dec 11, 2018 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Tue, Dec 11, 2018 at 09:29:44AM +0000, Daniel P. Berrangé wrote:
> > > > > On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
> > > > > >   Hi,
> > > > > >
> > > > > > > Right. The main issue is that we need to make sure only
> > > > > > > in-tree devices are supported.
> > > > > >
> > > > > > Well, that is under debate right now, see:
> > > > > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html
> > > > >
> > > > > I've previously been against the idea of external plugins for QEMU,
> > > > > however, that was when the plugin was something that would be dlopen'd
> > > > > by QEMU. That would cause our internal ABI to be exposed to 3rd parties
> > > > > which is highly undesirable, even if they were open source to comply
> > > > > with the license needs.
> > > > >
> > > > > When the plugin is a completely isolated process communicating with a
> > > > > well defined protocol, it is not placing a significant burden on the
> > > > > QEMU developers' ongoing maintainence, nor has problems with license
> > > > > compliance. The main problem would come from debugging the combined
> > > > > system as the external process is essentially a black box from QEMU's
> > > > > POV. Downstream OS vendors are free to place restrictions on which
> > > > > backend processes they'd be willing to support with QEMU, and upstream
> > > > > is under no obligation to debug stuff beyond the QEMU boundary.
> > > > >
> > > > > We have already accepted that tradeoff with networking by supporting
> > > > > vhost-user and have externals impls like DPDK, so I don't see a
> > > > > compelling reason to try to restrict it for other vhost-user backends.
> > > >
> > > > OK seems to be more or less a rough concensus then.
> > > >
> > > > I wonder what's the approach wrt migration though.
> > >
> > > The series doesn't take care of migration.
> > >
> > > >
> > > > Even the compatibility story about vhost-user isn't
> > > > great, I would like to see something solid before
> > > > we allow that.
> > >
> > > To allow migration? vhost-user has partial support for migration
> > > (dirty memory tracking), and there is also "[PATCH v2 for-4.0 0/7]
> > > vhost-user-blk: Add support for backend reconnecting" - allowing the
> > > backend to store some state, if I understand correctly, which could be
> > > leveraged I guess...
> > >
> > > But I don't think we should block this series because migration isn't
> > > tackled here.
> > >
> > > thanks
> > >
> > >
> > > .
> >
> > How about blocking migration for now then?
> 
> The device here (vhost-user-input) blocks migration (unmigratable = 1)

Right. But that device is just an excersize, right?
It bothers me that next device might not remember and
we will get a mess.
Could we make it somehow that if there is no vmsd
then migration is blocked?


> >
> > We need someone to work on a solution for cross version/device
> > compatibility, vhost net/blk without that is bad enough but at least we
> > have a feature bits, for random devices it would be very very bad.
> 
> For now, if migration is somehow supported, it must be handled mostly
> by the qemu device, and the vhost-user backend must track dirty memory
> and be "stateless": *able to reconnect & resume where it left off).
> 
> A backend shouldn't declare VHOST_USER_PROTOCOL_F_LOG_SHMFD if it can't do that.
> 
> When a backend will have a state to migrate, we will have to implement
> a new feature. I assume that's what you are asking.

No, I am talking about a cross version/backend migration.

And it's such a boutique problem backends do not even
understand what it's about. A way to handle it
in the frontend is needed.

We also need a way to make sure incorrect backends do not
connect to incorrect devices, and we need a mechanism
that allows cross-version migration compatibility.

There's been a long discussion about cross-version
migration for vhost-user and I think Maxime had
a plan there. Maxime could you repost the
set of issues and what the plan is maybe?


> >
> >
> > > >
> > > > Are we happy to just block live migration?
> > > > For sure that's already the case with VFIO.
> > > >
> > > >
> > > > > > > vhost-user by design
> > > > > > > is for out of tree users. It needn't be hard,
> > > > > > > maybe it's enough to just make qemu launch these processes
> > > > > > > as opposed to connecting to them on command line.
> > > > > >
> > > > > > Not sure this is a good idea, with security being one of the motivating
> > > > > > factors to move device emulation to other processes.  When libvirt
> > > > > > launches the processes it can place them in separate sandboxes ...
> > > > >
> > > > > Yep, libvirt already turns on seccomp policies which forbid QEMU from
> > > > > forking/execing anything, and we have no desire to go backwards here.
> > > > > Any external processes have to be launched by libvirt ahead of time.
> > > > >
> > > > >
> > > > > 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 :|
> > > >
> > >
> > >
> > > --
> > > Marc-André Lureau
> 
> 
> 
> -- 
> Marc-André Lureau

Hi

On Wed, Dec 19, 2018 at 7:42 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Dec 19, 2018 at 12:01:59PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Dec 19, 2018 at 3:20 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Tue, Dec 18, 2018 at 10:35:05PM +0400, Marc-André Lureau wrote:
> > > > Hi
> > > >
> > > > On Tue, Dec 11, 2018 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > >
> > > > > On Tue, Dec 11, 2018 at 09:29:44AM +0000, Daniel P. Berrangé wrote:
> > > > > > On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
> > > > > > >   Hi,
> > > > > > >
> > > > > > > > Right. The main issue is that we need to make sure only
> > > > > > > > in-tree devices are supported.
> > > > > > >
> > > > > > > Well, that is under debate right now, see:
> > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html
> > > > > >
> > > > > > I've previously been against the idea of external plugins for QEMU,
> > > > > > however, that was when the plugin was something that would be dlopen'd
> > > > > > by QEMU. That would cause our internal ABI to be exposed to 3rd parties
> > > > > > which is highly undesirable, even if they were open source to comply
> > > > > > with the license needs.
> > > > > >
> > > > > > When the plugin is a completely isolated process communicating with a
> > > > > > well defined protocol, it is not placing a significant burden on the
> > > > > > QEMU developers' ongoing maintainence, nor has problems with license
> > > > > > compliance. The main problem would come from debugging the combined
> > > > > > system as the external process is essentially a black box from QEMU's
> > > > > > POV. Downstream OS vendors are free to place restrictions on which
> > > > > > backend processes they'd be willing to support with QEMU, and upstream
> > > > > > is under no obligation to debug stuff beyond the QEMU boundary.
> > > > > >
> > > > > > We have already accepted that tradeoff with networking by supporting
> > > > > > vhost-user and have externals impls like DPDK, so I don't see a
> > > > > > compelling reason to try to restrict it for other vhost-user backends.
> > > > >
> > > > > OK seems to be more or less a rough concensus then.
> > > > >
> > > > > I wonder what's the approach wrt migration though.
> > > >
> > > > The series doesn't take care of migration.
> > > >
> > > > >
> > > > > Even the compatibility story about vhost-user isn't
> > > > > great, I would like to see something solid before
> > > > > we allow that.
> > > >
> > > > To allow migration? vhost-user has partial support for migration
> > > > (dirty memory tracking), and there is also "[PATCH v2 for-4.0 0/7]
> > > > vhost-user-blk: Add support for backend reconnecting" - allowing the
> > > > backend to store some state, if I understand correctly, which could be
> > > > leveraged I guess...
> > > >
> > > > But I don't think we should block this series because migration isn't
> > > > tackled here.
> > > >
> > > > thanks
> > > >
> > > >
> > > > .
> > >
> > > How about blocking migration for now then?
> >
> > The device here (vhost-user-input) blocks migration (unmigratable = 1)
>
> Right. But that device is just an excersize, right?

Mostly

> It bothers me that next device might not remember and
> we will get a mess.

The next device (the one I care most about) is vhost-user-gpu.

> Could we make it somehow that if there is no vmsd
> then migration is blocked?

Where would you do that? in DeviceClass? That might break other
devices, needs code review. In VirtIODevice? that would be probably
simpler.



-- 
Marc-André Lureau

On Thu, Dec 20, 2018 at 04:40:55PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Dec 19, 2018 at 7:42 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Dec 19, 2018 at 12:01:59PM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Wed, Dec 19, 2018 at 3:20 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Tue, Dec 18, 2018 at 10:35:05PM +0400, Marc-André Lureau wrote:
> > > > > Hi
> > > > >
> > > > > On Tue, Dec 11, 2018 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > >
> > > > > > On Tue, Dec 11, 2018 at 09:29:44AM +0000, Daniel P. Berrangé wrote:
> > > > > > > On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
> > > > > > > >   Hi,
> > > > > > > >
> > > > > > > > > Right. The main issue is that we need to make sure only
> > > > > > > > > in-tree devices are supported.
> > > > > > > >
> > > > > > > > Well, that is under debate right now, see:
> > > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html
> > > > > > >
> > > > > > > I've previously been against the idea of external plugins for QEMU,
> > > > > > > however, that was when the plugin was something that would be dlopen'd
> > > > > > > by QEMU. That would cause our internal ABI to be exposed to 3rd parties
> > > > > > > which is highly undesirable, even if they were open source to comply
> > > > > > > with the license needs.
> > > > > > >
> > > > > > > When the plugin is a completely isolated process communicating with a
> > > > > > > well defined protocol, it is not placing a significant burden on the
> > > > > > > QEMU developers' ongoing maintainence, nor has problems with license
> > > > > > > compliance. The main problem would come from debugging the combined
> > > > > > > system as the external process is essentially a black box from QEMU's
> > > > > > > POV. Downstream OS vendors are free to place restrictions on which
> > > > > > > backend processes they'd be willing to support with QEMU, and upstream
> > > > > > > is under no obligation to debug stuff beyond the QEMU boundary.
> > > > > > >
> > > > > > > We have already accepted that tradeoff with networking by supporting
> > > > > > > vhost-user and have externals impls like DPDK, so I don't see a
> > > > > > > compelling reason to try to restrict it for other vhost-user backends.
> > > > > >
> > > > > > OK seems to be more or less a rough concensus then.
> > > > > >
> > > > > > I wonder what's the approach wrt migration though.
> > > > >
> > > > > The series doesn't take care of migration.
> > > > >
> > > > > >
> > > > > > Even the compatibility story about vhost-user isn't
> > > > > > great, I would like to see something solid before
> > > > > > we allow that.
> > > > >
> > > > > To allow migration? vhost-user has partial support for migration
> > > > > (dirty memory tracking), and there is also "[PATCH v2 for-4.0 0/7]
> > > > > vhost-user-blk: Add support for backend reconnecting" - allowing the
> > > > > backend to store some state, if I understand correctly, which could be
> > > > > leveraged I guess...
> > > > >
> > > > > But I don't think we should block this series because migration isn't
> > > > > tackled here.
> > > > >
> > > > > thanks
> > > > >
> > > > >
> > > > > .
> > > >
> > > > How about blocking migration for now then?
> > >
> > > The device here (vhost-user-input) blocks migration (unmigratable = 1)
> >
> > Right. But that device is just an excersize, right?
> 
> Mostly
> 
> > It bothers me that next device might not remember and
> > we will get a mess.
> 
> The next device (the one I care most about) is vhost-user-gpu.
> 
> > Could we make it somehow that if there is no vmsd
> > then migration is blocked?
> 
> Where would you do that? in DeviceClass? That might break other
> devices, needs code review. In VirtIODevice? that would be probably
> simpler.

In vhost user core somehow?

> 
> 
> -- 
> Marc-André Lureau

Hi

On Thu, Dec 20, 2018 at 8:34 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Dec 20, 2018 at 04:40:55PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Dec 19, 2018 at 7:42 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Wed, Dec 19, 2018 at 12:01:59PM +0400, Marc-André Lureau wrote:
> > > > Hi
> > > >
> > > > On Wed, Dec 19, 2018 at 3:20 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > >
> > > > > On Tue, Dec 18, 2018 at 10:35:05PM +0400, Marc-André Lureau wrote:
> > > > > > Hi
> > > > > >
> > > > > > On Tue, Dec 11, 2018 at 10:56 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > > >
> > > > > > > On Tue, Dec 11, 2018 at 09:29:44AM +0000, Daniel P. Berrangé wrote:
> > > > > > > > On Tue, Dec 11, 2018 at 08:42:41AM +0100, Hoffmann, Gerd wrote:
> > > > > > > > >   Hi,
> > > > > > > > >
> > > > > > > > > > Right. The main issue is that we need to make sure only
> > > > > > > > > > in-tree devices are supported.
> > > > > > > > >
> > > > > > > > > Well, that is under debate right now, see:
> > > > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04912.html
> > > > > > > >
> > > > > > > > I've previously been against the idea of external plugins for QEMU,
> > > > > > > > however, that was when the plugin was something that would be dlopen'd
> > > > > > > > by QEMU. That would cause our internal ABI to be exposed to 3rd parties
> > > > > > > > which is highly undesirable, even if they were open source to comply
> > > > > > > > with the license needs.
> > > > > > > >
> > > > > > > > When the plugin is a completely isolated process communicating with a
> > > > > > > > well defined protocol, it is not placing a significant burden on the
> > > > > > > > QEMU developers' ongoing maintainence, nor has problems with license
> > > > > > > > compliance. The main problem would come from debugging the combined
> > > > > > > > system as the external process is essentially a black box from QEMU's
> > > > > > > > POV. Downstream OS vendors are free to place restrictions on which
> > > > > > > > backend processes they'd be willing to support with QEMU, and upstream
> > > > > > > > is under no obligation to debug stuff beyond the QEMU boundary.
> > > > > > > >
> > > > > > > > We have already accepted that tradeoff with networking by supporting
> > > > > > > > vhost-user and have externals impls like DPDK, so I don't see a
> > > > > > > > compelling reason to try to restrict it for other vhost-user backends.
> > > > > > >
> > > > > > > OK seems to be more or less a rough concensus then.
> > > > > > >
> > > > > > > I wonder what's the approach wrt migration though.
> > > > > >
> > > > > > The series doesn't take care of migration.
> > > > > >
> > > > > > >
> > > > > > > Even the compatibility story about vhost-user isn't
> > > > > > > great, I would like to see something solid before
> > > > > > > we allow that.
> > > > > >
> > > > > > To allow migration? vhost-user has partial support for migration
> > > > > > (dirty memory tracking), and there is also "[PATCH v2 for-4.0 0/7]
> > > > > > vhost-user-blk: Add support for backend reconnecting" - allowing the
> > > > > > backend to store some state, if I understand correctly, which could be
> > > > > > leveraged I guess...
> > > > > >
> > > > > > But I don't think we should block this series because migration isn't
> > > > > > tackled here.
> > > > > >
> > > > > > thanks
> > > > > >
> > > > > >
> > > > > > .
> > > > >
> > > > > How about blocking migration for now then?
> > > >
> > > > The device here (vhost-user-input) blocks migration (unmigratable = 1)
> > >
> > > Right. But that device is just an excersize, right?
> >
> > Mostly
> >
> > > It bothers me that next device might not remember and
> > > we will get a mess.
> >
> > The next device (the one I care most about) is vhost-user-gpu.
> >
> > > Could we make it somehow that if there is no vmsd
> > > then migration is blocked?
> >
> > Where would you do that? in DeviceClass? That might break other
> > devices, needs code review. In VirtIODevice? that would be probably
> > simpler.
>
> In vhost user core somehow?

I suppose something like that:

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index d51d1087f6..5680dc5d8e 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1470,12 +1470,16 @@ static int vhost_user_backend_init(struct
vhost_dev *dev, void *opaque)
         }
     }

-    if (dev->migration_blocker == NULL &&
-        !virtio_has_feature(dev->protocol_features,
-                            VHOST_USER_PROTOCOL_F_LOG_SHMFD)) {
-        error_setg(&dev->migration_blocker,
-                   "Migration disabled: vhost-user backend lacks "
-                   "VHOST_USER_PROTOCOL_F_LOG_SHMFD feature.");
+    if (dev->migration_blocker == NULL) {
+        if (!virtio_has_feature(dev->protocol_features,
+                                VHOST_USER_PROTOCOL_F_LOG_SHMFD)) {
+            error_setg(&dev->migration_blocker,
+                       "Migration disabled: vhost-user backend lacks "
+                       "VHOST_USER_PROTOCOL_F_LOG_SHMFD feature.");
+        } else if (!qdev_get_vmsd(DEVICE(dev->vdev))) {
+            error_setg(&dev->migration_blocker,
+                       "Migration disabled: vhost-user device lacks VMSD");
+        }
     }

Unfortunately, vdev is not set before vhost_dev_start().

We could add the migration blocker there somehow? There is no
dedicated backend callback for that (I don't think set_features(),
set_mem_table()  etc are good places..)

Looking for help here, this is currently blocking me on
vhost-user-input/vhost-user-gpu.

-- 
Marc-André Lureau

  Hi,

> Unfortunately, vdev is not set before vhost_dev_start().
> 
> We could add the migration blocker there somehow?

Sure.  Just use migrate_add_blocker() to do that at any time (see qxl.c
for an example).

HTH,
  Gerd

Hi

On Wed, Jan 9, 2019 at 12:45 PM Hoffmann, Gerd <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > Unfortunately, vdev is not set before vhost_dev_start().
> >
> > We could add the migration blocker there somehow?
>
> Sure.  Just use migrate_add_blocker() to do that at any time (see qxl.c
> for an example).

VhostUserInput inherits from VirtioInput, which implements vmsd.

The "Add vhost-user-input-pci" patch override the DeviceClass vmsd to
set it as unmigratable. If I understand correctly, Michael suggested
to add a check for device vmsd == NULL in hw/virtio/vhost-user.c
instead.

However, vhost-user devices would still need to overwrite vmsd to NULL.

I don't think there is a benefit in the generic vmsd == NULL check, as
you still need not to forget to overwrite vmsd to NULL in the
vhost-user device.

Am I missing something?

-- 
Marc-André Lureau