[parted-devel] [PATCH] When labeling a disk in --script mode, fail if it is in use.

Jim Meyering jim at meyering.net
Fri May 11 12:40:57 UTC 2007


Changing parted itself was easy.
Writing a relatively safe test to exercise it required
some careful thought and engineering.

To run the new test, you can use a command like this:

  cd tests
  sudo env DEVICE_TO_ERASE=/dev/sde DEVICE_TO_ERASE_SIZE=999MB \
    ./t1100-busy-label.sh

where /dev/sde is the device you're prepared to erase
(the entire device, not just a single partition).  I use
a 1GB flash card whose size "parted -s /dev/sde print" reports
as "999MB".

You can also run "make check" as root, but I would not recommend
that (potentially dangerous).  Instead, I'll write a new Makefile
rule that will run only the root-requiring tests.

The string used in $DEVICE_TO_ERASE_SIZE must match exactly what
is output by the parted print command, otherwise the test will
be skipped.  Similarly, the device must be accessible but not mounted.

	When labeling a disk in --script mode, fail if it is in use.
	* parted/parted.c (_disk_warn_busy): In script mode,
	throw a "PED_EXCEPTION_ERROR", not a warning.
	(do_mklabel): Guard only the _disk_warn_loss call with
	"if (!opt_script_mode...", not the _disk_warn_loss call.
	* tests/t1100-busy-label.sh: New file.  Test the above,
	in interactive mode as well as in script mode.  Requires
	root privilege (to mount a fs), and an actual block device.
	* tests/Makefile.am (TESTS): Add t1100-busy-label.sh.
	* tests/test-lib.sh: Add infrastructure to support new
	privileges_required_=1 and erasable_device_required_=1
	settings used by t1100.

Signed-off-by: Jim Meyering <jim at meyering.net>
---
 parted/parted.c           |   14 +++---
 tests/Makefile.am         |    1 +
 tests/t1100-busy-label.sh |   84 +++++++++++++++++++++++++++++++++++++++
 tests/test-lib.sh         |   97 ++++++++++++++++++++++++++++++++++----------
 4 files changed, 167 insertions(+), 29 deletions(-)
 create mode 100644 tests/t1100-busy-label.sh

