qemuConnectAgent return -1 or -2 in case of different errors. A. -1 is a case of unsuccessuful connection to guest agent. B. -2 is a case of destoyed domain during connection attempt. All qemuConnectAgent callers handle the first error the same way so let's move this logic into qemuConnectAgent itself. Patched function returns 0 in case A and -1 in case B. --- src/qemu/qemu_driver.c | 10 ++------ src/qemu/qemu_migration.c | 13 ++--------- src/qemu/qemu_process.c | 58 ++++++++++++----------------------------------- 3 files changed, 19 insertions(+), 62 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 38c8414..9ebe66b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4409,7 +4409,6 @@ processSerialChangedEvent(virQEMUDriverPtr driver, virObjectEventPtr event = NULL; virDomainDeviceDef dev; qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; if (connected) newstate = VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED; @@ -4462,13 +4461,8 @@ processSerialChangedEvent(virQEMUDriverPtr driver, if (STREQ_NULLABLE(dev.data.chr->target.name, "org.qemu.guest_agent.0")) { if (newstate == VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) { - if (!priv->agent) { - if ((rc = qemuConnectAgent(driver, vm)) == -2) - goto endjob; - - if (rc < 0) - priv->agentError = true; - } + if (!priv->agent && qemuConnectAgent(driver, vm) < 0) + goto endjob; } else { if (priv->agent) { qemuAgentClose(priv->agent); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d4a55d8..b029319 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -6183,7 +6183,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver, unsigned short port; unsigned long long timeReceived = 0; virObjectEventPtr event; - int rc; qemuDomainJobInfoPtr jobInfo = NULL; bool inPostCopy = false; bool doKill = true; @@ -6256,16 +6255,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_IN) < 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 (qemuConnectAgent(driver, vm) < 0) + goto endjob; if (flags & VIR_MIGRATE_PERSIST_DEST) { if (qemuMigrationPersist(driver, vm, mig, !v3proto) < 0) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1b67aee..34f8b04 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -204,7 +204,6 @@ int qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - int ret = -1; qemuAgentPtr agent = NULL; virDomainChrDefPtr config = qemuFindAgentConfig(vm->def); @@ -248,8 +247,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) qemuAgentClose(agent); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest crashed while connecting to the guest agent")); - ret = -2; - goto cleanup; + return -1; } if (virSecurityManagerClearSocketLabel(driver->securityManager, @@ -260,18 +258,18 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) goto cleanup; } - priv->agent = agent; - - if (priv->agent == NULL) { + if (!priv->agent) VIR_INFO("Failed to connect agent for %s", vm->def->name); - goto cleanup; - } - - ret = 0; cleanup: - return ret; + if (!priv->agent) { + VIR_WARN("Cannot connect to QEMU guest agent for %s", vm->def->name); + priv->agentError = true; + virResetLastError(); + } + + return 0; } @@ -3245,7 +3243,6 @@ qemuProcessReconnect(void *opaque) int reason; virQEMUDriverConfigPtr cfg; size_t i; - int ret; unsigned int stopFlags = 0; bool jobStarted = false; virCapsPtr caps = NULL; @@ -3389,16 +3386,8 @@ qemuProcessReconnect(void *opaque) if (qemuProcessUpdateDevices(driver, obj) < 0) goto error; - /* 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; - } + if (qemuConnectAgent(driver, obj) < 0) + goto error; /* update domain state XML with possibly updated state in virDomainObj */ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, obj, driver->caps) < 0) @@ -5533,16 +5522,8 @@ 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; - - VIR_WARN("Cannot connect to QEMU guest agent for %s", - vm->def->name); - virResetLastError(); - priv->agentError = true; - } + if (qemuConnectAgent(driver, vm) < 0) + goto cleanup; VIR_DEBUG("Detecting if required emulator features are present"); if (!qemuProcessVerifyGuestCPU(driver, vm, asyncJob)) @@ -6232,7 +6213,6 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virCapsPtr caps = NULL; bool active = false; - int ret; VIR_DEBUG("Beginning VM attach process"); @@ -6359,16 +6339,8 @@ 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; - - VIR_WARN("Cannot connect to QEMU guest agent for %s", - vm->def->name); - virResetLastError(); - priv->agentError = true; - } + if (qemuConnectAgent(driver, vm) < 0) + goto error; VIR_DEBUG("Detecting VCPU PIDs"); if (qemuDomainRefreshVcpuInfo(driver, vm, QEMU_ASYNC_JOB_NONE, false) < 0) -- 1.8.3.1

Sometimes after domain restart agent is unavailabe even if it is up and running in guest. Diagnostic message is "QEMU guest agent is not available due to an error" that is 'priv->agentError' is set. Investiagion shows that 'priv->agent' is not NULL, so error flag is set probably during domain shutdown process and not cleaned up eventually. The patch is quite simple - just clean up error flag unconditionally upon domain stop. Other hunks address other cases when error flag is not cleaned up. 1. processSerialChangedEvent. We need to clean error flag unconditionally here too. For example if upon first 'connected' event we fail to connect and set error flag and then connect on second 'connected' event then error flag will remain set erroneously and make agent unavailable. 2. qemuProcessHandleAgentEOF. If error flag is set and we get EOF we need to change state (and diagnostic) from 'error' to 'not connected'. --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ae1b9fe..c2a6336 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4467,8 +4467,8 @@ processSerialChangedEvent(virQEMUDriverPtr driver, if (priv->agent) { qemuAgentClose(priv->agent); priv->agent = NULL; - priv->agentError = false; } + priv->agentError = false; } event = virDomainEventAgentLifecycleNewFromObj(vm, newstate, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 34f8b04..ffc8126 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -151,6 +151,7 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent, qemuAgentClose(agent); priv->agent = NULL; + priv->agentError = false; virObjectUnlock(vm); return; @@ -5963,8 +5964,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, if (priv->agent) { qemuAgentClose(priv->agent); priv->agent = NULL; - priv->agentError = false; } + priv->agentError = false; if (priv->mon) { qemuMonitorClose(priv->mon); -- 1.8.3.1