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

[Chris Lalancette]

On 03/11/2010 11:31 AM, Daniel Veillard wrote:
> 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()

Thanks, I've pushed this now.

-- 
Chris Lalancette