diff --git a/parted/parted.c b/parted/parted.c
index d16ad96..c7b3e22 100644
--- a/parted/parted.c
+++ b/parted/parted.c
@@ -206,7 +206,9 @@ _disk_warn_busy (PedDisk* disk)
 {
         if (ped_device_is_busy (disk->dev))
                 return ped_exception_throw (
-                        PED_EXCEPTION_WARNING,
+                        (opt_script_mode
+                         ? PED_EXCEPTION_ERROR
+                         : PED_EXCEPTION_WARNING),
                         PED_EXCEPTION_IGNORE_CANCEL,
                         _("Partition(s) on %s are being used."),
                         disk->dev->path) == PED_EXCEPTION_IGNORE;
@@ -602,12 +604,10 @@ do_mklabel (PedDevice** dev)
         ped_exception_leave_all ();
 
         if (disk) {
-                if (!opt_script_mode) {
-                        if (!_disk_warn_busy (disk))
-                                goto error_destroy_disk;
-                        if (!_disk_warn_loss (disk))
-                                goto error_destroy_disk;
-                }
+                if (!_disk_warn_busy (disk))
+                        goto error_destroy_disk;
+                if (!opt_script_mode && !_disk_warn_loss (disk))
+                        goto error_destroy_disk;
 
                 ped_disk_destroy (disk);
         }
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1d2a043..df18eb4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,6 +1,7 @@
 TESTS = \
   t0000-basic.sh \
   t1000-mkpartfs.sh \
+  t1100-busy-label.sh \
   t2000-mkfs.sh
 
 TESTS_ENVIRONMENT = \
diff --git a/tests/t1100-busy-label.sh b/tests/t1100-busy-label.sh
new file mode 100644
index 0000000..a120c68
--- /dev/null
+++ b/tests/t1100-busy-label.sh
@@ -0,0 +1,84 @@
+#!/bin/sh
+
+# Copyright (C) 2007 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# 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., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+test_description='partitioning (parted -s $dev mklabel) a busy disk must fail.'
+
+privileges_required_=1
+erasable_device_required_=1
+
+. ./init.sh
+dev=$DEVICE_TO_ERASE
+
+test_expect_success \
+    "setup: create a fat32 file system on $dev" \
+    'dd if=/dev/zero of=$dev bs=1k count=1 2> /dev/null &&
+     parted -s $dev mklabel msdos                > out 2>&1 &&
+     parted -s $dev mkpartfs primary fat32 1 40 >> out 2>&1'
+test_expect_success 'expect no output' '$compare out /dev/null'
+
+mount_point="`pwd`/mnt"
+
+# Be sure to unmount upon interrupt, failure, etc.
+cleanup_() { umount ${dev}1 > /dev/null 2>&1; }
+
+# There's a race condition here: on udev-based systems, ${dev}1
+# is not created immediately, and without some delay, this mount
+# command would fail.  Using a flash card as $dev, the loop below
+# typically iterates 7-20 times.
+test_expect_success \
+    'create mount point dir. and mount the just-created partition on it' \
+    'mkdir $mount_point &&
+     i=0; while :; do test -e ${dev}1 && break; test $i = 90 && break;
+	              i=$(expr $i + 1); done;
+     mount ${dev}1 $mount_point'
+
+test_expect_failure \
+    'now that a partition is mounted, mklabel attempt must fail' \
+    'parted -s $dev mklabel msdos > out 2>&1'
+test_expect_success \
+    'create expected output file' \
+    'echo "Error: Partition(s) on $dev are being used." > exp'
+test_expect_success \
+    'check for expected failure diagnostic' \
+    '$compare out exp'
+
+# ==================================================
+# Now, test it in interactive mode.
+test_expect_success 'create input file' 'echo c > in'
+test_expect_failure \
+    'as above, this mklabel attempt must fail' \
+    'parted ---pretend-input-tty $dev mklabel msdos < in > out 2>&1'
+
+fail=0
+cat <<EOF > exp || fail=1
+Warning: Partition(s) on $dev are being used.
+Ignore/Cancel? c
+EOF
+test_expect_success 'create expected output file' 'test $fail = 0'
+
+# Transform the actual output, removing ^M   ...^M.
+test_expect_success \
+    'normalize the actual output' \
+    'mv out o2 && sed "s,
   *
,,;s, $,," o2 > out'
+
+test_expect_success \
+    'check for expected failure diagnostic' \
+    '$compare out exp'
+
+test_done
diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index d6804cf..5544cfd 100644
--- a/tests/test-lib.sh
+++ b/tests/test-lib.sh
@@ -29,19 +29,11 @@ say () {
 	echo "* $*"
 }
 
+this_test_() { expr "./$0" : '.*/\(t[0-9]*\)-[^/]*$'; }
+
 test "${test_description}" != "" ||
 error "Test script did not set test_description."
 
-# If $srcdir is not set, set it, if it's ".".  Otherwise, fail.
-if test -a "$srcdir"; then
-    if test -f test-lib.sh; then
-	srcdir=.
-    else
-	error '$srcdir is not set; either set it, or run the test' \
-	    'from the source directory'
-    fi
-fi
-
 while test "$#" -ne 0
 do
 	case "$1" in
@@ -98,7 +90,7 @@ test_run_ () {
 }
 
 test_skip () {
-	this_test=$(expr "./$0" : '.*/\(t[0-9]*\)-[^/]*$')
+	this_test=$(this_test_)
 	this_test="$this_test.$(expr "$test_count" + 1)"
 	to_skip=
 	for skp in $SKIP_TESTS
@@ -193,24 +185,73 @@ test_done () {
 	esac
 }
 
+this_test=$(this_test_)
+
+skip_=0
+# If $privileges_required_ is nonempty, non-root skips this test.
+if test "$privileges_required_" != ''; then
+    uid=`id -u` || error 'failed to run "id -u"'
+    if test "$uid" != 0; then
+	SKIP_TESTS="$SKIP_TESTS $this_test"
+	say "you have insufficient privileges for test $this_test"
+	skip_=1
+    fi
+fi
+
 pwd_=`pwd`
 
-# Test the binaries we have just built.  The tests are kept in
-# t/ subdirectory and are run in trash subdirectory.
+# Test the binaries we have just built.
 PATH=$pwd_/../parted:$PATH
 export PATH
 
-t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp_=$t0/$$
-trap 'st=$?; cd "$pwd_" && chmod -R u+rwx $t0 && rm -rf $t0 && exit $st' 0
-trap '(exit $?); exit $?' 1 2 13 15
+fail=
+# Some tests require an actual hardware device, e.g., a real disk with a
+# spindle, a USB key, or a CD-RW.  If this variable is nonempty, the user
+# has properly set the $DEVICE_TO_ERASE and $DEVICE_TO_ERASE_SIZE envvars,
+# then the test will proceed.  Otherwise, it is skipped.
+if test $skip_ = 0 && test "$erasable_device_required_" != ''; then
+  # Since testing a drive with parted destroys all data on that drive,
+  # we have rather draconian safety requirements that should help avoid
+  # accidents.  If $dev_ is the name of the device,
+  # - running "parted -s $dev_ print" must succeed, and
+  # - its output must include a line matching /^Disk $dev_: $DEV_SIZE$/
+  # - Neither $dev_ nor any $dev_[0-9]* may be mounted.
+  if test "$DEVICE_TO_ERASE" != '' && test "$DEVICE_TO_ERASE_SIZE" != ''; then
+    dev_=$DEVICE_TO_ERASE
+    sz=$DEVICE_TO_ERASE_SIZE
+    parted_output=$(parted -s $dev_ print) || fail="no such device: $dev_"
+    parted -s $dev_ print|grep "^Disk $dev_: $sz$" \
+	> /dev/null || fail="actual device size is not $sz"
+    # Try to see if $dev_ or any of its partitions is mounted.
+    # This is not reliable.  FIXME: find a better way.
+    # Maybe expose parted's own test for whether a disk is in use.
+    # The following assume that $dev_ is canonicalized, e.g., that $dev_
+    # contains no "//" or "/./" components.
 
-framework_failure=0
-mkdir -p $tmp_ || framework_failure=1
-cd $tmp_ || framework_failure=1
-test $framework_failure = 0 \
-     || error 'failed to create temporary directory'
+    # Prefer df --local, if it works, so we don't waste time
+    # enumerating with lots of automounted file systems.
+    ( df --local / > /dev/null 2>&1 ) && df='df --local' || df=df
+    $df | grep "^$dev_" && fail="$dev_ is already mounted"
+    $df | grep "^$dev_[0-9]" && fail="a partition of $dev_ is already mounted"
+
+    # Skip this test and complain if anything failed.
+    if test "$fail" != ''; then
+      SKIP_TESTS="$SKIP_TESTS $this_test"
+      say "invalid setup: $fail"
+    fi
+  else
+    # Skip quietly if both envvars are not specified.
+    SKIP_TESTS="$SKIP_TESTS $this_test"
+    say 'This test requires an erasable device and you have not properly'
+    say 'set the $DEVICE_TO_ERASE and $DEVICE_TO_ERASE_SIZE envvars.'
+  fi
+fi
+
+# This is a stub function that is run upon trap (upon regular exit and
+# interrupt).  Override it with a per-test function, e.g., to unmount
+# a partition, or to undo any other global state changes.
+cleanup_() { :; }
 
-this_test=$(expr "./$0" : '.*/\(t[0-9]*\)-[^/]*$')
 for skp in $SKIP_TESTS
 do
 	to_skip=
@@ -229,6 +270,18 @@ do
 	esac
 done
 
+# Run each test from within a temporary sub-directory named after the
+# test itself, and arrange to remove it upon exception and upon normal exit.
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp_=$t0/$$
+trap 'st=$?; cleanup_; cd "$pwd_" && chmod -R u+rwx $t0 && rm -rf $t0 && exit $st' 0
+trap '(exit $?); exit $?' 1 2 13 15
+
+framework_failure=0
+mkdir -p $tmp_ || framework_failure=1
+cd $tmp_ || framework_failure=1
+test $framework_failure = 0 \
+     || error 'failed to create temporary directory'
+
 if ( diff --version < /dev/null 2>&1 | grep GNU ) 2>&1 > /dev/null; then
   compare='diff -u'
 elif ( cmp --version < /dev/null 2>&1 | grep GNU ) 2>&1 > /dev/null; then
-- 
1.5.2.rc3



More information about the parted-devel mailing list