[libvirt] [PATCH v2 7/8] util: Add magic number check for object validity

The virObjectIsClass API has only ever checked object validity based on if the @obj is not NULL and it was derived from some class. While this has worked well in general, there is one additional check that could be made prior to calling virClassIsDerivedFrom which loops through the classes

[libvirt] [PATCH v2 8/8] util: Add object checking for virObject{Ref|Unref}

Rather than assuming that what's passed to virObject{Ref|Unref} would be a virObjectPtr as long as it's not NULL, let's do the similar checks virObjectIsClass in order to prevent a possible increment or decrement to some field at the obj->u.s.refs offset. Signed-off-by: John Ferlan

[libvirt] [PATCH v2 2/8] util: Introduce and use virObjectRWLockWrite

Instead of making virObjectLock be the entry point for two different types of locks, let's create a virObjectRWLockWrite API which will only handle the virObjectRWLockableClass objects. Use the new virObjectRWLockWrite for the virdomainobjlist code in order to handle the Add, Remove, Rename, and

[libvirt] [PATCH v2 3/8] util: Only have virObjectLock handle virObjectLockable

Now that virObjectRWLockWrite exists to handle the virObjectRWLockable objects, let's restore virObjectLock to only handle virObjectLockable class locks. There still exists the possibility that the input @anyobj isn't a valid object and the resource isn't truly locked, but that also exists before

[libvirt] [PATCH v2 5/8] util: Introduce and use virObjectRWUnlock

Rather than overload virObjectUnlock as commit id '77f4593b' has done, create a separate virObjectRWUnlock API that will force the consumers to make the proper decision regarding unlocking the RWLock's. Similar to the RWLockRead and RWLockWrite, use the virObjectGetRWLockableObj helper. This

[libvirt] [PATCH v2 4/8] util: Introduce virObjectGetRWLockableObj

Introduce a helper to handle the error path more cleanly. The same as virObjectGetLockableObj in order to essentially follow the original logic of commit 'b545f65d' to ensure that the input argument at least has some validity before using. Signed-off-by: John Ferlan ---

[libvirt] [PATCH v2 6/8] util: Create common error path for invalid object

If virObjectIsClass fails "internally" to virobject.c, create a macro to generate the VIR_WARN describing what the problem is. Also improve the checks and message a bit to indicate which was the failure - whether the obj was NULL or just not the right class Signed-off-by: John Ferlan

[libvirt] [PATCH v2 0/8] Some virObjectRW* adjustments

v1: https://www.redhat.com/archives/libvir-list/2017-July/msg01313.html Differences from v1: * Use the names virObjectRWLockRead, virObjectRWLockWrite and virObjectRWUnlock * Instead of an 'int' return, make the virObjectRWLock{Read|Write} be void returns just like

[libvirt] [PATCH v2 1/8] util: Rename virObjectLockRead to virObjectRWLockRead

Since the class it represents is based on virObjectRWLockableClass and in order to make sure we diffentiate lest anyone somehow believes they could use virObjectLockRead for a virObjectLockableClass, let's rename the API to use the RW in the name. Besides the RW locks refer to

Re: [libvirt] [PATCH 0/7] Misc improvements

On Mon, Jul 31, 2017 at 10:26:30AM +0100, Daniel P. Berrange wrote: On Mon, Jul 31, 2017 at 08:58:41AM +0200, Martin Kletzander wrote: On Fri, Jul 28, 2017 at 04:58:57PM +0100, Daniel P. Berrange wrote: > On Fri, Jul 28, 2017 at 11:47:56AM -0400, John Ferlan wrote: > > > > > > On 07/28/2017

Re: [libvirt] [libvirt-php PATCH 0/7] add bindings for NWFilter APIs

On Mon, 2017-07-31 at 12:35 +0200, Michal Privoznik wrote: > On 06/27/2017 03:45 PM, Dawid Zamirski wrote: > > Dawid - any progress on this? I'd like to make the release as > requested. > For that the NWFilter usage example should be enough. The split of > libvirt-php.c is rather a big change and

Re: [libvirt] [PATCH 7/7] util: Add safety net of checks to ensure valid object

[...] > > I really don't think these changes are a positive move. > > If you have code that is passing in something that is not a valid object, > then silently doing nothing in virObjectRef / virObjectIsClass is not > going to make the code any more correct. In fact you're turning something >

Re: [libvirt] [PATCH 1/7] util: Alter virObjectLockRead to return status

[...] >> --- a/src/util/virobject.c >> +++ b/src/util/virobject.c >> @@ -410,17 +410,21 @@ virObjectLock(void *anyobj) >> * The object must be unlocked before releasing this >> * reference. >> */ >> -void >> +int > > I'm NACK on this return value change. > OK - that's fine. >>

Re: [libvirt] [libvirt-php PATCH 0/7] add bindings for NWFilter APIs

