Re: [libvirt] [PATCH v2] qemu: Connect to guest agent iff needed

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(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 11 ++++------- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 25 +++++++++++++++++++------ src/qemu/qemu_process.h | 4 +++- 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 + 10 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f274068..3aca227 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -198,16 +198,29 @@ static qemuAgentCallbacks agentCallbacks = { int -qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) +qemuConnectAgent(virQEMUDriverPtr driver, + virDomainObjPtr vm, + bool force) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; qemuAgentPtr agent = NULL; - virDomainChrSourceDefPtr config = qemuFindAgentConfig(vm->def); + virDomainChrDefPtr config = qemuFindAgentConfig(vm->def); if (!config) return 0; + if (!force && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE) &&

+ config->state == VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED) { + /* 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 or we are called from the event + * callback (@force == true). */

+ return 0; + } + if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager, vm->def) < 0) { VIR_ERROR(_("Failed to set security context for agent for %s"),