[libvirt] [PATCH] Fix hang in qemudDomainCoreDump.

[Chris Lalancette]

On 03/11/2010 11:42 AM, Daniel Veillard wrote:
> On Thu, Mar 11, 2010 at 10:36:29AM -0500, Chris Lalancette wrote:
>> Currently if you dump the core of a qemu guest with
>> qemudDomainCoreDump, subsequent commands will hang
>> up libvirtd.  This is because qemudDomainCoreDump
>> uses qemuDomainWaitForMigrationComplete, which expects
>> the qemuDriverLock to be held when it's called.  This
>> patch does the simple thing and moves the qemuDriveUnlock
>> to the end of the qemudDomainCoreDump so that the driver
>> lock is held for the entirety of the call (as it is done
>> in qemudDomainSave).  We will probably want to make the
>> lock more fine-grained than that in the future, but
>> we can fix both qemudDomainCoreDump and qemudDomainSave
>> at the same time.
>>
>> Signed-off-by: Chris Lalancette <clalance at redhat.com>
>> ---
>>  src/qemu/qemu_driver.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index ee3dbd3..49983dd 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4390,7 +4390,6 @@ static int qemudDomainCoreDump(virDomainPtr dom,
>>  
>>      qemuDriverLock(driver);
>>      vm = virDomainFindByUUID(&driver->domains, dom->uuid);
>> -    qemuDriverUnlock(driver);
>>  
>>      if (!vm) {
>>          char uuidstr[VIR_UUID_STRING_BUFLEN];
>> @@ -4401,7 +4400,7 @@ static int qemudDomainCoreDump(virDomainPtr dom,
>>      }
>>      priv = vm->privateData;
>>  
>> -    if (qemuDomainObjBeginJob(vm) < 0)
>> +    if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
>>          goto cleanup;
>>  
>>      if (!virDomainObjIsActive(vm)) {
>> @@ -4499,6 +4498,7 @@ cleanup:
>>          virDomainObjUnlock(vm);
>>      if (event)
>>          qemuDomainEventQueue(driver, event);
>> +    qemuDriverUnlock(driver);
>>      return ret;
>>  }
>>  
> 
>   Okay, I don't see why CoreDump would need to use finer-grained locking
> thank Save,

Well, my point is that we really should make both of them have finer-grained
locking, since they are potentially very long operations (think of a guest
with a large amount of memory).  But we can go back and fix them both later.

> 
>  ACK,

Thanks, I've pushed this now.

-- 
Chris Lalancette