On 06/27/2017 03:45 PM, Dawid Zamirski wrote: > On Sat, 2017-06-24 at 19:35 +0200, Michal Privoznik wrote: >> On 06/23/2017 10:58 AM, Michal Privoznik wrote: >>> >>> >>> But this got me thinking, should we follow libvirt's example and >>> finally >>> split src/libvirt-php.c into smaller files

Re: [libvirt] [PATCH 7/7] util: Add safety net of checks to ensure valid object

On Fri, Jul 28, 2017 at 12:39:01PM -0400, John Ferlan wrote: > The virObject logic "assumes" that whatever is passed to its API's > would be some sort of virObjectPtr; however, if it is not then some > really bad things can happen. > > So far there's been only virObject{Ref|Unref},

Re: [libvirt] [PATCH 2/7] util: Introduce and use virObjectLockWrite

On Fri, Jul 28, 2017 at 12:38:56PM -0400, John Ferlan wrote: > Instead of making virObjectLock be the entry point for two > different types of locks, let's create a virObjectLockWrite API > which will be able to return status and force the (new) consumers > of the RWLock to make sure the lock is

Re: [libvirt] [PATCH 4/7] util: Introduce virObjectGetRWLockableObj

On Fri, Jul 28, 2017 at 12:38:58PM -0400, John Ferlan wrote: > Introduce a helper to handle the error path more cleanly. The same > as virObjectGetLockableObj. > > Signed-off-by: John Ferlan > --- > src/util/virobject.c | 51

Re: [libvirt] [PATCH 3/7] util: Only have virObjectLock handle virObjectLockable

On Fri, Jul 28, 2017 at 12:38:57PM -0400, John Ferlan wrote: > Now that virObjectLockWrite exists to handle the virObjectRWLockable > objects, let's restore virObjectLock to only handle virObjectLockable > type locks. There still exists the possibility that the input @anyobj > isn't a valid object

Re: [libvirt] [PATCH 1/7] util: Alter virObjectLockRead to return status

On Fri, Jul 28, 2017 at 12:38:55PM -0400, John Ferlan wrote: > Rather than ignore errors, let's have virObjectLockRead check for > the correct usage and issue an error when not properly used so > so that we don't run into situations where the resource we think > we're locking really isn't locked

Re: [libvirt] [PATCH 0/7] Misc improvements

On Mon, Jul 31, 2017 at 08:58:41AM +0200, Martin Kletzander wrote: > On Fri, Jul 28, 2017 at 04:58:57PM +0100, Daniel P. Berrange wrote: > > On Fri, Jul 28, 2017 at 11:47:56AM -0400, John Ferlan wrote: > > > > > > > > > On 07/28/2017 11:24 AM, Daniel P. Berrange wrote: > > > > On Fri, Jul 28,

Re: [libvirt] [PATCH v2] tools: virsh: Adding unix socket support to 'domdisplay' command.

On 07/28/2017 11:49 PM, Julio Faracco wrote: > This commit adds the unix socket URL support to 'domdisplay' command. > Before, even if an user was using unix socket to define a spice graphics, > the command 'domdisplay' showed that the settings were not supported. Now, > the command shows the

Re: [libvirt] [PATCH 1/7] util: Alter virObjectLockRead to return status

On 07/28/2017 08:26 PM, John Ferlan wrote: > > > On 07/28/2017 12:56 PM, Pavel Hrdina wrote: >> On Fri, Jul 28, 2017 at 12:38:55PM -0400, John Ferlan wrote: >>> Rather than ignore errors, let's have virObjectLockRead check for >>> the correct usage and issue an error when not properly used so

Re: [libvirt] [PATCH 5/7] util: Introduce and use virObjectRWUnlock

On 07/28/2017 07:07 PM, Pavel Hrdina wrote: > On Fri, Jul 28, 2017 at 12:38:59PM -0400, John Ferlan wrote: >> Rather than overload virObjectUnlock as commit id '77f4593b' has >> done, create a separate virObjectRWUnlock API that will force the >> consumers to make the proper decision regarding

Re: [libvirt] Availability of libvirt-3.6.0 Release Candidate 2

On Mon, Jul 31, 2017 at 10:11:14 +0800, Daniel Veillard wrote: >It is out, I have tagged it in git and pushed the signed tarball and rpms > to the usual place: > ftp://libvirt.org/libvirt/ > > Seems to work for me, Jenkins also complains about libvirt-master-build > having > failed but

Re: [libvirt] [PATCH 0/7] Misc improvements

On Fri, Jul 28, 2017 at 04:58:57PM +0100, Daniel P. Berrange wrote: On Fri, Jul 28, 2017 at 11:47:56AM -0400, John Ferlan wrote: On 07/28/2017 11:24 AM, Daniel P. Berrange wrote: > On Fri, Jul 28, 2017 at 11:09:03AM -0400, John Ferlan wrote: >> >> >> On 07/28/2017 10:32 AM, Martin Kletzander