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

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(a)redhat.com&gt; >> --- >> 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).

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.

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