[libvirt] [PATCH 1/2] qemu: Relax locking in DomainHasManagedSaveImage and DomainMonitorCommand

[Peter Krempa]

On 12/13/12 13:50, Peter Krempa wrote:
> On 12/13/12 12:40, Michal Privoznik wrote:
>> There is no need to hold qemu lock during the whole execution
>> of these two APIs.
>> ---
>>   src/qemu/qemu_driver.c |   12 ++++++------
>>   1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 2fda44e..74e442b 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -3288,6 +3288,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom,
>> unsigned int flags)
>>
>>       qemuDriverLock(driver);
>>       vm = virDomainFindByUUID(&driver->domains, dom->uuid);
>> +    qemuDriverUnlock(driver);
>>       if (!vm) {
>>           char uuidstr[VIR_UUID_STRING_BUFLEN];
>>           virUUIDFormat(dom->uuid, uuidstr);
>> @@ -3301,7 +3302,6 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom,
>> unsigned int flags)
>>   cleanup:
>>       if (vm)
>>           virDomainObjUnlock(vm);
>> -    qemuDriverUnlock(driver);
>>       return ret;
>>   }
>
> This is already taken care of upstream with:
> commit 2745177b346a0ebbd3629e298178c60a54af242a
> Author: Peter Krempa <pkrempa at redhat.com>
> Date:   Tue Dec 11 19:28:17 2012 +0100
>
>      qemu: Refactor managed save functions to use domain lookup helpers
>
>
>>
>> @@ -12676,6 +12676,7 @@ static int
>> qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
>>
>>       qemuDriverLock(driver);
>>       vm = virDomainFindByUUID(&driver->domains, domain->uuid);
>> +    qemuDriverUnlock(driver);
>>       if (!vm) {
>>           char uuidstr[VIR_UUID_STRING_BUFLEN];
>>           virUUIDFormat(domain->uuid, uuidstr);
>> @@ -12688,9 +12689,9 @@ static int
>> qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
>>           virReportError(VIR_ERR_OPERATION_INVALID,
>>                          "%s", _("domain is not running"));
>>           goto cleanup;
>> -   }
>> +    }
>>
>> -    if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)
>> < 0)
>> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>>           goto cleanup;
>>
>>       if (!virDomainObjIsActive(vm)) {
>> @@ -12705,9 +12706,9 @@ static int
>> qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
>>
>>       hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP);
>>
>> -    qemuDomainObjEnterMonitorWithDriver(driver, vm);
>> +    qemuDomainObjEnterMonitor(driver, vm);
>>       ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp);
>> -    qemuDomainObjExitMonitorWithDriver(driver, vm);
>> +    qemuDomainObjExitMonitor(driver, vm);
>>
>>   endjob:
>>       if (qemuDomainObjEndJob(driver, vm) == 0) {
>> @@ -12717,7 +12718,6 @@ endjob:
>>   cleanup:
>>       if (vm)
>>           virDomainObjUnlock(vm);
>> -    qemuDriverUnlock(driver);
>>       return ret;
>>   }
>
> This cleanup makes sense, but it'd be great to take advantage of the
> lookup helpers I introduced lately to decrease the complexity even more.

Ah yeah, you do that in 2/2. ACK in this case if you rebase this patch 
on the current HEAD (and thus get rid of the first two hunks.

>
>>
>>
>
> Peter
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list