Re: [lvm-devel] vgremove -f option add

[Dave Wysochanski <dwysocha redhat com>]

On Thu, 2007-08-23 at 16:56 -0400, Dave Wysochanski wrote:

> I did some cleanup work first and I've attached an updated patch against
> the latest code.  Patch was tested against a simple cluster as well as
> single node and seems to work fine.  Some sample output in a clustered
> environment:
> 
> # lvs
>   LV       VG         Attr   LSize  Origin Snap%  Move Log Copy% 
>   LogVol00 VolGroup00 -wi-ao  5.84G                              
>   LogVol01 VolGroup00 -wi-ao  1.03G                              
>   lv0      vg0        -wi-a- 96.00M                              
> # vgs
>   VG         #PV #LV #SN Attr   VSize   VFree  
>   VolGroup00   1   2   0 wz--n-   6.91G  32.00M
>   vg0          5   1   0 wz--nc 240.00M 144.00M
> # vgremove vg0
> Do you really want to remove volume group "vg0" with active logical volumes? [y/n]: n
>   Volume group "vg0" not removed
> # vgremove -f vg0
>   Error locking on node rhel4u5-node1: Volume is busy on another node
>   Can't get exclusive access to volume "lv0"
> # vgremove vg0
> Do you really want to remove volume group "vg0" with active logical volumes? [y/n]: y
> Do you really want to remove active logical volume "lv0"? [y/n]: y
>   Error locking on node rhel4u5-node1: Volume is busy on another node
>   Can't get exclusive access to volume "lv0"

Updated patch attached that fixes the above case.  Now we can remove LVs
that may be active on other nodes in the cluster:
[root rhel4u5-node1 LVM2]# ./tools/lvm vgremove vg0
Do you really want to remove volume group "vg0" containing 1 logical
volumes? [y/n]: y
  Error locking on node rhel4u5-node1: Volume is busy on another node
Logical volume "lv0" is active on other cluster nodes.  Really remove?
[y/n]: y
  Logical volume "lv0" successfully removed
  Volume group "vg0" successfully removed

The new check seems ok even when another node has the LV open (e.g.
mounted) - the remove just fails at the deactivate_lv() call for the
node(s) that have it open:
[root rhel4u5-node1 LVM2]# ./tools/lvm vgremove vg0
Do you really want to remove volume group "vg0" containing 1 logical
volumes? [y/n]: y
  Error locking on node rhel4u5-node1: Volume is busy on another node
Logical volume "lv0" is active on other cluster nodes.  Really remove?
[y/n]: y
  Error locking on node rhel4u5-node3: LV vg0/lv0 in use: not
deactivating
  Unable to deactivate logical volume "lv0"


Also cleaned up the first message "Do you really want to remove..." -
got rid of "active" word (at that point, we don't know any LVs are
active) and printed the number of LVs in the VG.


Index: lib/metadata/lv_manip.c
===================================================================
RCS file: /cvs/lvm2/LVM2/lib/metadata/lv_manip.c,v
retrieving revision 1.127
diff -u -r1.127 lv_manip.c
--- lib/metadata/lv_manip.c	22 Aug 2007 14:38:17 -0000	1.127
+++ lib/metadata/lv_manip.c	24 Aug 2007 16:59:02 -0000
@@ -1839,37 +1839,49 @@
 
 	/* FIXME Ensure not referred to by another existing LVs */
 
