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

[Daniel Veillard]

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 at redhat.com>
> ---
>  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()

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/