fish: Don't fail if some mountpoints in /etc/fstab are bogus (RHBZ#668574).

Fix guestfish (and other C tools) so that they ignore errors
when /etc/fstab contains bogus entries.

Update the documentation for inspect-get-mountpoints to emphasize
that callers must be aware of this when mounting the returned
values.

Add a regression test.

Update the example code ("inspect_vm") to reflect the way this
API ought to be called.

For more detail see:
https://bugzilla.redhat.com/show_bug.cgi?id=668574

diff --git a/examples/inspect_vm.c b/examples/inspect_vm.c
index 4019a01..327a566 100644 (file)
--- a/examples/inspect_vm.c
+++ b/examples/inspect_vm.c
@@ -98,8 +98,10 @@ main (int argc, char *argv[])
     qsort (mountpoints, count_strings (mountpoints) / 2, 2 * sizeof (char *),
            compare_keys_len);
     for (i = 0; mountpoints[i] != NULL; i += 2) {
-      if (guestfs_mount_ro (g, mountpoints[i+1], mountpoints[i]) == -1)
-        exit (EXIT_FAILURE);
+      /* Ignore failures from this call, since bogus entries can
+       * appear in the guest's /etc/fstab.
+       */
+      guestfs_mount_ro (g, mountpoints[i+1], mountpoints[i]);
       free (mountpoints[i]);
       free (mountpoints[i+1]);
     }

diff --git a/fish/inspect.c b/fish/inspect.c
index 28c1b88..713501e 100644 (file)
--- a/fish/inspect.c
+++ b/fish/inspect.c
@@ -111,6 +111,7 @@ inspect_mount_root (const char *root)
          compare_keys_len);
 
   size_t i;
+  size_t mount_errors = 0;
   for (i = 0; mountpoints[i] != NULL; i += 2) {
     int r;
     if (!read_only)
@@ -118,10 +119,14 @@ inspect_mount_root (const char *root)
     else
       r = guestfs_mount_ro (g, mountpoints[i+1], mountpoints[i]);
     if (r == -1)
-      exit (EXIT_FAILURE);
+      mount_errors++;
   }
 
   free_strings (mountpoints);
+
+  if (mount_errors)
+    fprintf (stderr, _("%s: some filesystems could not be mounted (ignored)\n"),
+             program_name);
 }
 
 /* This function is called only if the above function was called,

diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
index 5c6ce21..ccaf10a 100644 (file)
--- a/generator/generator_actions.ml
+++ b/generator/generator_actions.ml
@@ -907,6 +907,10 @@ This returns a hash of where we think the filesystems
 associated with this operating system should be mounted.
 Callers should note that this is at best an educated guess
 made by reading configuration files such as C</etc/fstab>.
+I<In particular note> that this may return filesystems
+which are non-existent or not mountable and callers should
+be prepared to handle or ignore failures if they try to
+mount them.
 
 Each element in the returned hashtable has a key which
 is the path of the mountpoint (eg. C</boot>) and a value

diff --git a/ocaml/examples/inspect_vm.ml b/ocaml/examples/inspect_vm.ml
index 1d370c9..44d348e 100644 (file)
--- a/ocaml/examples/inspect_vm.ml
+++ b/ocaml/examples/inspect_vm.ml
@@ -43,7 +43,11 @@ let () =
       let cmp (a,_) (b,_) =
         compare (String.length a) (String.length b) in
       let mps = List.sort cmp mps in
-      List.iter (fun (mp, dev) -> g#mount_ro dev mp) mps;
+      List.iter (
+        fun (mp, dev) ->
+          try g#mount_ro dev mp
+          with Guestfs.Error msg -> eprintf "%s (ignored)\n" msg
+      ) mps;
 
       (* If /etc/issue.net file exists, print up to 3 lines. *)
       let filename = "/etc/issue.net" in

diff --git a/python/examples/inspect_vm.py b/python/examples/inspect_vm.py
index 0ba6e3e..c491a2c 100644 (file)
--- a/python/examples/inspect_vm.py
+++ b/python/examples/inspect_vm.py
@@ -44,7 +44,10 @@ for root in roots:
             return -1
     mps.sort (compare)
     for mp_dev in mps:
-        g.mount_ro (mp_dev[1], mp_dev[0])
+        try:
+            g.mount_ro (mp_dev[1], mp_dev[0])
+        except RuntimeError as msg:
+            print "%s (ignored)" % msg
 
     # If /etc/issue.net file exists, print up to 3 lines.
     filename = "/etc/issue.net"

diff --git a/regressions/Makefile.am b/regressions/Makefile.am
index c1dedcd..844bdfe 100644 (file)
--- a/regressions/Makefile.am
+++ b/regressions/Makefile.am
@@ -39,6 +39,7 @@ TESTS = \
        test-guestfish-a.sh \
        test-guestfish-d.sh \
        test-guestfish-tilde.sh \
+       test-inspect-fstab.sh \
        test-launch-race.pl \
        test-luks.sh \
        test-lvm-filtering.sh \

diff --git a/regressions/test-inspect-fstab.sh b/regressions/test-inspect-fstab.sh
new file mode 100755 (executable)
index 0000000..fb28415
--- /dev/null
+++ b/regressions/test-inspect-fstab.sh
@@ -0,0 +1,70 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2011 Red Hat 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., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+# Test various aspects of core inspection of /etc/fstab.
+# This also tests: https://bugzilla.redhat.com/668574
+
+set -e
+export LANG=C
+
+guestfish=../fish/guestfish
+
+rm -f test1.img test.fstab test.output
+
+# Start with the regular (good) fedora image, modify /etc/fstab
+# and then inspect it.
+cp ../images/fedora.img test1.img
+
+cat <<'EOF' > test.fstab
+/dev/VG/Root / ext2 default 0 0
+
+# Xen-style partition names.
+/dev/xvda1 /boot ext2 default 0 0
+
+# Non-existant device.
+/dev/sdb3 /var ext2 default 0 0
+
+# Non-existant mountpoint.
+/dev/VG/LV1 /nosuchfile ext2 default 0 0
+EOF
+
+$guestfish -a test1.img <<'EOF'
+  run
+  mount-options "" /dev/VG/Root /
+  upload test.fstab /etc/fstab
+EOF
+
+rm test.fstab
+
+# This will give a warning, but should not fail.
+$guestfish -a test1.img -i <<'EOF' | sort > test.output
+  inspect-get-mountpoints /dev/VG/Root
+EOF
+
+rm test1.img
+
+if [ "$(cat test.output)" != "/: /dev/VG/Root
+/boot: /dev/vda1
+/nosuchfile: /dev/VG/LV1
+/var: /dev/sdb3" ]; then
+    echo "$0: error: unexpected output from inspect-get-mountpoints command"
+    cat test.output
+    exit 1
+fi
+
+rm test.output

diff --git a/ruby/examples/inspect_vm.rb b/ruby/examples/inspect_vm.rb
index 032dec4..abf2279 100644 (file)
--- a/ruby/examples/inspect_vm.rb
+++ b/ruby/examples/inspect_vm.rb
@@ -41,7 +41,11 @@ for root in roots do
   mps = g.inspect_get_mountpoints(root)
   mps = mps.sort {|a,b| a[0].length <=> b[0].length}
   for mp in mps do
-    g.mount_ro(mp[1], mp[0])
+    begin
+      g.mount_ro(mp[1], mp[0])
+    rescue Guestfs::Error => msg
+      printf("%s (ignored)\n", msg)
+    end
   end
 
   # If /etc/issue.net file exists, print up to 3 lines.