On 07/14/2017 11:07 AM, Pavel Hrdina wrote:
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@redhat.com> --- src/storage/storage_driver.c | 1158 +++++++++++++++++++++--------------------- src/storage/storage_driver.h | 4 +- 2 files changed, 582 insertions(+), 580 deletions(-)
I have had some more time to think about the other comment regarding whether @obj should be @poolObj or @poolobj... If some day in the future (as noted in the patch 7 response) the @pools changes to @poolobjs it'll be eye test in order to spot the difference between @poolobj and @poolobjs, so I'd like to keep it as @obj. Long time ago I had the sheer joy of trying to search code for 'l' and '1' as well as 'O' and '0'
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
[...] lots to cut to first comment I saw [...]
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
Works for me - consider it done. John