[libvirt] [PATCH] qemu: Obtain job before checking if domain is live
Since obtaining a job can wait for another job to finish, the state might change in the meantime. And checking it more than once is pointless. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index efb3f854fb10..e677440c20b2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5388,16 +5388,16 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) goto cleanup; if (flags & VIR_DOMAIN_VCPU_GUEST) { + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("vCPU count provided by the guest agent can only be " "requested for live domains")); - goto cleanup; + goto endjob; } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) - goto cleanup; - if (!qemuDomainAgentAvailable(vm, true)) goto endjob; @@ -17796,10 +17796,13 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); - goto cleanup; + goto endjob; } if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_WAKEUP) && @@ -17808,7 +17811,7 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Unable to suspend domain due to " "missing system_wakeup monitor command")); - goto cleanup; + goto endjob; } if (vm->def->pm.s3 || vm->def->pm.s4) { @@ -17817,29 +17820,20 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, target == VIR_NODE_SUSPEND_TARGET_HYBRID)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("S3 state is disabled for this domain")); - goto cleanup; + goto endjob; } if (vm->def->pm.s4 == VIR_TRISTATE_BOOL_NO && target == VIR_NODE_SUSPEND_TARGET_DISK) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("S4 state is disabled for this domain")); - goto cleanup; + goto endjob; } } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - if (!qemuDomainAgentAvailable(vm, true)) goto endjob; - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto endjob; - } - qemuDomainObjEnterAgent(vm); ret = qemuAgentSuspend(priv->agent, target); qemuDomainObjExitAgent(vm); @@ -17942,15 +17936,15 @@ qemuDomainQemuAgentCommand(virDomainPtr domain, if (virDomainQemuAgentCommandEnsureACL(domain->conn, vm->def) < 0) goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); - goto cleanup; + goto endjob; } - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - if (!qemuDomainAgentAvailable(vm, true)) goto endjob; -- 2.8.4
On 13.06.2016 15:33, Martin Kletzander wrote:
Since obtaining a job can wait for another job to finish, the state might change in the meantime. And checking it more than once is pointless.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index efb3f854fb10..e677440c20b2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
@@ -17942,15 +17936,15 @@ qemuDomainQemuAgentCommand(virDomainPtr domain, if (virDomainQemuAgentCommandEnsureACL(domain->conn, vm->def) < 0) goto cleanup;
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); - goto cleanup; + goto endjob; }
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - if (!qemuDomainAgentAvailable(vm, true)) goto endjob;
There's another check for domain activeness just below this line. It can be safely removed. ACK Michal
On Tue, Jun 14, 2016 at 10:13:13AM +0200, Michal Privoznik wrote:
On 13.06.2016 15:33, Martin Kletzander wrote:
Since obtaining a job can wait for another job to finish, the state might change in the meantime. And checking it more than once is pointless.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index efb3f854fb10..e677440c20b2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
@@ -17942,15 +17936,15 @@ qemuDomainQemuAgentCommand(virDomainPtr domain, if (virDomainQemuAgentCommandEnsureACL(domain->conn, vm->def) < 0) goto cleanup;
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); - goto cleanup; + goto endjob; }
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; - if (!qemuDomainAgentAvailable(vm, true)) goto endjob;
There's another check for domain activeness just below this line. It can be safely removed.
And the same thing happened in qemuDomainGetVcpusFlags(), so I fixed it there as well and pushed. Thanks.
ACK
Michal
participants (2)
-
Martin Kletzander -
Michal Privoznik