On Fri, Apr 15, 2016 at 10:00:14AM -0400, Cole Robinson wrote:
This converts SetTime to the common obj->agent->isactive pattern (which adds a _third_ IsActive check), and documents the extra checks
I would not expect a commit with the summary 'Clarify usage' to change job handling.
--- src/qemu/qemu_driver.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f8dfa27..43f22f1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18725,9 +18725,9 @@ qemuDomainSetTime(virDomainPtr dom,
priv = vm->privateData;
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; -
This will let us report the capability error without even getting the job.
+ /* We also perform this check further down after grabbing the + job lock, but do it here too so we can throw an error for + an invalid config, before trying to talk to the guest agent */
s/trying to talk to the guest agent/acquiring the job/
if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -18746,9 +18746,18 @@ qemuDomainSetTime(virDomainPtr dom, goto endjob;
This goto would end the job before it was even started.
}
+ 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); rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync); qemuDomainObjExitAgent(vm); @@ -18756,6 +18765,7 @@ qemuDomainSetTime(virDomainPtr dom, if (rv < 0) goto endjob;
+ /* The VM may have shut down inbetween, check state again */
This pattern is so common that commenting it is pointless. Maybe the IsActive check could be moved inside qemuDomainObjExitAgent as I've done for qemuDomainObjExitMonitor. Jan
if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); -- 2.7.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list