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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]