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

https://bugzilla.redhat.com/show_bug.cgi?id=1293351 Since we already have virtio channel events, we know when guest agent within guest has (dis-)connected. Instead of us blindly connecting to a socket that no one is listening to, we can just follow what qemu-ga does. This has a nice benefit that we don't need to 'guest-ping' the agent just to timeout and find out nobody is listening. The way that this commit is implemented: - don't connect in qemuProcessStart directly, defer that to event callback (which already follows the agent).

- after migration is settled, before we resume vCPUs, ask qemu whether somebody is listening on the socket and if so, connect to it. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- diff to v3: -Move cap detection into qemuConnectAgent src/qemu/qemu_driver.c | 13 ++++++++++++- src/qemu/qemu_migration.c | 12 ++++++++++++ src/qemu/qemu_process.c | 24 ++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8ccf68b..3751ba6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6650,12 +6650,13 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, bool start_paused, qemuDomainAsyncJob asyncJob) { - int ret = -1; + int rc, ret = -1; virObjectEventPtr event; int intermediatefd = -1; virCommandPtr cmd = NULL; char *errbuf = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; if ((header->version == 2) && (header->compressed != QEMU_SAVE_FORMAT_RAW)) { @@ -6710,6 +6711,16 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, goto cleanup; } + if ((rc = qemuConnectAgent(driver, vm)) < 0) { + if (rc == -2) + goto cleanup; + + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_RESTORED); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 51e7125..9678adf 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5795,6 +5795,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, unsigned short port; unsigned long long timeReceived = 0; virObjectEventPtr event; + int rc; VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d", @@ -5863,6 +5864,17 @@ qemuMigrationFinish(virQEMUDriverPtr driver, if (qemuMigrationStopNBDServer(driver, vm, mig) < 0) goto endjob; + if ((rc = qemuConnectAgent(driver, vm)) < 0) { + if (rc == -2) + goto endjob; + + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + + if (flags & VIR_MIGRATE_PERSIST_DEST) { if (qemuMigrationPersist(driver, vm, mig, !v3proto) < 0) { /* Hmpf. Migration was successful, but making it persistent diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 05cbda2..e1a25dd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -77,6 +77,10 @@ VIR_LOG_INIT("qemu.qemu_process"); +static int +qemuProcessReconnectRefreshChannelVirtioState(virQEMUDriverPtr driver, + virDomainObjPtr vm); + /** * qemuProcessRemoveDomainStatus * @@ -208,6 +212,26 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) if (!config) return 0; + if (priv->agent) + return 0; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE) && + config->state != VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) { + /* So qemu is capable of sending us event whenever guest agent + * (dis-)connects. And we reflect its state in config->state. Well, + * nearly. It may happen that what we think about the guest agent state + * is not accurate, e.g. right after migration. Ask qemu to be sure. */ + + if (qemuProcessReconnectRefreshChannelVirtioState(driver, vm) < 0) + goto cleanup; + + /* Now we are sure about the channel state. */ + if (config->state != VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) { + VIR_DEBUG("Deferring connecting to guest agent"); + return 0; + } + } + if (virSecurityManagerSetDaemonSocketLabel(driver->securityManager, vm->def) < 0) { VIR_ERROR(_("Failed to set security context for agent for %s"),