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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list