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; }

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?