Re: [Qemu-devel] [PATCH 11/18] vhost-user: add shutdown support

[Michael S. Tsirkin]

On Mon, May 02, 2016 at 01:29:18PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Mon, May 2, 2016 at 12:45 PM, Michael S. Tsirkin <address@hidden> wrote:
> > 1. Graceful disconnect
> > - One should be able to do vmstop, disconnect, connect then vm start
> >   This looks like a nice intermediate step.
> > - Why would we always assume it's always remote initiating the disconnect?
> >   Monitor commands for disconnecting seem called for.
> 
> Those two solutions involve VM management. This looks more complex to
> communicate/synchronize vhost-user backend & vm management & qemu. The
> use case I cover is request from the backend to shutdown,

Right, but flushing buffers + closing the socket looks like
a cleaner interface than a ton of messages going back and forth.

> because
> that's what the users wanted (and it is already possible to stop the
> backend and disconnect it from qemu, we would only need to know when,
> with a new command..)

You assume the backend has a monitor interface to disconnect though.
That's not a given.

> >
> > 2. Unexpected disconnect
> > - If buffers are processed in order or flushed before socket close,
> >   then on disconnect status can be recovered
> >   from ring alone. If that is of interest, we might want to add
> >   a protocol flag for that. F_DISCONNECT_FLUSH ? Without this,
> >   only graceful disconnect or reset with guest's help can be supported.
> 
> (doing all this, at this point, I don't see much difference with
> having an explicit shutdown)

With graceful shutdown we implicitly request flush when we ask
backend to stop.

> > - As Marc-André said, without graceful shutdown it is not enough to
> >   handle ring state though.  We must handle socket getting disconnected
> >   in the middle of send/receive.  While it's more work,
> >   it does seem like a nice interface to support.
> > - I understand the concern that existing guests do not handle NEED_RESET
> >   status. One way to fix this would be a new feature bit. If NEED_RESET not
> >   handled, request hot-unplug instead.
> >
> > 3. Running while disconnected
> > - At the moment, we wait on vm start for remote to connect,
> >   if we support disconnecting backend without stopping
> >   we probably should also support starting it without waiting
> >   for connection
> 
> That's what Tetsuya proposed in its initial series, but handling the
> flags was quite tedious.

That would be up to management. E.g. let backend export the list
of flags it supports in some file, and apply to QEMU.

> I think this can be considered easily a
> seperate enhancement. What I proposed is to keep waiting for the
> initial connect, and check the flags remains compatible on reconnect.

Seems asymmetrical unless we stop the vm.

> > - Guests expect tx buffers to be used in a timely manner, thus:
> > - If vm is running we must use them in qemu, maybe discarding packets
> >   in the process.
> >   There already are commands for link down, I'm not sure there's value
> >   in doing this automatically in qemu.
> 
> Testing doesn't show such buffer issues when the link is down (this
> can be tested with vubr example above)

Not enough testing then - it's a race condition: buffers can be sent
before link down.

> > - Alternatively, we should also have an option to stop vm automatically 
> > (like we do on
> >   disk errors) to reduce number of dropped packets.
> 
> Why not, we would need to know if this is actually useful.
> 
> >
> > 4. Reconnecting
> > - When acting as a server, we might want an option to go back to
> >   listening state, but it should not be the only option,
> >   there should be a monitor command for moving it back to
> >   listening state.
> > - When acting as a client, auto-reconnect might be handy sometimes, but 
> > should not be the only
> >   option, there should be a way to manually request connect, possibly to
> >   a different target, so a monitor command for re-connecting seems called 
> > for.
> > - We'll also need monitor events for disconnects so management knows it
> >   must re-connect/restart listening.
> > - If we stopped VM, there could be an option to restart on reconnect.
> 
> That's all involving a third party, adding complexity but the benefit
> isn't so clear.

It's rather clear to me. Let's assume you want to restart bridge,
so you trigger disconnect.
If qemu auto-reconnects there's a race as it might re-connect
to the old bridge.
Management is required to make this robust, auto-reconnect
is handy for people bypassing management.

> -- 
> Marc-André Lureau