Re: [libvirt] [PATCH] Make sure qemudDomainSetVcpus doesn't hang.

On Thu, Mar 11, 2010 at 10:36:28AM -0500, Chris Lalancette wrote: > The code to add job support into libvirtd caused a problem > in qemudDomainSetVcpus. In particular, a qemuDomainObjEndJob() > call was added at the end of the function, but a > corresponding qemuDomainObjBeginJob() was not. Additionally, > a call to qemuDomainObj{Enter,Exit}Monitor() was also missed > in qemudDomainHotplugVcpus(). These missing calls conspired to > cause a hang in the libvirtd process after the command was > finished. Fix this by adding the missing calls. > > Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; > --- > src/qemu/qemu_driver.c | 11 +++++++++-- > 1 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 6bfae93..ee3dbd3 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -4515,7 +4515,9 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) > if (nvcpus > vm->def->vcpus) { > for (i = vm->def->vcpus ; i < nvcpus ; i++) { > /* Online new CPU */ > + qemuDomainObjEnterMonitor(vm); > rc = qemuMonitorSetCPU(priv->mon, i, 1); > + qemuDomainObjExitMonitor(vm); > if (rc == 0) > goto unsupported; > if (rc < 0) > @@ -4526,7 +4528,9 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) > } else { > for (i = vm->def->vcpus - 1 ; i >= nvcpus ; i--) { > /* Offline old CPU */ > + qemuDomainObjEnterMonitor(vm); > rc = qemuMonitorSetCPU(priv->mon, i, 0); > + qemuDomainObjExitMonitor(vm); > if (rc == 0) > goto unsupported; > if (rc < 0) > @@ -4559,18 +4563,21 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { > vm = virDomainFindByUUID(&driver->domains, dom->uuid); > qemuDriverUnlock(driver); > > + if (qemuDomainObjBeginJob(vm) < 0) > + goto cleanup; > + > if (!vm) { > char uuidstr[VIR_UUID_STRING_BUFLEN]; > virUUIDFormat(dom->uuid, uuidstr); > qemuReportError(VIR_ERR_NO_DOMAIN, > _("no domain with matching uuid '%s'"), uuidstr); > - goto cleanup; > + goto endjob; > } > > if (!virDomainObjIsActive(vm)) { > qemuReportError(VIR_ERR_OPERATION_INVALID, > "%s", _("domain is not running")); > - goto cleanup; > + goto endjob; > } > > if (!(type = virDomainVirtTypeToString(vm->def->virtType))) { ACK, I was confused at first looking for a non-existent qemuDomainObjEndJob() call in qemudDomainHotplugVcpus() but it's actually one layer up in qemudDomainSetVcpus()