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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list