Re: [Qemu-devel] [PATCH v2 for-4.0 1/7] chardev: Add disconnected option for chardev socket

[Michael S. Tsirkin]

On Wed, Dec 19, 2018 at 05:09:09PM +0000, Daniel P. Berrangé wrote:
> On Wed, Dec 19, 2018 at 11:50:38AM -0500, Michael S. Tsirkin wrote:
> > On Wed, Dec 19, 2018 at 04:01:02PM +0000, Daniel P. Berrangé wrote:
> > > On Wed, Dec 19, 2018 at 10:55:09AM -0500, Michael S. Tsirkin wrote:
> > > > On Tue, Dec 18, 2018 at 04:09:19PM +0000, Daniel P. Berrangé wrote:
> > > > > On Tue, Dec 18, 2018 at 11:02:46AM -0500, Michael S. Tsirkin wrote:
> > > > > > On Tue, Dec 18, 2018 at 03:25:20PM +0000, Daniel P. Berrangé wrote:
> > > > > > > On Tue, Dec 18, 2018 at 04:24:26PM +0400, Marc-André Lureau wrote:
> > > > > > > > Hi
> > > > > > > > 
> > > > > > > > On Tue, Dec 18, 2018 at 2:01 PM <address@hidden> wrote:
> > > > > > > > >
> > > > > > > > > From: Xie Yongji <address@hidden>
> > > > > > > > >
> > > > > > > > > New option "disconnected" is added to init the chardev socket
> > > > > > > > > in disconnected state. Then we can use 
> > > > > > > > > qemu_chr_fe_wait_connected()
> > > > > > > > > to connect when necessary. Now it would be used for unix 
> > > > > > > > > domain
> > > > > > > > > socket of vhost-user-blk device to support reconnect.
> > > > > > > > 
> > > > > > > > What difference does that make if you wait for connection in
> > > > > > > > qemu_chr_fe_wait_connected(), or during chardev setup?
> > > > > > > > 
> > > > > > > > "disconnected" is misleading, would it be possible to reuse
> > > > > > > > "wait/nowait" instead?
> > > > > > > 
> > > > > > > Currently we default to doing a blocking connect in foreground,
> > > > > > > except if reconnect is non-zero, in which case we do a connect
> > > > > > > async in the background. This "disconnected" proposal effectively
> > > > > > > does a blocking connect, but delayed to later in startup.
> > > > > > > 
> > > > > > > IOW, this could already be achieved if "reconnect" were set to
> > > > > > > non-zero. If the usage doesn't want reconnect though, I tend
> > > > > > > to agree that we should use the exisiting wait/nowait options
> > > > > > > to let it be controlled. I don't see that this "disconnected"
> > > > > > > option gives a compelling benefit over using wait/nowait.
> > > > > > 
> > > > > > So the tricky thing is that we can not expose the
> > > > > > device to guest until we get a connection and can query
> > > > > > it for the first time. After that is completed,
> > > > > > we can reasonably support disconnected operation at least for net.
> > > > > 
> > > > > The device code would still use  qemu_chr_fe_wait_connected() in my 
> > > > > proposal,
> > > > > so that its no different to the situation with the proposed 
> > > > > "disconnected"
> > > > > flag.
> > > > > 
> > > > > Regards,
> > > > > Daniel
> > > > 
> > > > I guess so, but wouldn't it be confusing to users that one says
> > > > "nowait" and qemu still waits for a connection and does not
> > > > run the VM? That's different from how nowait behaves normally.
> > > 
> > > Well "nowait" is only referring to the chardev behaviour which is
> > > still an accurate description.
> > 
> > man page seems to say
> > 
> >            nowait specifies that QEMU should not block waiting
> >            for a client to connect to a listening socket.
> > 
> > if we do wait for a client to connect then how is it accurate?
> 
> Well the manpage needs to be clarified that this applies to the
> initialization of the chardev.  What a downstream objet/device
> does with the chardev is outside the scope of chardev config
> options.
> 
> Regards,
> Daniel

Practically qemu blocks. I suspect the distinction which object
within qemu does it will be lost on users.

And it's not exactly about waiting anyway.
It's about whether we should stop the VM on disconnect
which I feel is different.

So I think we need a new flag along the lines of
        disconnect=ignore/exit/stop

with proper documentation.

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