On 2011年11月01日 11:01, zituan(a)taobao.com wrote: > From: Chang Liu&lt;lingjiao.lc(a)taobao.com&gt; > > 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.