LVM2/tools pvremove.c

LVM2/tools pvremove.c

[wysochanski]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	wysochanski at sourceware.org	2007-06-14 15:51:36

Modified files:
	tools          : pvremove.c 

Log message:
	Fix inverted logic on last checkin for pvremove

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/pvremove.c.diff?cvsroot=lvm2&r1=1.15&r2=1.16

--- LVM2/tools/pvremove.c	2007/06/14 15:48:04	1.15
+++ LVM2/tools/pvremove.c	2007/06/14 15:51:36	1.16
@@ -43,7 +43,7 @@
 	}
 
 	/* orphan ? */
-	if (!is_orphan(pv))
+	if (is_orphan(pv))
 		return 1;
 
 	/* Allow partial & exported VGs to be destroyed. */

LVM2/tools pvremove.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2012-03-01 21:46:31

Modified files:
	tools          : pvremove.c 

Log message:
	Replace goto with just log_debug
	
	FIXME: temporal quick hack to make things green
	There might be a better reaction.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/pvremove.c.diff?cvsroot=lvm2&r1=1.38&r2=1.39

--- LVM2/tools/pvremove.c	2012/02/28 18:08:08	1.38
+++ LVM2/tools/pvremove.c	2012/03/01 21:46:31	1.39
@@ -129,7 +129,7 @@
 	}
 
 	if (!lvmetad_pv_gone(dev->dev))
-		goto_out;
+		log_debug("PV gone"); /* FIXME any better idea */
 
 	log_print("Labels on physical volume \"%s\" successfully wiped",
 		  pv_name);

LVM2/tools pvremove.c

[Alasdair G Kergon]

On Thu, Mar 01, 2012 at 09:46:32PM -0000, zkabelac at sourceware.org wrote:
> 	Replace goto with just log_debug
> 	FIXME: temporal quick hack to make things green
> 	There might be a better reaction.

The problem here is that this code is not distinguishing between a successful
communication with the daemon but with a reply that indicates the device is
unknown - which is an error we can ignore - and a failure in the code
trying to communicate with the daemon - which should give log_error.

The second case is more important to know about than the first so until the
code can distinguish, failing with an error should take precedence.

IOW the 'reason' the daemon gives needs checking, or else it could return
something like 'not_found' rather than 'failed' so the caller can decide
whether that's OK or not.

(Other parts of the code may need a similar change. Older LVM2 lookup code
knows that 'not found' is not necessarily an error and it depends on the
context.)

Alasdair

LVM2/tools pvremove.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2012-03-02 02:55:45

Modified files:
	tools          : pvremove.c 

Log message:
	Reinstate error if e.g. daemon comms fails.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/pvremove.c.diff?cvsroot=lvm2&r1=1.39&r2=1.40

--- LVM2/tools/pvremove.c	2012/03/01 21:46:31	1.39
+++ LVM2/tools/pvremove.c	2012/03/02 02:55:45	1.40
@@ -128,8 +128,9 @@
 		goto out;
 	}
 
+	/* FIXME Avoid error if we expect that daemon might not know device */
 	if (!lvmetad_pv_gone(dev->dev))
-		log_debug("PV gone"); /* FIXME any better idea */
+		goto_out;
 
 	log_print("Labels on physical volume \"%s\" successfully wiped",
 		  pv_name);