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

[Roman Bogorodskiy]

  Eric Blake wrote:

> 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.

OK, then I'll push the current bhyve patch that was already ACKed by
Daniel and leave virObjectUnlock related cleanup for now; thanks.

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140521/401bc2f3/attachment-0001.sig>