[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Move one misplaced driver unlock to the right function



On 01/18/2013 10:19 AM, Eric Blake wrote:
> On 01/18/2013 08:45 AM, Viktor Mihajlovski wrote:
>> This should have been commit 56fd513 already, but was missed
>> by initially. The driver unlock call in the cleanup section of
> 
> s/by //
> 
>> DomainManagedSave does actually belong to DomainSendKey.
> 
> Please also call out commit 8c5d2ba as the source of the original problem.
> 
>>

>> @@ -3174,7 +3175,6 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
>>  cleanup:
>>      if (vm)
>>          virObjectUnlock(vm);
>> -    qemuDriverUnlock(driver);
>>      VIR_FREE(name);
> 
> This is in qemuDomainManagedSave, which was not mentioned in the commit
> message of 8c5d2ba, was not touched in 56fd513, but does fix a bug
> (unlocking without owning the lock).  Now I have to go dig which commit
> introduced _this_ problem...
> 
> Found it - commit 2745177 was the culprit here.

Scratch that; I just reread the function.  qemuDomainManagedSave used
qemuDomObjFromDomainDriver(), which returns driver locked; so commit
2745177 is correct, and there is no locking bug here, and _your_ hunk
would make it wrong.

> 
> Yuck.  You _still_ haven't fixed DomainHasManagedSaveImage (which
> 8c5d2ba claimed to fix), and we've found yet another bogus commit.  We
> need a v2 of this patch that scrubs ALL of the bugs at once.

This is still true - we need a v2 of the patch that fixes all of the
bugs, it's just that there are fewer bugs than I was worried about,
because right now it looks like there is only one bogus commit, not two.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]