-	if (lv_info(cmd, lv, &info, 1)) {
-		if (info.open_count) {
-			log_error("Can't remove open logical volume \"%s\"",
-				  lv->name);
-			return 0;
-		}
-
-		if (info.exists && (force == PROMPT)) {
-			if (yes_no_prompt("Do you really want to remove active "
-					  "logical volume \"%s\"? [y/n]: ",
-					  lv->name) == 'n') {
-				log_print("Logical volume \"%s\" not removed",
-					  lv->name);
-				return 0;
-			}
-		}
+	/*
+	 * If we can't get information about the LV from the kernel, or
+	 * someone has the LV device open, fail.
+	 */
+	if (!lv_info(cmd, lv, &info, 1)) {
+		log_error("Unable to obtain status for logical volume \"%s\"",
+			  lv->name);
+		return 0;
 	}
-
-	if (!archive(vg))
+	if (info.open_count) {
+		log_error("Can't remove open logical volume \"%s\"",
+			  lv->name);
 		return 0;
+	}
 
-	/* If the VG is clustered then make sure no-one else is using the LV
-	   we are about to remove */
-	if (vg_status(vg) & CLUSTERED) {
-		if (!activate_lv_excl(cmd, lv)) {
-			log_error("Can't get exclusive access to volume \"%s\"",
+	/*
+	 * Check for confirmation prompts in the following cases:
+	 * 1) Clustered VG, and some remote nodes have the LV active
+	 * 2) Non-clustered VG, but LV active locally
+	 */
+	if ((vg_status(vg) & CLUSTERED) && !activate_lv_excl(cmd, lv) &&
+	    (force == PROMPT)) {
+		if (yes_no_prompt("Logical volume \"%s\" is active on other "
+				  "cluster nodes.  Really remove? [y/n]: ",
+				  lv->name) == 'n') {
+			log_print("Logical volume \"%s\" not removed",
 				  lv->name);
 			return 0;
 		}
+	} else if (info.exists && (force == PROMPT)) {
+		 if (yes_no_prompt("Do you really want to remove active "
+				   "logical volume \"%s\"? [y/n]: ",
+				   lv->name) == 'n') {
+			 log_print("Logical volume \"%s\" not removed",
+				  lv->name);
+			 return 0;
+		 }
 	}
 
+
+	if (!archive(vg))
+		return 0;
+
 	/* FIXME Snapshot commit out of sequence if it fails after here? */
 	if (!deactivate_lv(cmd, lv)) {
 		log_error("Unable to deactivate logical volume \"%s\"",
Index: lib/metadata/metadata.c
===================================================================
RCS file: /cvs/lvm2/LVM2/lib/metadata/metadata.c,v
retrieving revision 1.134
diff -u -r1.134 metadata.c
--- lib/metadata/metadata.c	22 Aug 2007 14:38:17 -0000	1.134
+++ lib/metadata/metadata.c	24 Aug 2007 16:59:02 -0000
@@ -249,6 +249,20 @@
 	return 1;
 }
 
+static int remove_lvs_in_vg(struct cmd_context *cmd,
+			    struct volume_group *vg,
+			    force_t force)
+{
+	struct lv_list *lvl;
+
+	list_iterate_items(lvl, &vg->lvs)
+		if (!lv_remove_single(cmd, lvl->lv, force))
+			return 0;
+
+	return 1;
+}
+
+/* FIXME: remove redundant vg_name */
 int vg_remove_single(struct cmd_context *cmd, const char *vg_name,
 		     struct volume_group *vg, int consistent,
 		     force_t force __attribute((unused)))
@@ -269,6 +283,19 @@
 		return 0;
 
 	if (vg->lv_count) {
+		if ((force == PROMPT) &&
+		    (yes_no_prompt("Do you really want to remove volume "
+				   "group \"%s\" containing %d "
+				   "logical volumes? [y/n]: ",
+				   vg_name, vg->lv_count) == 'n')) {
+			log_print("Volume group \"%s\" not removed", vg_name);
+			return 0;
+		}
+		if (!remove_lvs_in_vg(cmd, vg, force))
+			return 0;
+	}
+	
+	if (vg->lv_count) {
 		log_error("Volume group \"%s\" still contains %d "
 			  "logical volume(s)", vg_name, vg->lv_count);
 		return 0;
Index: man/vgremove.8
===================================================================
RCS file: /cvs/lvm2/LVM2/man/vgremove.8,v
retrieving revision 1.4
diff -u -r1.4 vgremove.8
--- man/vgremove.8	18 Nov 2004 19:45:52 -0000	1.4
+++ man/vgremove.8	24 Aug 2007 16:59:02 -0000
@@ -3,17 +3,24 @@
 vgremove \- remove a volume group
 .SH SYNOPSIS
 .B vgremove
-[\-d/\-\-debug] [\-h/\-?/\-\-help] [\-t/\-\-test] [\-v/\-\-verbose]
+[\-d/\-\-debug] [\-f/\-\-force] [\-h/\-?/\-\-help]
+[\-t/\-\-test] [\-v/\-\-verbose]
 VolumeGroupName [VolumeGroupName...]
 .SH DESCRIPTION
 vgremove allows you to remove one or more volume groups.
-The volume group(s) must not have any logical volumes allocated:
-Remove them first with \fBlvremove\fP. If one or more physical
-volumes in the volume group are lost, consider 
-\fBvgreduce --removemissing\fP to make the volume group
+If one or more physical volumes in the volume group are lost,
+consider \fBvgreduce --removemissing\fP to make the volume group
 metadata consistent again.
+.sp
+If there are logical volumes that exist in the volume group,
+a prompt will be given to confirm removal.  You can override
+the prompt with \fB-f\fP.
 .SH OPTIONS
 See \fBlvm\fP for common options.
+.TP
+.BR \-f ", " \-\-force
+Force the removal of any logical volumes on the volume group
+without confirmation.
 .SH SEE ALSO
 .BR lvm (8), 
 .BR lvremove (8), 
Index: tools/commands.h
===================================================================
RCS file: /cvs/lvm2/LVM2/tools/commands.h,v
retrieving revision 1.101
diff -u -r1.101 commands.h
--- tools/commands.h	21 Aug 2007 19:46:36 -0000	1.101
+++ tools/commands.h	24 Aug 2007 16:59:03 -0000
@@ -807,13 +807,14 @@
    "Remove volume group(s)",
    "vgremove\n"
    "\t[-d|--debug]\n"
+   "\t[-f|--force]\n"
    "\t[-h|--help]\n"
    "\t[-t|--test]\n"
    "\t[-v|--verbose]\n"
    "\t[--version]" "\n"
    "\tVolumeGroupName [VolumeGroupName...]\n",
 
-   test_ARG)
+   force_ARG, test_ARG)
 
 xx(vgrename,
    "Rename a volume group",