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

Re: [libvirt] [PATCH 11/19] storage: Introduce virStoragePoolObjNew



On Thu, Jul 20, 2017 at 06:00:05PM -0400, John Ferlan wrote:
> 
> 
> On 07/11/2017 11:17 AM, Pavel Hrdina wrote:
> > On Tue, May 09, 2017 at 11:30:18AM -0400, John Ferlan wrote:
> >> Create/use a helper to perform object allocation.
> >>
> >> Signed-off-by: John Ferlan <jferlan redhat com>
> >> ---
> >>  src/conf/virstorageobj.c | 34 +++++++++++++++++++++++-----------
> >>  1 file changed, 23 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> >> index 7d6b311..0079472 100644
> >> --- a/src/conf/virstorageobj.c
> >> +++ b/src/conf/virstorageobj.c
> >> @@ -37,6 +37,27 @@
> >>  VIR_LOG_INIT("conf.virstorageobj");
> >>  
> >>  
> >> +static virStoragePoolObjPtr
> >> +virStoragePoolObjNew(virStoragePoolDefPtr def)
> >> +{
> >> +    virStoragePoolObjPtr obj;
> >> +
> >> +    if (VIR_ALLOC(obj) < 0)
> >> +        return NULL;
> >> +
> >> +    if (virMutexInit(&obj->lock) < 0) {
> >> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> +                       _("cannot initialize mutex"));
> >> +        VIR_FREE(obj);
> >> +        return NULL;
> >> +    }
> >> +    virStoragePoolObjLock(obj);
> >> +    obj->active = 0;
> >> +    obj->def = def;
> >> +    return obj;
> >> +}
> >> +
> >> +
> >>  virStoragePoolDefPtr
> >>  virStoragePoolObjGetDef(virStoragePoolObjPtr obj)
> >>  {
> >> @@ -386,24 +407,15 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
> >>          return obj;
> >>      }
> >>  
> >> -    if (VIR_ALLOC(obj) < 0)
> >> +    if (!(obj = virStoragePoolObjNew(def)))
> >>          return NULL;
> > 
> > The new virStoragePoolObjNew() doesn't have to take @def and ...
> > 
> >>  
> >> -    if (virMutexInit(&obj->lock) < 0) {
> >> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> -                       _("cannot initialize mutex"));
> >> -        VIR_FREE(obj);
> >> -        return NULL;
> >> -    }
> >> -    virStoragePoolObjLock(obj);
> >> -    obj->active = 0;
> >> -
> >>      if (VIR_APPEND_ELEMENT_COPY(pools->objs, pools->count, obj) < 0) {
> >> +        obj->def = NULL;
> > 
> > ... need to set the @obj->def to NULL and ...
> > 
> >>          virStoragePoolObjUnlock(obj);
> >>          virStoragePoolObjFree(obj);
> >>          return NULL;
> >>      }
> >> -    obj->def = def;
> > 
> > ... just keep this line as it is.
> > 
> 
> These are all changed in my local branch.
> 
> I figure as long as you're good with my most recent comments to 8/19
> that since I have ACKs now through 9/19 - I'll push those, then post a
> new version for the rest (eventually, but not right away).

That sound good to me, patches 01-09 with the fixes can be pushed.

> Hopefully that works.  I think I want to work through the NodeDev,
> Secrets, NWFilter, and Interface first. Then jump back into Storage and
> Network before perhaps one day considering domain.

Sure, the Storage and Network objects are more complex so it makes
perfect sense.

> Thanks for the reviews here though - I certainly appreciate it.

You're welcome :)

Pavel

Attachment: signature.asc
Description: Digital signature


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