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

[lvm-devel] [PATCH] require certain lvconvert operations to be done in 2 steps (BZ 640051)



Hi,

the attached patch disallows certain ambiguous (and buggy) invocations
of lvconvert. We have chatted on IRC with Alasdair, and the conclusion
was that it's better to disallow this (at least for now): expressing
such commands in a single request is likely to have confusing and
unexpected behaviour for many users.

(The operations that are not allowed anymore is removing and adding
devices to a mirror at the same time, while specifying the devices to be
added/removed. We do not have syntax for distinguishing which devices
should be added and which removed and even the "obvious" interpretation
has problems. That is, if a device is in mirror, remove it and if it is
not, add it -- it is not clear whether to treat removed devices as
candidates for additions, and both variants would make sense in
different contexts. It is also not clear in what order to do the
removals/additions: the best route may be different in different
scenarios again.)

I am also attaching a test for the behaviour. Turns out that even this
trivial patch had bugs (that the test caught).

Yours,
   Petr.

diff -rN -u -p old-lvconvert-nomulti/test/t-lvconvert-twostep.sh new-lvconvert-nomulti/test/t-lvconvert-twostep.sh
--- old-lvconvert-nomulti/test/t-lvconvert-twostep.sh	1970-01-01 01:00:00.000000000 +0100
+++ new-lvconvert-nomulti/test/t-lvconvert-twostep.sh	2010-11-17 19:32:05.000000000 +0100
@@ -0,0 +1,21 @@
+#!/bin/bash
+# Copyright (C) 2010 Red Hat, Inc. All rights reserved.
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+. ./test-utils.sh
+
+aux prepare_vg 4
+lvcreate -m 1 --mirrorlog disk --ig -L 1 -n mirror $vg
+not lvconvert -m 2 --mirrorlog core $vg/mirror $dev3 2>&1 | tee errs
+grep "two steps" errs
+lvconvert -m 2 $vg/mirror $dev3
+lvconvert --mirrorlog core $vg/mirror
+not lvconvert -m 1 --mirrorlog disk $vg/mirror $dev3 2>&1 | tee errs
+grep "two steps" errs
diff -rN -u -p old-lvconvert-nomulti/tools/lvconvert.c new-lvconvert-nomulti/tools/lvconvert.c
--- old-lvconvert-nomulti/tools/lvconvert.c	2010-11-17 19:32:03.000000000 +0100
+++ new-lvconvert-nomulti/tools/lvconvert.c	2010-11-17 19:32:05.000000000 +0100
@@ -1345,6 +1345,15 @@ static int _lvconvert_mirrors(struct cmd
 					     &new_mimage_count, &new_log_count))
 		return 0;
 
+        if (((old_mimage_count < new_mimage_count && old_log_count > new_log_count) ||
+             (old_mimage_count > new_mimage_count && old_log_count < new_log_count)) &&
+            lp->pv_count) {
+		log_error("The requested operation would need to both allocate and free extents:");
+		log_error("this is not supported, when specifying the physical volumes to use.");
+		log_error("Please specify the operation in two steps.");
+		return 0;
+        }
+
 	/* Nothing to do?  (Probably finishing collapse.) */
 	if ((old_mimage_count == new_mimage_count) &&
 	    (old_log_count == new_log_count) && !repair)

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