qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] qemu-ga behavior on virtio-serial unplug


From: Amit Shah
Subject: Re: [Qemu-devel] qemu-ga behavior on virtio-serial unplug
Date: Tue, 13 Aug 2013 17:30:51 +0530

Hi,

(Sorry for the late reply!)

On (Thu) 20 Jun 2013 [10:20:01], mdroth wrote:
> On Thu, Jun 20, 2013 at 10:12:30AM -0500, mdroth wrote:
> > On Wed, Jun 19, 2013 at 01:17:57PM +0200, Laszlo Ersek wrote:
> > > Hello Michael,
> > > 
> > > this is with reference to
> > > <https://bugzilla.redhat.com/show_bug.cgi?id=907733>.
> > > 
> > > Ever since the initial qemu-ga commit AFAICS an exception for
> > > virtio-serial has existed, when reading EOF from the channel.
> > > 
> > > For isa-serial, EOF results in the client connection being closed. I
> > > assume this exits the glib main loop somehow (otherwise qemu-ga would
> > > just sit there and do nothing, as no further connections are accepted I
> > > think).
> > 
> > I think it would actually do the latter, unfortunately. It's distinct
> > from virtio-serial handling in that we remove the GSource by return false
> > in the event handler (qga/main.c:channel_event_cb), but I think we'd
> > need to drop in a g_main_loop_quit() to actually get qemu-ga to exit in
> > that scenario.
> > 
> > This doesn't normally get triggered though, as isa-serial does not send
> > EOF when nothing is connected to the chardev backend, but instead just
> > blocks. Might till make sense to make qemu-ga exit in this case though
> > since it won't be doing anything useful and wrapper scripts would at
> > least have some indication that something is up.
> > 
> > > 
> > > For a unix domain socket, we can continue accepting new connections
> > > after reading EOF.
> > > 
> > > For virtio-serial, EOF means "no host-side process is connected". In
> > > this case we sleep a bit and go back to reading from the same fd (and
> > > this turns into a sleep loop until the next host-side process connects).
> > > 
> > > 
> > > Can we tell "virtio-serial port unplug" from "no host-side process"?
> > > Because in the former case qemu-ga should really close the channel (and
> > > maybe exit (*)), otherwise the unplug won't complete in the guest kernel.

Port unplug will give an error return, and 'no host-side process' will
give EOF.  A bug was fixed recently in 3.11-rc5, and that fix is
queued for the -stable kernels as well.

Upstream kernel commit 96f97a83910cdb9d89d127c5ee523f8fc040a804

