[libvirt] [PATCH] storage: lvm: use correct lv* command parameters

[Osier Yang]

On 04/13/2012 07:50 PM, Cole Robinson wrote:
> lvcreate want's the parent pool's name, not the pool path
> lvchange and lvremove want lv specified as $vgname/$lvname
>
> This largely worked before because these commands strip off a
> starting /dev. But https://bugzilla.redhat.com/show_bug.cgi?id=714986
> is from a user using a 'nested VG' that was having problems.
>
> I couldn't find any info on nested LVM and the reporter never responded,
> but I reproduced with XML that specified a valid source name, and
> set target path to a symlink.
>
> Signed-off-by: Cole Robinson<crobinso at redhat.com>
> ---
>   src/storage/storage_backend_logical.c |   21 +++++++++++----------
>   1 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index 6a235f6..9a91dd9 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -672,7 +672,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
>       char size[100];
>       const char *cmdargvnew[] = {
>           LVCREATE, "--name", vol->name, "-L", size,
> -        pool->def->target.path, NULL
> +        pool->def->source.name, NULL

This makes sense.

>       };
>       const char *cmdargvsnap[] = {
>           LVCREATE, "--name", vol->name, "-L", size,
> @@ -778,23 +778,23 @@ virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                     unsigned int flags)
>   {
>       int ret = -1;
> +    char *volpath = NULL;
>
>       virCommandPtr lvchange_cmd = NULL;
>       virCommandPtr lvremove_cmd = NULL;
>
>       virCheckFlags(0, -1);
>
> -    virFileWaitForDevices();
> +    if (virAsprintf(&volpath, "%s/%s",
> +                    pool->def->source.name, vol->name)<  0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
>
> -    lvchange_cmd = virCommandNewArgList(LVCHANGE,
> -                                        "-aln",
> -                                        vol->target.path,
> -                                        NULL);
> +    virFileWaitForDevices();
>
> -    lvremove_cmd = virCommandNewArgList(LVREMOVE,
> -                                        "-f",
> -                                        vol->target.path,
> -                                        NULL);
> +    lvchange_cmd = virCommandNewArgList(LVCHANGE, "-aln", volpath, NULL);
> +    lvremove_cmd = virCommandNewArgList(LVREMOVE, "-f", volpath, NULL);

I tried with both vol->target.path, and $vgname/$lvname, both
of them work. So do we really need these changes?

>
>       if (virCommandRun(lvremove_cmd, NULL)<  0) {
>           if (virCommandRun(lvchange_cmd, NULL)<  0) {
> @@ -807,6 +807,7 @@ virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>
>       ret = 0;
>   cleanup:
> +    VIR_FREE(volpath);
>       virCommandFree(lvchange_cmd);
>       virCommandFree(lvremove_cmd);
>       return ret;

Osier