On 10/01/2018 14:18, Klim Kireev wrote:
The following behavior was observed for QEMU configured by libvirt
to use guest agent as usual for the guests without virtio-serial
driver (Windows or the guest remaining in BIOS stage).
In QEMU on first connect to listen character device socket
the listen socket is removed from poll just after the accept().
virtio_serial_guest_ready() returns 0 and the descriptor
of the connected Unix socket is removed from poll and it will
not be present in poll() until the guest will initialize the driver
and change the state of the serial to "guest connected".
In libvirt connect() to guest agent is performed on restart and
is run under VM state lock. Connect() is blocking and can
wait forever.
In this case libvirt can not perform ANY operation on that VM.
The bug can be easily reproduced this way:
Terminal 1:
qemu-system-x86_64 -m 512 -device pci-serial,chardev=serial1 -chardev
socket,id=serial1,path=/tmp/console.sock,server,nowait
(virtio-serial and isa-serial also fit)
Terminal 2:
minicom -D unix\#/tmp/console.sock
(type something and pres enter)
C-a x (to exit)
Do 3 times:
minicom -D unix\#/tmp/console.sock
C-a x
It needs 4 connections, because the first one is accepted by QEMU, then two are
queued by
the kernel, and the 4th blocks.
The problem is that QEMU doesn't add a read watcher after succesful read
until the guest device wants to acquire recieved data, so
I propose to install a separate pullhup watcher regardless of
whether the device waits for data or not. After closing the connection,
the guest has a capability to read the data within timeout.
I don't understand the timeout part.
Apart from that, maybe the bug is in io_watch_poll_prepare, which needs
to _always_ set up a "G_IO_ERR | G_IO_HUP | G_IO_NVAL" watch. Only
G_IO_IN depends on iwp->fd_can_read(...) > 0.
So the change would start with something like this:
diff --git a/chardev/char-io.c b/chardev/char-io.c
index f81052481a..a5e65d4e7c 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -29,6 +29,7 @@ typedef struct IOWatchPoll {
QIOChannel *ioc;
GSource *src;
+ GIOCondition cond;
IOCanReadHandler *fd_can_read;
GSourceFunc fd_read;
@@ -41,25 +42,32 @@ static IOWatchPoll *io_watch_poll_from_source(GSource
*source)
return container_of(source, IOWatchPoll, parent);
}
+static void io_watch_poll_destroy_source(IOWatchPoll *iwp)
+{
+ if (iwp->src) {
+ g_source_destroy(iwp->src);
+ g_source_unref(iwp->src);
+ iwp->src = NULL;
+ iwp->cond = 0;
+ }
+}
+
static gboolean io_watch_poll_prepare(GSource *source,
gint *timeout)
{
IOWatchPoll *iwp = io_watch_poll_from_source(source);
bool now_active = iwp->fd_can_read(iwp->opaque) > 0;
- bool was_active = iwp->src != NULL;
- if (was_active == now_active) {
- return FALSE;
+ GIOCondition cond = G_IO_ERR | G_IO_HUP | G_IO_NVAL;
+ if (now_active) {
+ cond |= G_IO_IN;
}
- if (now_active) {
- iwp->src = qio_channel_create_watch(
- iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
+ if (iwp->cond != cond) {
+ io_watch_poll_destroy_source(iwp);
+ iwp->cond = cond;
+ iwp->src = qio_channel_create_watch(iwp->ioc, cond);
g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
g_source_attach(iwp->src, iwp->context);
- } else {
- g_source_destroy(iwp->src);
- g_source_unref(iwp->src);
- iwp->src = NULL;
}
return FALSE;
}
@@ -131,11 +139,7 @@ static void io_remove_watch_poll(GSource *source)
IOWatchPoll *iwp;
iwp = io_watch_poll_from_source(source);
- if (iwp->src) {
- g_source_destroy(iwp->src);
- g_source_unref(iwp->src);
- iwp->src = NULL;
- }
+ io_watch_poll_destroy_source(iwp);
g_source_destroy(&iwp->parent);
}
Thanks,
Paolo