> > > According to Amit's comments in the RHBZ, at unplug a SIGIO is
> > > delivered, and a POLLHUP event is reported. However,
> > > 
> > > (a) I think the glib iochannel abstraction doesn't allow us to tell
> > > POLLHUP apart from reading EOF;
> > 
> > AFAICT we can actually access the POLLHUP event via the 'condition' param
> > that gets passed to the cb, but the issue is we also get POLLUP when
> > the chardev backend isn't open.
> > 
> > > 
> > > (b) delivery of an unhandled SIGIO seems to terminate the victim
> > > process. qemu-ga doesn't seem to either catch or block SIGIO, which is
> > > why I think a SIGIO signal is not sent in reality (maybe we should ask
> > > for it first?)
> > > 
> > > ... Actually I'm confused about this as well. The virtio-serial port
> > > *is* opened with O_ASYNC (and on Solaris, it is replaced with an
> > > I_SETSIG ioctl()). What's the reason for this? g_io_channel_unix_new()
> > > doesn't seem to list it as a requirement, and qemu-ga doesn't seem to
> > > handle SIGIO.
> > 
> > At some point I played around with trying to use SIGIO to handle channel
> > resets and whatnot (since we're also supposed to get one when someone
> > opens the chardev backend and causes VIRTIO_CONSOLE_PORT_OPEN to get
> > sent). I don't think I ever got it working, SIGIO doesn't seem to get
> > sent, so that O_ASYNC might just be a relic from that.
> > 
> > I tried installing a handler retested host-connect as well as hot
> > unplug and still don't seem to be getting the signal. Not sure if i'm
> > doing something wrong or if there's an issue with the guest driver.
> > 
> > I did notice something interesting though:
> > 
> > 1371740628.596505: debug: cb: condition: 17, status: 2
> > 1371740628.596541: debug: received EOF
> > 1371740628.395726: debug: cb: condition: 17, status: 2
> > 1371740628.395760: debug: received EOF
> > 1371740628.496035: debug: cb: condition: 17, status: 2
> > 1371740628.496072: debug: received EOF
> > 1371740628.596505: debug: cb: condition: 17, status: 2
> > 1371740628.596541: debug: received EOF
> > 
> > <host opened chardev backend>
> > 
> > 1371740634.195524: debug: cb: condition: 1, status: 1
> > 1371740634.195556: debug: read data, count: 25, data:
> > {'execute':'guest-ping'}
> > 
> > 1371740634.195634: debug: process_event: called
> > 1371740634.195660: debug: processing command
> > 1371740634.196007: debug: sending data, count: 15
> > 
> > <virtio-serial unplugged>
> > 
> > 1371740644.113346: debug: cb: condition: 16, status: 2
> > 1371740644.113379: debug: received EOF
> > 1371740644.213694: debug: cb: condition: 16, status: 2
> > 1371740644.213725: debug: received EOF
> > 1371740644.314041: debug: cb: condition: 16, status: 2
> > 1371740644.314168: debug: received EOF
> > 
> > i.e. we got the POLLHUP if we read from an
> > unconnected-but-present port, and we *don't* get the POLLHUP
> > if the port has been unplugged.
> 
> Er...silly me, POLLHUP=16, POLLIN=1 for glib, so I mixed them
> up. For unplugged case we get POLLHUP, for unconnected case we get
> POLLIN | POLLHUP, so that might actually be enough to distinguish
> unplug if this is the intended behavior.
> 
> Amit, can you confirm?

For unconnected, you should get POLLHUP.  For unplug, even poll should
return error now.

> > And in none of these cases do the SIGIO seem to be sent.

Yes, that was a bug where SIGIO wasn't sent when port was unplugged.
Fixed in upstream kernel 92d3453815fbe74d539c86b60dab39ecdf01bb99, and
will trickle to -stable kernels soon.

> > > In any case we'd need a way to tell "host side close" from "port unplug".
> > > 
> > 
> > Poking around a bit it seems that the SIGIO can be tied to a specific
> > kind of event we can extract via siginfo_t. Right now it looks like
> > virtio-serial is hard-coded to set POLL_OUT, but with some driver
> > changes we could maybe tie unplug to POLL_HUP or something.
> > 
> > So with some driver changes qemu-ga can be made to handle this
> > gracefully, but otherwise I don't have any good ideas.
> > 
> > The only that comes to mind is adding a 'quit' command to qemu-ga that
> > libvirt could call prior to unplug, and once we get around to integrating
> > qemu-ga into qmp qemu could issue it internally as part of the unplug.
> > This isn't consumable for other stuff uses virtio-serial though so I
> > think working SIGIO into doing what we want is probably the best
> > approach.
> > 
> > There's also taking advantage of the above behavior (EOF and no POLLHUP
> > means we were hot-unplugged) but based I what I've read that's not the
> > intended behavior.
> > 
> > > (*) Then, there's the question what to do after unplug. Should we keep
> > > reopening the same virtio-serial port (and sleeping a bit in-between)?
> > > Or exit and let udev / systemd restart qemu-ga on any new virtio-serial
> > > port, passing -p accordingly?
> > 
> > Event-based would be pretty spiffy, but that doesn't preclude us from
> > adding a "--wait-for-channel" type of option that plays a little more
> > nicely with a watchdog-style deployment.

Thanks,

                Amit



reply via email to

[Prev in Thread] Current Thread [Next in Thread]