Re: [libvirt] [PATCH 08/19] storage: Use consistent variable names for driver

On Tue, May 09, 2017 at 11:30:15AM -0400, John Ferlan wrote: > A virStoragePoolObjPtr will be an 'obj'. > > A virStoragePoolPtr will be a 'pool'. > > A virStorageVolPtr will be a 'vol'. > > A virStorageVolDefPtr will be a 'voldef'. > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/storage/storage_driver.c | 1158 +++++++++++++++++++++--------------------- > src/storage/storage_driver.h | 4 +- > 2 files changed, 582 insertions(+), 580 deletions(-) >

> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index 2103ed1..6122396 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c

> > static virStorageVolPtr > -storageVolCreateXML(virStoragePoolPtr obj, > +storageVolCreateXML(virStoragePoolPtr pool, > const char *xmldesc, > unsigned int flags) > { > - virStoragePoolObjPtr pool; > + virStoragePoolObjPtr obj; > virStorageBackendPtr backend; > virStorageVolDefPtr voldef = NULL; > - virStorageVolPtr ret = NULL, volobj = NULL; > + virStorageVolPtr vol = NULL, volobj = NULL; The @volobj should be also renamed, I would rename it like this: @ret -> @vol @volobj -> @newvol and ... > > virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); > > - if (!(pool = virStoragePoolObjFromStoragePool(obj))) > + if (!(obj = virStoragePoolObjFromStoragePool(pool))) > return NULL; > > - if (!virStoragePoolObjIsActive(pool)) { > + if (!virStoragePoolObjIsActive(obj)) { > virReportError(VIR_ERR_OPERATION_INVALID, > - _("storage pool '%s' is not active"), pool->def->name); > + _("storage pool '%s' is not active"), obj->def->name); > goto cleanup; > } > > - if ((backend = virStorageBackendForType(pool->def->type)) == NULL) > + if ((backend = virStorageBackendForType(obj->def->type)) == NULL) > goto cleanup; > > - voldef = virStorageVolDefParseString(pool->def, xmldesc, > + voldef = virStorageVolDefParseString(obj->def, xmldesc, > VIR_VOL_XML_PARSE_OPT_CAPACITY); > if (voldef == NULL) > goto cleanup; > @@ -1799,10 +1799,10 @@ storageVolCreateXML(virStoragePoolPtr obj, > goto cleanup; > } > > - if (virStorageVolCreateXMLEnsureACL(obj->conn, pool->def, voldef) < 0) > + if (virStorageVolCreateXMLEnsureACL(pool->conn, obj->def, voldef) < 0) > goto cleanup; > > - if (virStorageVolDefFindByName(pool, voldef->name)) { > + if (virStorageVolDefFindByName(obj, voldef->name)) { > virReportError(VIR_ERR_STORAGE_VOL_EXIST, > _("'%s'"), voldef->name); > goto cleanup; > @@ -1815,21 +1815,21 @@ storageVolCreateXML(virStoragePoolPtr obj, > goto cleanup; > } > > - if (VIR_REALLOC_N(pool->volumes.objs, > - pool->volumes.count+1) < 0) > + if (VIR_REALLOC_N(obj->volumes.objs, > + obj->volumes.count + 1) < 0) > goto cleanup; > > /* Wipe any key the user may have suggested, as volume creation > * will generate the canonical key. */ > VIR_FREE(voldef->key); > - if (backend->createVol(obj->conn, pool, voldef) < 0) > + if (backend->createVol(pool->conn, obj, voldef) < 0) > goto cleanup; > > - pool->volumes.objs[pool->volumes.count++] = voldef; > - volobj = virGetStorageVol(obj->conn, pool->def->name, voldef->name, > + obj->volumes.objs[obj->volumes.count++] = voldef; > + volobj = virGetStorageVol(pool->conn, obj->def->name, voldef->name, > voldef->key, NULL, NULL); > if (!volobj) { > - pool->volumes.count--; > + obj->volumes.count--; > goto cleanup; > } > > @@ -1850,24 +1850,24 @@ storageVolCreateXML(virStoragePoolPtr obj, > memcpy(buildvoldef, voldef, sizeof(*voldef)); > > /* Drop the pool lock during volume allocation */ > - pool->asyncjobs++; > + obj->asyncjobs++; > voldef->building = true; > - virStoragePoolObjUnlock(pool); > + virStoragePoolObjUnlock(obj); > > - buildret = backend->buildVol(obj->conn, pool, buildvoldef, flags); > + buildret = backend->buildVol(pool->conn, obj, buildvoldef, flags); > > VIR_FREE(buildvoldef); > > storageDriverLock(); > - virStoragePoolObjLock(pool); > + virStoragePoolObjLock(obj); > storageDriverUnlock(); > > voldef->building = false; > - pool->asyncjobs--; > + obj->asyncjobs--; > > if (buildret < 0) { > /* buildVol handles deleting volume on failure */ > - storageVolRemoveFromPool(pool, voldef); > + storageVolRemoveFromPool(obj, voldef); > voldef = NULL; > goto cleanup; > } > @@ -1875,8 +1875,8 @@ storageVolCreateXML(virStoragePoolPtr obj, > } > > if (backend->refreshVol && > - backend->refreshVol(obj->conn, pool, voldef) < 0) { > - storageVolDeleteInternal(volobj, backend, pool, voldef, > + backend->refreshVol(pool->conn, obj, voldef) < 0) { > + storageVolDeleteInternal(volobj, backend, obj, voldef, > 0, false); > voldef = NULL; > goto cleanup; > @@ -1885,35 +1885,39 @@ storageVolCreateXML(virStoragePoolPtr obj, > /* Update pool metadata ignoring the disk backend since > * it updates the pool values. > */ > - if (pool->def->type != VIR_STORAGE_POOL_DISK) { > - pool->def->allocation += voldef->target.allocation; > - pool->def->available -= voldef->target.allocation; > + if (obj->def->type != VIR_STORAGE_POOL_DISK) { > + obj->def->allocation += voldef->target.allocation; > + obj->def->available -= voldef->target.allocation; > } > > VIR_INFO("Creating volume '%s' in storage pool '%s'", > - volobj->name, pool->def->name); > - ret = volobj; > + volobj->name, obj->def->name); > + vol = volobj; > volobj = NULL; > voldef = NULL; > > cleanup: > virObjectUnref(volobj); > virStorageVolDefFree(voldef); > - if (pool) > - virStoragePoolObjUnlock(pool); > - return ret; > + if (obj) > + virStoragePoolObjUnlock(obj); > + return vol; > } > > static virStorageVolPtr > -storageVolCreateXMLFrom(virStoragePoolPtr obj, > +storageVolCreateXMLFrom(virStoragePoolPtr pool, > const char *xmldesc, > - virStorageVolPtr vobj, > + virStorageVolPtr volsrc, > unsigned int flags) > { > - virStoragePoolObjPtr pool, origpool = NULL; > + virStoragePoolObjPtr obj; > + virStoragePoolObjPtr objsrc = NULL; > virStorageBackendPtr backend; > - virStorageVolDefPtr origvol = NULL, newvol = NULL, shadowvol = NULL; > - virStorageVolPtr ret = NULL, volobj = NULL; > + virStorageVolDefPtr voldefsrc = NULL; > + virStorageVolDefPtr voldef = NULL; > + virStorageVolDefPtr shadowvol = NULL; > + virStorageVolPtr volobj = NULL; > + virStorageVolPtr vol = NULL; ... same here. Pavel