Re: [libvirt] [PATCH 2/2] storage: volume: Rework lookup of volume objects

On 06/05/2014 01:52 PM, Peter Krempa wrote: > Add a helper to do all the lookup steps and remove a ton of duplicated > code. > --- > src/storage/storage_driver.c | 292 ++++++++++--------------------------------- > 1 file changed, 69 insertions(+), 223 deletions(-) ACK > > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index c9916ff..26b2601 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c > @@ -1520,45 +1520,75 @@ storageVolDeleteInternal(virStorageVolPtr obj, > } > > > -static int > -storageVolDelete(virStorageVolPtr obj, > - unsigned int flags) > +static virStorageVolDefPtr > +virStorageVolDefFromVol(virStorageVolPtr obj, > + virStoragePoolObjPtr *pool, > + virStorageBackendPtr *backend) > { > virStorageDriverStatePtr driver = obj->conn->storagePrivateData; > - virStoragePoolObjPtr pool; > - virStorageBackendPtr backend; > virStorageVolDefPtr vol = NULL; > - int ret = -1; > + > + *pool = NULL; > + if (backend) > + *backend = NULL; Initializing *pool here might be useful if someone decides to rearrange the code again, since it's used in the cleanup section, but I don't think we need to initialize backend - the caller should only use the values if this function returns non-NULL.

> > storageDriverLock(driver); > - pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); > + *pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); > storageDriverUnlock(driver); > > - if (!pool) { > + if (!*pool) { > virReportError(VIR_ERR_NO_STORAGE_POOL, > _("no storage pool with matching name '%s'"), > obj->pool); > - goto cleanup; > + return NULL; > } > > + > + > +static int > +storageVolDelete(virStorageVolPtr obj, > + unsigned int flags) > +{ > + virStoragePoolObjPtr pool; > + virStorageBackendPtr backend; > + virStorageVolDefPtr vol = NULL; > + int ret = -1; > + > + if (!(vol = virStorageVolDefFromVol(obj, &pool, &backend))) > + goto cleanup; You can return -1 here.

> + > if (virStorageVolDeleteEnsureACL(obj->conn, pool->def, vol) < 0) > goto cleanup; > Jan