Re: docs: Update vhost-user spec regarding backend program conventions

["Boeuf, Sebastien" <sebastien.boeuf@intel.com>]

Hi Marc-Andre,

On Tue, 2020-02-11 at 22:24 +0100, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Feb 11, 2020 at 4:24 PM Boeuf, Sebastien
> <sebastien.boeuf@intel.com> wrote:
> > From c073d528b8cd7082832fd1825dc33dd65b305aa2 Mon Sep 17 00:00:00
> > 2001
> > From: Sebastien Boeuf <sebastien.boeuf@intel.com>
> > Date: Tue, 11 Feb 2020 16:01:22 +0100
> > Subject: [PATCH] docs: Update vhost-user spec regarding backend
> > program
> >  conventions
> > 
> > The vhost-user specification is not clearly stating the expected
> > behavior from a backend program whenever the client disconnects.
> > 
> > This patch addresses the issue by defining the default behavior and
> > proposing an alternative through a command line option.
> > 
> > By default, a backend program will have to keep listening even if
> > the
> > client disconnects, unless told otherwise through the newly
> > introduced
> > option --exit-on-disconnect.
> > 
> > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  docs/interop/vhost-user.rst | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-
> > user.rst
> > index 5f8b3a456b..da9a1ebc4c 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -1323,6 +1323,10 @@ The backend program must end (as quickly and
> > cleanly as possible) when
> >  the SIGTERM signal is received. Eventually, it may receive SIGKILL
> > by
> >  the management layer after a few seconds.
> > 
> > +By default, the backend program continues running after the client
> > +disconnects. It accepts only 1 connection at a time on each UNIX
> > domain
> > +socket.
> 
> I don't think that's the most common behaviour. libvhost-user will
> panic() on disconnect in general, unless the error/exit is handled
> gracefully by the backend.

It's not the default behavior from libvhost-user, but that's exactly
something I'd like to see changing. This should be the normal case if
you think about a standard client/server connection, where the server
would simply listen again after the client disconnects.

> 
> The most common case is to have 1-1 relation between device/qemu
> instance and backend.

Yes this part is fine, but that's not a reason why the backend should
terminates.

> 
> Why not restart the backend for another instance?

Because you need some management tool to do so. And I think that by
default it could be interesting to have the least amount of extra
management involved.

> 
> > +
> >  The following command line options have an expected behaviour.
> > They
> >  are mandatory, unless explicitly said differently:
> > 
> > @@ -1337,6 +1341,12 @@ are mandatory, unless explicitly said
> > differently:
> >    vhost-user socket as file descriptor FDNUM. It is incompatible
> > with
> >    --socket-path.
> > 
> > +--exit-on-disconnect
> > +
> > +  When this option is provided, the backend program must terminate
> > when
> > +  the client disconnects. This can be used to keep the backend
> > program's
> > +  lifetime synchronized with its client process.
> 
> This section list options that are mandatory. It's probably a bit
> late
> to add more mandatory options (I regret already some of them)

The spec states "They are mandatory, unless explicitly said
differently", and in this case I'm explicitely saying "When this option
is provided", which means if it's not provided it's fine and we can
ignore the fact it's not there.

> 
> Do we need to specify the behaviour on client disconnect? Can't we
> leave that to the backend and management layer to decide?

My goal here is to make the spec a bit less loose. I know libvhost-user 
is the de-facto implementation but we cannot just assume everything out
of the libvhost-user implementation, especially since there is a
dedicated spec. That's the reason why I thought it'd be nice to have
the backend behavior well defined in the spec.
The point is, relying on the current definition, there's not enough
information to make sure a VMM will properly interface with a vhost-
user backend.

Thanks,
Sebastien

> 
> 
> > +
> >  --print-capabilities
> > 
> >    Output to stdout the backend capabilities in JSON format, and
> > then
> > --
> > 2.20.1
> > 
> > -----------------------------------------------------------------
> > ----
> > Intel Corporation SAS (French simplified joint stock company)
> > Registered headquarters: "Les Montalets"- 2, rue de Paris,
> > 92196 Meudon Cedex, France
> > Registration Number:  302 456 199 R.C.S. NANTERRE
> > Capital: 4,572,000 Euros
> > 
> > This e-mail and any attachments may contain confidential material
> > for
> > the sole use of the intended recipient(s). Any review or
> > distribution
> > by others is strictly prohibited. If you are not the intended
> > recipient, please contact the sender and delete all copies.
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.