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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
