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

[Peter Krempa]

On 06/11/14 17:13, Ján Tomko wrote:
> 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.

You are right, I've removed it. Also by rearranging the code I was able
to remove touching of "backend" in the error: section.

> 
>>
>>      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.

Thanks; I've done so.

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

And pushed this one too. Thanks for reviewing.

Peter


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140612/072c26ab/attachment-0001.sig>