From 0c60e4d9dd6549c2135699490ba8a9ec1dd50ab9 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Tue, 11 Jan 2011 10:43:51 +0000 Subject: [PATCH] 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 --- examples/inspect_vm.c | 6 ++-- fish/inspect.c | 7 +++- generator/generator_actions.ml | 4 +++ ocaml/examples/inspect_vm.ml | 6 +++- python/examples/inspect_vm.py | 5 ++- regressions/Makefile.am | 1 + regressions/test-inspect-fstab.sh | 70 +++++++++++++++++++++++++++++++++++++++ ruby/examples/inspect_vm.rb | 6 +++- 8 files changed, 99 insertions(+), 6 deletions(-) create mode 100755 regressions/test-inspect-fstab.sh diff --git a/examples/inspect_vm.c b/examples/inspect_vm.c index 4019a01..327a566 100644 --- 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 --- 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 --- 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. +I 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) and a value diff --git a/ocaml/examples/inspect_vm.ml b/ocaml/examples/inspect_vm.ml index 1d370c9..44d348e 100644 --- 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 --- 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 --- 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 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 --- 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. -- 1.8.3.1