Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine

On 23.04.2015 10:30, Peter Krempa wrote: > On Thu, Apr 23, 2015 at 11:44:49 +0800, zhang bo wrote: >> The function qemuBuildCommandLine() may take a long time, for example >> if we configure tens of vifs for the guest, each may cost hundrands of >> milliseconds to create tap dev, senconds in total. Thus, unlock vm >> before calling it. >> >> Signed-off-by: Zhang Bo <oscar.zhangbo(a)huawei.com&gt; >> Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com&gt; >> --- >> src/qemu/qemu_process.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index 753afe8..d1aaaec 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn, >> } >> >> VIR_DEBUG("Building emulator command line"); >> + virObjectUnlock(vm); >> if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, >> priv->monJSON, priv->qemuCaps, >> migrateFrom, stdin_fd, snapshot, vmop, >> &buildCommandLineCallbacks, false, >> qemuCheckFips(), >> priv->autoNodeset, >> - &nnicindexes, &nicindexes))) >> + &nnicindexes, &nicindexes))) { >> + virObjectLock(vm); >> goto cleanup; >> + } >> + virObjectLock(vm); > > Why do you need to unlock the object? The VM is starting at this point > so you won't be able to save any time since APIs will either be blocked > by a job or by the fact that the VM was not started. Not true. We still allow QUERY jobs, or APIs that lock a domain object but don't necessarily take a job. For instance, if in one thread I'm doing virDomainGetOSType(), virDomainGetInfo(), ... (essentially 'virsh dominfo') while the other just starts the domain. I don't see a reason why virDomainGetOSType() should wait for the domain to be started up. Domain state should have no affect on the guest OS type, should it?

On the other hand, I don't think we can just lock and unlock the domain object as we please. qemuBuildCommandLine is a very complex function and as such it calls many others. Some of them may rely on the fact, that the object is locked by caller.