[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 2011年11月01日 11:01, zituan taobao com wrote:
From: Chang Liu<lingjiao lc 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]