[libvirt] [PATCH V3]Sheepdog: Auto Adding volume and pool refresh.

Joel Simoes joel.simoes at laposte.net
Mon Feb 3 16:12:10 UTC 2014


Howto check size of **char

Thx.

> +        char *line = *(lines + i);
> +        cells = virStringSplit(line, " ", 0);
> +        size_t j;
> +        for (j = 0; *(cells + j); j++) {

You can eliminate this whole loop by checking if cells has at least two
entries and using cells[1] directly.


Le 03/02/2014 14:07, Ján Tomko a écrit :
> On 01/31/2014 01:02 PM, joel SIMOES wrote:
>> From: Joel <joel.simoes at laposte.net>
>>
>> Fix bug 1055479
>> https://bugzilla.redhat.com/show_bug.cgi?id=1055479
>> Libvirt lose sheepdogs volumes on pool refresh or restart.
>> When restarting sheepdog pool, all volumes are missing.
>> This patch add automatically all volume from the added pool.
>> ---
>>   src/storage/storage_backend_sheepdog.c | 74 +++++++++++++++++++++++++++++++++-
>>   1 file changed, 72 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
>> index a6981ce..a836f27 100644
>> --- a/src/storage/storage_backend_sheepdog.c
>> +++ b/src/storage/storage_backend_sheepdog.c
>> @@ -109,6 +109,70 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
>>       virCommandAddArgFormat(cmd, "%d", port);
>>   }
>>   
>> +static int
>> +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>> +                                       virStoragePoolObjPtr pool)
>> +{
>> +    int ret;
> int ret = -1;
>
>> +    char *output = NULL;
> 'lines' and 'cells' should be declared here as well, not below the virCommand
> calls.
>
>> +
>> +    virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL);
>> +    virStorageBackendSheepdogAddHostArg(cmd, pool);
>> +    virCommandSetOutputBuffer(cmd, &output);
>> +    ret = virCommandRun(cmd, NULL);
> No need to save the return value, see [1]
>
>> +    char** lines;
>> +    char** cells;
>> +
>> +    if (ret < 0)
>> +        goto cleanup;
>> +
>> +    ret = -1;
>> +    lines = virStringSplit(output, "\n", 0);
> The return value of virStringSplit needs to be checked for NULL.
>
>> +    size_t i;
> This should be declared at the beginning of the function.
>
>> +    for (i = 0; *(lines + i); i++) {
> lines[i] looks nicer than *(lines + i), see [2]
>
>> +        char *line = *(lines + i);
>> +        cells = virStringSplit(line, " ", 0);
>> +        size_t j;
>> +        for (j = 0; *(cells + j); j++) {
> You can eliminate this whole loop by checking if cells has at least two
> entries and using cells[1] directly.
>
>> +
>> +            char *cell = *(cells + j);
>> +            if (j == 1) {
>> +                virStorageVolDefPtr vol = NULL;
>> +                if (VIR_ALLOC(vol) < 0)
>> +                    goto cleanup;
>> +
>> +                if (VIR_STRDUP(vol->name, cell) < 0)
>> +                    goto cleanup;
>> +
>> +                vol->type = VIR_STORAGE_VOL_BLOCK;
>> +
>> +                if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0)
>> +                    goto cleanup;
>> +                pool->volumes.objs[pool->volumes.count - 1] = vol;
>> +
>> +                if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
>> +                    goto cleanup;
>> +
>> +                vol = NULL;
>> +            }
>> +
>> +            VIR_FREE(*(cells + j));
> No need to free every string separately,
>
>> +        }
>> +
>> +        VIR_FREE(cells);
> use virStringFreeList(cells) instead.
>
>> +        VIR_FREE(*(lines + i));
> Same here.
>
>> +    }
>> +
>> +
>> +    VIR_FREE(lines);
>> +    ret = 0;
>> +
>> +cleanup:
>> +    virCommandFree(cmd);
>> +    VIR_FREE(lines);
>> +    VIR_FREE(cells);
>> +    return ret;
>> +}
>>   
>>   static int
>>   virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>> @@ -122,9 +186,15 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>>       virStorageBackendSheepdogAddHostArg(cmd, pool);
>>       virCommandSetOutputBuffer(cmd, &output);
>>       ret = virCommandRun(cmd, NULL);
>> -    if (ret == 0)
>> -        ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output);
>> +    if (ret < 0)
>> +        goto cleanup;
>>   
>> +    ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output);
>> +    if (ret < 0)
>> +        goto cleanup;
> There's no need to overwrite the 'ret' variable with every call.
> if (functionCall() < 0)
>    goto cleanup;
>
> works too, since most of the functions only return 0 and -1.
>
>> +    ret = virStorageBackendSheepdogRefreshAllVol(conn, pool);
>> +
>> +cleanup:
>>       virCommandFree(cmd);
>>       VIR_FREE(output);
>>       return ret;
>>
> Jan
>
> [1] https://www.redhat.com/archives/libvir-list/2014-January/msg01232.html
> [2] https://www.redhat.com/archives/libvir-list/2014-January/msg01215.html
>




More information about the libvir-list mailing list