[libvirt] [PATCH v2] storage:add lvchange operation while lvremove fails.

Osier Yang jyang at redhat.com
Tue Nov 22 07:06:37 UTC 2011


On 2011年11月01日 11:01, zituan at taobao.com wrote:
> From: Chang Liu<lingjiao.lc at taobao.com>
>
> We found an issue that virStorageBackendLogicalDeleteVol() could not remove
> the lv with notificaton "could not remove open logical volume.", in such
> situation, we should disable the lv first, then delete it. this patch fix it.
>
> *src/storage/storage_backend_logical.c
> (virStorageBackendLogicalDeleteVol):lvremove fail, lvchange the volume
> and then lvremove it second.
> ---
>   configure.ac                          |    4 +++
>   src/storage/storage_backend_logical.c |   39 ++++++++++++++++++++++++++------
>   2 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 5753c08..6092c47 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1691,6 +1691,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
>     AC_PATH_PROG([PVREMOVE], [pvremove], [], [$PATH:/sbin:/usr/sbin])
>     AC_PATH_PROG([VGREMOVE], [vgremove], [], [$PATH:/sbin:/usr/sbin])
>     AC_PATH_PROG([LVREMOVE], [lvremove], [], [$PATH:/sbin:/usr/sbin])
> +  AC_PATH_PROG([LVCHANGE], [lvchange], [], [$PATH:/sbin:/usr/sbin])
>     AC_PATH_PROG([VGCHANGE], [vgchange], [], [$PATH:/sbin:/usr/sbin])
>     AC_PATH_PROG([VGSCAN], [vgscan], [], [$PATH:/sbin:/usr/sbin])
>     AC_PATH_PROG([PVS], [pvs], [], [$PATH:/sbin:/usr/sbin])
> @@ -1704,6 +1705,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
>       if test -z "$PVREMOVE" ; then AC_MSG_ERROR([We need pvremove for LVM storage driver]) ; fi
>       if test -z "$VGREMOVE" ; then AC_MSG_ERROR([We need vgremove for LVM storage driver]) ; fi
>       if test -z "$LVREMOVE" ; then AC_MSG_ERROR([We need lvremove for LVM storage driver]) ; fi
> +    if test -z "$LVCHANGE" ; then AC_MSG_ERROR([We need lvchange for LVM storage driver]) ; fi
>       if test -z "$VGCHANGE" ; then AC_MSG_ERROR([We need vgchange for LVM storage driver]) ; fi
>       if test -z "$VGSCAN" ; then AC_MSG_ERROR([We need vgscan for LVM storage driver]) ; fi
>       if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage driver]) ; fi
> @@ -1716,6 +1718,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
>       if test -z "$PVREMOVE" ; then with_storage_lvm=no ; fi
>       if test -z "$VGREMOVE" ; then with_storage_lvm=no ; fi
>       if test -z "$LVREMOVE" ; then with_storage_lvm=no ; fi
> +    if test -z "$LVCHANGE" ; then with_storage_lvm=no ; fi
>       if test -z "$VGCHANGE" ; then with_storage_lvm=no ; fi
>       if test -z "$VGSCAN" ; then with_storage_lvm=no ; fi
>       if test -z "$PVS" ; then with_storage_lvm=no ; fi
> @@ -1733,6 +1736,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then
>       AC_DEFINE_UNQUOTED([PVREMOVE],["$PVREMOVE"],[Location of pvremove program])
>       AC_DEFINE_UNQUOTED([VGREMOVE],["$VGREMOVE"],[Location of vgremove program])
>       AC_DEFINE_UNQUOTED([LVREMOVE],["$LVREMOVE"],[Location of lvremove program])
> +    AC_DEFINE_UNQUOTED([LVCHANGE],["$LVCHANGE"],[Location of lvchange program])
>       AC_DEFINE_UNQUOTED([VGCHANGE],["$VGCHANGE"],[Location of vgchange program])
>       AC_DEFINE_UNQUOTED([VGSCAN],["$VGSCAN"],[Location of vgscan program])
>       AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program])
> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index 3c3e736..f84c068 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -768,18 +768,41 @@ virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                     virStorageVolDefPtr vol,
>                                     unsigned int flags)
>   {
> -    const char *cmdargv[] = {
> -        LVREMOVE, "-f", vol->target.path, NULL
> -    };
> +    int ret = -1;
>
> +    virCommandPtr lvchange_cmd = NULL;
> +    virCommandPtr lvremove_cmd = NULL;
> +
>       virCheckFlags(0, -1);
> -
> +
>       virFileWaitForDevices();
> +
> +    lvchange_cmd = virCommandNewArgList(LVCHANGE,
> +                                        "-aln",
> +                                        vol->target.path,
> +                                        NULL);
> +
> +    lvremove_cmd = virCommandNewArgList(LVREMOVE,
> +                                        "-f",
> +                                        vol->target.path,
> +                                        NULL);
> +
> +    if (virCommandRun(lvremove_cmd, NULL)<  0)
> +    {
> +        if(virCommandRun(lvchange_cmd, NULL)<  0)
> +            goto cleanup;
> +        else{
> +            if(virCommandRun(lvremove_cmd,NULL)<  0)
> +                goto cleanup;
> +        }
> +    }

Coding style nits here. libvirt prefers:

if (true) {
}

And things like:

if (true)
dummy();
else {
foo();
bar();
}

is not allowed.

>
> -    if (virRun(cmdargv, NULL)<  0)
> -        return -1;
> -
> -    return 0;
> +    ret = 0;
> +
> +cleanup:
> +    virCommandFree(lvchange_cmd);
> +    virCommandFree(lvremove_cmd);
> +    return ret;
>   }
>
>

ACK with the nits fixed.

Thanks
Osier




More information about the libvir-list mailing list