[libvirt] [PATCH] lock qemu_driver early in qemuGetSchedulerParametersFlags()

[Eric Blake]

On 06/28/2011 04:09 AM, Michal Privoznik wrote:
> On 28.06.2011 09:58, Wen Congyang wrote:
>> If we pass VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG to
>> qemuGetSchedulerParametersFlags() or *nparams is less than 1,
>> we will unlock qemu_driver without locking it. It's very dangerous.
>>
>> We should lock qemu_driver after calling virCheckFlags().
>>
>> ---
>>  src/qemu/qemu_driver.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 768e0f2..c6994cd 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -5832,6 +5832,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom,
>>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>>                    VIR_DOMAIN_AFFECT_CONFIG, -1);
>>  
>> +    qemuDriverLock(driver);
>> +
>>      if ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) ==
>>          (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) {
>>          qemuReportError(VIR_ERR_INVALID_ARG, "%s",
>> @@ -5845,7 +5847,6 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom,
>>          goto cleanup;
>>      }

An alternative would have been to replace the 'goto cleanup' with
'return -1'.  In fact, with that alternative, we can error out without
even spending time grabbing the lock.  Then again, the error is
unexpected (you can assume that most callers are compliant), and
optimizing for the rare bad code won't have any impact to the execution
speed of good code, so I'm happy with the patch as-is.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110628/c9020b37/attachment-0001.sig>