[libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking

[Michal Privoznik]

On 24.01.2013 11:13, Viktor Mihajlovski wrote:
> On 01/24/2013 10:41 AM, Michal Privoznik wrote:
>> Currently, there is no reason to hold qemu driver locked
>> throughout whole API execution. Moreover, we can use the
>> new qemuDomObjFromDomain() internal API to lookup domain then.
>> ---
>>   src/qemu/qemu_driver.c | 16 ++++------------
>>   1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 6d4c1e9..100f10b 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -2407,19 +2407,12 @@ static int qemuDomainSendKey(virDomainPtr domain,
>>           }
>>       }
>>
>> -    qemuDriverLock(driver);
>> -    vm = virDomainFindByUUID(&driver->domains, domain->uuid);
>> -    if (!vm) {
>> -        char uuidstr[VIR_UUID_STRING_BUFLEN];
>> -        virUUIDFormat(domain->uuid, uuidstr);
>> -        virReportError(VIR_ERR_NO_DOMAIN,
>> -                       _("no domain with matching uuid '%s'"), uuidstr);
>> +    if (!(vm = qemuDomObjFromDomain(domain)))
>>           goto cleanup;
>> -    }
>>
>>       priv = vm->privateData;
>>
>> -    if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)
>> < 0)
>> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>>           goto cleanup;
>>
>>       if (!virDomainObjIsActive(vm)) {
>> @@ -2428,9 +2421,9 @@ static int qemuDomainSendKey(virDomainPtr domain,
>>           goto endjob;
>>       }
>>
>> -    qemuDomainObjEnterMonitorWithDriver(driver, vm);
>> +    qemuDomainObjEnterMonitor(driver, vm);
>>       ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes);
>> -    qemuDomainObjExitMonitorWithDriver(driver, vm);
>> +    qemuDomainObjExitMonitor(driver, vm);
>>
>>   endjob:
>>       if (qemuDomainObjEndJob(driver, vm) == 0)
>> @@ -2439,7 +2432,6 @@ endjob:
>>   cleanup:
>>       if (vm)
>>           virObjectUnlock(vm);
>> -    qemuDriverUnlock(driver);
>>       return ret;
>>   }
>>
> Formally this looks good and works on my system, so I would be fine with
> it.
> I am just wondering what the criteria are for holding a long-term lock
> vs a short-time on. I.e. why would DomainSendKey be relaxed while
> DomainInjectNMI keeps the driver lock?
> 

Yes, that's another candidate.
Basically for now, everything that uses qemu driver other than:
- domain lookup
- begin/end job
- enter/exit monitor
SHOULD be using long term lock. This, however, will soon be past - I
believe I saw an e-mail from Dan saying, qemu driver locking should be
turned into R/W locks.

Michal