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

[Marc-André Lureau]

Hi

On Fri, Oct 12, 2018 at 1:56 PM Daniel P. Berrangé <address@hidden> 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é <address@hidden> 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 :|