[libvirt] [PATCH 0/3] Only talk to qemu guest agent when the domain is running
Previously, only virDomainObjIsActive() was used to check domain liveness. This includes states like 'VIR_DOMAIN_PMSUSPENDED', where QEMU is running, but the guest is not. Explicitly check the state against VIR_DOMAIN_RUNNING and refuse to communicate with the agent if it does not match. https://bugzilla.redhat.com/show_bug.cgi?id=872424 Ján Tomko (3): Check for qemu guest agent availability after getting the job Pass virDomainObjPtr to qemuDomainAgentAvailable Check if domain is running in qemuDomainAgentIsAvailable src/qemu/qemu_domain.c | 11 ++++++++++- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++------------------------ 3 files changed, 36 insertions(+), 26 deletions(-) -- 2.0.5
This way checks requiring the job can be done in qemuDomainAgentAvailable. --- src/qemu/qemu_driver.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e282464..d5b9d0d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1939,16 +1939,16 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + agentForced = agentRequested && !acpiRequested; if (!qemuDomainAgentAvailable(priv, agentForced)) { if (agentForced) - goto cleanup; + goto endjob; useAgent = false; } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -2037,9 +2037,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) priv->agent)) useAgent = true; - if (useAgent && !qemuDomainAgentAvailable(priv, true)) { - goto cleanup; - } else { + if (!useAgent) { #if WITH_YAJL if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { @@ -2060,6 +2058,9 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; + if (useAgent && !qemuDomainAgentAvailable(priv, true)) + goto endjob; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -17610,12 +17611,12 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, } } - if (!qemuDomainAgentAvailable(priv, true)) - goto cleanup; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; + if (!qemuDomainAgentAvailable(priv, true)) + goto endjob; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -17730,12 +17731,12 @@ qemuDomainQemuAgentCommand(virDomainPtr domain, goto cleanup; } - if (!qemuDomainAgentAvailable(priv, true)) - goto cleanup; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; + if (!qemuDomainAgentAvailable(priv, true)) + goto endjob; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -17839,12 +17840,12 @@ qemuDomainFSTrim(virDomainPtr dom, goto cleanup; } - if (!qemuDomainAgentAvailable(priv, true)) - goto cleanup; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; + if (!qemuDomainAgentAvailable(priv, true)) + goto endjob; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); -- 2.0.5
Not just the DomainObj's private data. --- src/qemu/qemu_domain.c | 4 +++- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 24 ++++++++++++------------ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ac5ca74..f3f3910 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2798,9 +2798,11 @@ qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, } bool -qemuDomainAgentAvailable(qemuDomainObjPrivatePtr priv, +qemuDomainAgentAvailable(virDomainObjPtr vm, bool reportError) { + qemuDomainObjPrivatePtr priv = vm->privateData; + if (priv->agentError) { if (reportError) { virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b2c3881..fe3e2b1 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -395,7 +395,7 @@ bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver, virDomainDefPtr src, virDomainDefPtr dst); -bool qemuDomainAgentAvailable(qemuDomainObjPrivatePtr priv, +bool qemuDomainAgentAvailable(virDomainObjPtr vm, bool reportError); int qemuDomainJobInfoUpdateTime(qemuDomainJobInfoPtr jobInfo) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d5b9d0d..519705c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1943,7 +1943,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) goto cleanup; agentForced = agentRequested && !acpiRequested; - if (!qemuDomainAgentAvailable(priv, agentForced)) { + if (!qemuDomainAgentAvailable(vm, agentForced)) { if (agentForced) goto endjob; useAgent = false; @@ -2058,7 +2058,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - if (useAgent && !qemuDomainAgentAvailable(priv, true)) + if (useAgent && !qemuDomainAgentAvailable(vm, true)) goto endjob; if (!virDomainObjIsActive(vm)) { @@ -4766,7 +4766,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) goto endjob; if (nvcpus > vm->def->vcpus) { @@ -5490,7 +5490,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) goto endjob; if (!virDomainObjIsActive(vm)) { @@ -12892,7 +12892,7 @@ qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, qemuDomainObjPrivatePtr priv = vm->privateData; int frozen; - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) return -1; qemuDomainObjEnterAgent(vm); @@ -12912,7 +12912,7 @@ qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, int thawed; virErrorPtr err = NULL; - if (!qemuDomainAgentAvailable(priv, report)) + if (!qemuDomainAgentAvailable(vm, report)) return -1; qemuDomainObjEnterAgent(vm); @@ -17614,7 +17614,7 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) goto endjob; if (!virDomainObjIsActive(vm)) { @@ -17734,7 +17734,7 @@ qemuDomainQemuAgentCommand(virDomainPtr domain, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) goto endjob; if (!virDomainObjIsActive(vm)) { @@ -17843,7 +17843,7 @@ qemuDomainFSTrim(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) goto endjob; if (!virDomainObjIsActive(vm)) { @@ -18027,7 +18027,7 @@ qemuDomainGetTime(virDomainPtr dom, goto endjob; } - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) goto endjob; qemuDomainObjEnterAgent(vm); @@ -18086,7 +18086,7 @@ qemuDomainSetTime(virDomainPtr dom, goto endjob; } - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) goto endjob; qemuDomainObjEnterAgent(vm); @@ -19063,7 +19063,7 @@ qemuDomainGetFSInfo(virDomainPtr dom, goto endjob; } - if (!qemuDomainAgentAvailable(priv, true)) + if (!qemuDomainAgentAvailable(vm, true)) goto endjob; qemuDomainObjEnterAgent(vm); -- 2.0.5
If the domain is not running, the agent will not respond. Do not even try. https://bugzilla.redhat.com/show_bug.cgi?id=872424 --- src/qemu/qemu_domain.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f3f3910..28bfbee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2818,6 +2818,13 @@ qemuDomainAgentAvailable(virDomainObjPtr vm, } return false; } + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { + if (reportError) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + } + return false; + } return true; } -- 2.0.5
On 02/27/2015 08:30 AM, Ján Tomko wrote:
Previously, only virDomainObjIsActive() was used to check domain liveness. This includes states like 'VIR_DOMAIN_PMSUSPENDED', where QEMU is running, but the guest is not.
Explicitly check the state against VIR_DOMAIN_RUNNING and refuse to communicate with the agent if it does not match.
https://bugzilla.redhat.com/show_bug.cgi?id=872424
Ján Tomko (3): Check for qemu guest agent availability after getting the job Pass virDomainObjPtr to qemuDomainAgentAvailable Check if domain is running in qemuDomainAgentIsAvailable
ACK series; since it fixes a bug it is probably safe for freeze.
src/qemu/qemu_domain.c | 11 ++++++++++- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++------------------------ 3 files changed, 36 insertions(+), 26 deletions(-)
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Ján Tomko