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

Hi On Fri, Oct 12, 2018 at 1:56 PM Daniel P. Berrangé <berrange(a)redhat.com&gt; 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(a)redhat.com&gt; 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.