[libvirt] [PATCH] bhyve: fix virObjectUnlock() usage

[Eric Blake]

On 05/21/2014 08:02 AM, Roman Bogorodskiy wrote:

>>>> So, what you propose is checking explicitly for NULL in
>>>> virObjectUnlock before we do virObjectIsClass(), and if the passed
>>>> argument is NULL indeed, just return, without logging anything about
>>>> that?
>>>
>>> Yes, since we have other virObject code that special cases NULL (for
>>> example, virObjectUnref).
>>
>> IMHO passing NULL into the locking APIs is a coding error we shouldn't
>> try to paper over by ignoring.
>>
>> Ultimately I think the real flaw is the way we obtain the virDomainPtr
>> pointers in the first place. ie the virDomainObjListLookup functions
>> don't acquire a reference on the object they return. So the caller has
>> to worry about the object being released behind their back, hence all
>> our logic which has to set 'vm = NULL' in various places where it might
>> have been deleted.
>>
>> IOW, I'd much rather we looked at changing our design here so that we
>> didn't have so much NULL vm pointers in the first place.
> 
> Eric, could you please comment on that?

I trust Daniel's judgment here, since he wrote virObjectPtr and
virObjectUnlock.  Cleaning up the DomainObjListLookup function to grab a
reference will have a big ripple effect, so it probably doesn't need to
be done now (that is, you don't necessarily have to be the one doing the
cleanup he proposes); but I guess that means for the present that we are
still stuck with the current design pattern of checking for NULL ourself
before calling virObjectUnlock.

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

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