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

[Michal Privoznik]

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 at huawei.com>
>> Signed-off-by: Zhou Yimin <zhouyimin at huawei.com>
>> ---
>>  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.

Michal