[libvirt] [PATCH] qemu: Connect to guest agent iff needed
Michal Privoznik
mprivozn at redhat.com
Mon Dec 21 15:07:01 UTC 2015
https://bugzilla.redhat.com/show_bug.cgi?id=1293351
I've came across a bit of a silly scenario, but long story short:
after domain was resumed, the virDomainSetTime() API hung for 5
seconds after which it failed with an error. Problem was, that
there is no guest agent installed in the domain. But this got me
thinking and digging into the code. So even though we do listen
to VSERPORT_CHANGE events and connect and disconnect ourselves to
guest agent, we still do connect to the guest agent at both
domain startup and resume. This is a bit silly as it produces the
delay - basically, because we connect to the guest agent,
priv->agent is not NULL. Therefore qemuDomainAgentAvailable()
will return true. And the place where we hang? Well,
historically, when there was no VSERPORT_CHANGE event, we used a
dummy ping command to the guest which has 5 seconds timeout. And
it's still there and effective. So there's where the delay comes
from.
What's the resolution? Well, I'm introducing new capability
QEMU_CAPS_VSERPORT_CHANGE that is enabled iff qemu is capable of
the event. And, during domain startup, reconnect after resume and
attach we do connect to the agent socket iff the capability is
NOT set.
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_process.c | 66 ++++++++++++++++++----------
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 1 +
6 files changed, 48 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6e5d203..9e11af9 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -308,6 +308,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"virtio-tablet", /* 205 */
"virtio-input-host",
+ "vserport-change-event",
);
@@ -1479,6 +1480,7 @@ struct virQEMUCapsStringFlags virQEMUCapsEvents[] = {
{ "SPICE_MIGRATE_COMPLETED", QEMU_CAPS_SEAMLESS_MIGRATION },
{ "DEVICE_DELETED", QEMU_CAPS_DEVICE_DEL_EVENT },
{ "MIGRATION", QEMU_CAPS_MIGRATION_EVENT },
+ { "VSERPORT_CHANGE", QEMU_CAPS_VSERPORT_CHANGE },
};
struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 61d6379..983faf6 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -335,6 +335,7 @@ typedef enum {
/* 205 */
QEMU_CAPS_VIRTIO_TABLET, /* -device virtio-tablet-{device,pci} */
QEMU_CAPS_VIRTIO_INPUT_HOST, /* -device virtio-input-host-{device,pci} */
+ QEMU_CAPS_VSERPORT_CHANGE, /* VSERPORT_CHANGE event */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f274068..95a795c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3524,15 +3524,21 @@ qemuProcessReconnect(void *opaque)
if (qemuConnectMonitor(driver, obj, QEMU_ASYNC_JOB_NONE, NULL) < 0)
goto error;
- /* Failure to connect to agent shouldn't be fatal */
- if ((ret = qemuConnectAgent(driver, obj)) < 0) {
- if (ret == -2)
- goto error;
+ /* With newer qemus capable of VSERPORT_CHANGE event, we are connecting and
+ * disconnecting to the guest agent as it connects or disconnects to the
+ * channel within the guest. So, it's important to connect here iff we are
+ * running qemu not capable of the event. */
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE)) {
+ /* Failure to connect to agent shouldn't be fatal */
+ if ((ret = qemuConnectAgent(driver, obj)) < 0) {
+ if (ret == -2)
+ goto error;
- VIR_WARN("Cannot connect to QEMU guest agent for %s",
- obj->def->name);
- virResetLastError();
- priv->agentError = true;
+ VIR_WARN("Cannot connect to QEMU guest agent for %s",
+ obj->def->name);
+ virResetLastError();
+ priv->agentError = true;
+ }
}
if (qemuHostdevUpdateActiveDomainDevices(driver, obj->def) < 0)
@@ -4949,15 +4955,21 @@ qemuProcessLaunch(virConnectPtr conn,
if (qemuProcessWaitForMonitor(driver, vm, asyncJob, priv->qemuCaps, logCtxt) < 0)
goto cleanup;
- /* Failure to connect to agent shouldn't be fatal */
- if ((rv = qemuConnectAgent(driver, vm)) < 0) {
- if (rv == -2)
- goto cleanup;
+ /* With newer qemus capable of VSERPORT_CHANGE event, we are connecting and
+ * disconnecting to the guest agent as it connects or disconnects to the
+ * channel within the guest. So, it's important to connect here iff we are
+ * running qemu not capable of the event. */
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE)) {
+ /* Failure to connect to agent shouldn't be fatal */
+ if ((rv = qemuConnectAgent(driver, vm)) < 0) {
+ if (rv == -2)
+ goto cleanup;
- VIR_WARN("Cannot connect to QEMU guest agent for %s",
- vm->def->name);
- virResetLastError();
- priv->agentError = true;
+ VIR_WARN("Cannot connect to QEMU guest agent for %s",
+ vm->def->name);
+ virResetLastError();
+ priv->agentError = true;
+ }
}
VIR_DEBUG("Detecting if required emulator features are present");
@@ -5664,15 +5676,21 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
if (qemuProcessWaitForMonitor(driver, vm, QEMU_ASYNC_JOB_NONE, priv->qemuCaps, NULL) < 0)
goto error;
- /* Failure to connect to agent shouldn't be fatal */
- if ((ret = qemuConnectAgent(driver, vm)) < 0) {
- if (ret == -2)
- goto error;
+ /* With newer qemus capable of VSERPORT_CHANGE event, we are connecting and
+ * disconnecting to the guest agent as it connects or disconnects to the
+ * channel within the guest. So, it's important to connect here iff we are
+ * running qemu not capable of the event. */
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE)) {
+ /* Failure to connect to agent shouldn't be fatal */
+ if ((ret = qemuConnectAgent(driver, vm)) < 0) {
+ if (ret == -2)
+ goto error;
- VIR_WARN("Cannot connect to QEMU guest agent for %s",
- vm->def->name);
- virResetLastError();
- priv->agentError = true;
+ VIR_WARN("Cannot connect to QEMU guest agent for %s",
+ vm->def->name);
+ virResetLastError();
+ priv->agentError = true;
+ }
}
VIR_DEBUG("Detecting VCPU PIDs");
diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
index 1098dcf..332b85a 100644
--- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
@@ -159,4 +159,5 @@
<flag name='rtl8139'/>
<flag name='e1000'/>
<flag name='virtio-net'/>
+ <flag name='vserport-change-event'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
index d67a48d..1a5fe81 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps
@@ -167,4 +167,5 @@
<flag name='virtio-mouse'/>
<flag name='virtio-tablet'/>
<flag name='virtio-input-host'/>
+ <flag name='vserport-change-event'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
index f4f3673..8b3586a 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps
@@ -168,4 +168,5 @@
<flag name='virtio-mouse'/>
<flag name='virtio-tablet'/>
<flag name='virtio-input-host'/>
+ <flag name='vserport-change-event'/>
</qemuCaps>
--
2.4.10
More information about the libvir-list
mailing list