Matthew Booth
2011-Nov-23 14:00 UTC
[Libguestfs] [PATCH 0/8] Add MD inspection support to libguestfs
This series fixes inspection in the case that fstab contains references to md devices. I've made a few changes since the previous posting, which I've summarised below. [PATCH 1/8] build: Create an MD variant of the dummy Fedora image I've double checked that no timestamp is required in the Makefile. The script will not run a second time to build fedora-md2.img. [PATCH 2/8] build: Nothing under images/ should be translated [PATCH 3/8] NFC: Declare and use variables on the same line in [PATCH 4/8] md: Inspect MD devices I separated this, as it's potentially independently useful. I've also split out the NFs as requested. [PATCH 5/8] NFC: Consolidate the error path in check_fstab in [PATCH 6/8] NFC: Allow multiple config files in inspect_with_augeas [PATCH 7/8] inspection: Handle MD devices in fstab * Split out the NFCs * Changed warning() to debug() in all cases * Following the pattern of other code, I have: * Not called error() when failure is due to the error return of a guestfs api * Explicitly called g->abort_cb() for malloc failures, consistent with the safe_ functions There are no other failure paths in the code. [PATCH 8/8] inspection: Add a test for MD device mapping in fstab
Matthew Booth
2011-Nov-23 14:00 UTC
[Libguestfs] [PATCH 1/8] build: Create an MD variant of the dummy Fedora image
This change involves rewriting make-fedora-img.sh in perl. This allows the
flexibility to write mdadm.conf containing whichever uuids where randomly
generated when the md devices were created.
---
.gitignore | 2 +
images/Makefile.am | 18 +++-
images/guest-aux/make-fedora-img.pl | 194 +++++++++++++++++++++++++++++++++++
images/guest-aux/make-fedora-img.sh | 110 --------------------
4 files changed, 210 insertions(+), 114 deletions(-)
create mode 100755 images/guest-aux/make-fedora-img.pl
delete mode 100755 images/guest-aux/make-fedora-img.sh
diff --git a/.gitignore b/.gitignore
index 9ebd4c7..b3b5a2e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -170,6 +170,8 @@ images/10klines
images/abssymlink
images/debian.img
images/fedora.img
+images/fedora-md1.img
+images/fedora-md2.img
images/guest-aux/fedora-name.db
images/guest-aux/fedora-packages.db
images/hello.b64
diff --git a/images/Makefile.am b/images/Makefile.am
index 1cae8bf..e413750 100644
--- a/images/Makefile.am
+++ b/images/Makefile.am
@@ -39,7 +39,7 @@ EXTRA_DIST = \
test-grep.txt \
guest-aux/make-debian-img.sh \
guest-aux/debian-packages \
- guest-aux/make-fedora-img.sh \
+ guest-aux/make-fedora-img.pl \
guest-aux/fedora-name.db.txt \
guest-aux/fedora-name.db \
guest-aux/fedora-packages.db.txt \
@@ -56,7 +56,7 @@ noinst_DATA = test.iso
# This is 'check_DATA' because we don't need it until 'make
check'
# time and we need the tools we have built in order to make it.
-check_DATA = debian.img fedora.img ubuntu.img windows.img
+check_DATA = debian.img fedora.img fedora-md1.img fedora-md2.img ubuntu.img
windows.img
CLEANFILES = \
test.iso test.sqsh \
@@ -170,12 +170,22 @@ $(builddir)/test-grep.txt.gz: test-grep.txt
mv $@-t $@
# Make a (dummy) Fedora image.
-fedora.img: guest-aux/make-fedora-img.sh \
+fedora.img: guest-aux/make-fedora-img.pl \
guest-aux/fedora-name.db \
guest-aux/fedora-packages.db
TMPDIR=$(top_builddir) \
SRCDIR=$(srcdir) \
- bash $<
+ LAYOUT=partitions \
+ ../run $<
+
+# Make a (dummy) Fedora image using md devices
+fedora-md1.img fedora-md2.img: guest-aux/make-fedora-img.pl \
+ guest-aux/fedora-name.db \
+ guest-aux/fedora-packages.db
+ TMPDIR=$(top_builddir) \
+ SRCDIR=$(srcdir) \
+ LAYOUT=partitions-md \
+ ../run $<
guest-aux/fedora-name.db: guest-aux/fedora-name.db.txt
rm -f $@ $@-t
diff --git a/images/guest-aux/make-fedora-img.pl
b/images/guest-aux/make-fedora-img.pl
new file mode 100755
index 0000000..a5dfd06
--- /dev/null
+++ b/images/guest-aux/make-fedora-img.pl
@@ -0,0 +1,194 @@
+#!/usr/bin/perl
+# libguestfs
+# Copyright (C) 2010-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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Make a standard test image which is used by all the tools test
+# scripts. This test image is supposed to look like a Fedora
+# installation, or at least enough of one to fool the inspection API
+# heuristics.
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+use File::Temp;
+
+my @images;
+my $g = Sys::Guestfs->new();
+
+my $bootdev;
+my $rootdev;
+
+foreach ('LAYOUT', 'SRCDIR') {
+ defined($ENV{$_}) or die "Missing environment variable: $_";
+}
+
+if ($ENV{LAYOUT} eq 'partitions') {
+ push (@images, 'fedora.img.tmp');
+
+ open(my $fstab, '>', 'fstab.tmp') or die;
+ print $fstab <<EOF;
+LABEL=BOOT /boot ext2 default 0 0
+LABEL=ROOT / ext2 default 0 0
+EOF
+ close($fstab) or die;
+
+ $bootdev = '/dev/sda1';
+ $rootdev = '/dev/sda2';
+
+ open(my $img, '>', 'fedora.img.tmp') or die;
+ truncate($img, 512*1024*1024) or die;
+ close($img) or die;
+
+ $g->add_drive('fedora.img.tmp');
+ $g->launch();
+
+ $g->part_init('/dev/sda', 'mbr');
+ $g->part_add('/dev/sda', 'p', 64, 524287);
+ $g->part_add('/dev/sda', 'p', 524288, -64);
+}
+
+elsif ($ENV{LAYOUT} eq 'partitions-md') {
+ push(@images, 'fedora-md1.img.tmp', 'fedora-md2.img.tmp');
+
+ open(my $fstab, '>', 'fstab.tmp') or die;
+ print $fstab <<EOF;
+/dev/md0 /boot ext2 default 0 0
+LABEL=ROOT / ext2 default 0 0
+EOF
+ close($fstab) or die;
+
+ $bootdev = '/dev/md/boot';
+ $rootdev = '/dev/md/root';
+
+ foreach my $img (@images) {
+ open(my $fh, '>', $img) or die;
+ truncate($fh, 512*1024*1024) or die;
+ close($fh) or die;
+
+ $g->add_drive($img);
+ }
+
+ $g->launch();
+
+ # Format the disks.
+ foreach ('a', 'b') {
+ $g->part_init("/dev/sd$_", 'mbr');
+ $g->part_add("/dev/sd$_", 'p', 64, 524287);
+ $g->part_add("/dev/sd$_", 'p', 524288, -64);
+ }
+
+ $g->mdadm_create('boot', ['/dev/sda1',
'/dev/sdb1']);
+ $g->mdadm_create('root', ['/dev/sda2',
'/dev/sdb2']);
+
+ open(my $mdadm, '>', 'mdadm.tmp') or die;
+ print $mdadm <<EOF;
+MAILADDR root
+AUTO +imsm +1.x -all
+EOF
+
+ my $i = 0;
+ foreach ('boot', 'root') {
+ my %detail = $g->mdadm_detail("/dev/md/$_");
+ print $mdadm "ARRAY /dev/md$i level=raid1 num-devices=2 UUID=",
+ $detail{uuid}, "\n";
+ $i++;
+ }
+
+ close($mdadm) or die;
+}
+
+else {
+ print STDERR "$0: Unknown LAYOUT: ",$ENV{LAYOUT},"\n";
+ exit 1;
+}
+
+$g->pvcreate($rootdev);
+$g->vgcreate('VG', [$rootdev]);
+$g->lvcreate('Root', 'VG', 32);
+$g->lvcreate('LV1', 'VG', 32);
+$g->lvcreate('LV2', 'VG', 32);
+$g->lvcreate('LV3', 'VG', 64);
+
+# Phony /boot filesystem
+$g->mkfs_opts('ext2', $bootdev, blocksize => 4096);
+$g->set_e2label($bootdev, 'BOOT');
+$g->set_e2uuid($bootdev, '01234567-0123-0123-0123-012345678901');
+
+# Phony root filesystem.
+$g->mkfs_opts('ext2', '/dev/VG/Root', blocksize => 4096);
+$g->set_e2label('/dev/VG/Root', 'ROOT');
+$g->set_e2uuid('/dev/VG/Root',
'01234567-0123-0123-0123-012345678902');
+
+# Enough to fool inspection API.
+$g->mount_options('', '/dev/VG/Root', '/');
+$g->mkdir('/boot');
+$g->mount_options('', $bootdev, '/boot');
+$g->mkdir('/bin');
+$g->mkdir('/etc');
+$g->mkdir('/etc/sysconfig');
+$g->mkdir('/usr');
+$g->mkdir_p('/var/lib/rpm');
+
+$g->upload('fstab.tmp', '/etc/fstab');
+$g->write('/etc/redhat-release', 'Fedora release 14
(Phony)');
+$g->write('/etc/fedora-release', 'Fedora release 14
(Phony)');
+$g->write('/etc/sysconfig/network',
'HOSTNAME=fedora.invalid');
+
+if (-f 'mdadm.tmp') {
+ $g->upload('mdadm.tmp', '/etc/mdadm.conf');
+ unlink('mdadm.tmp') or die;
+}
+
+$g->upload('guest-aux/fedora-name.db', '/var/lib/rpm/Name');
+$g->upload('guest-aux/fedora-packages.db',
'/var/lib/rpm/Packages');
+
+$g->upload($ENV{SRCDIR}.'/bin-x86_64-dynamic', '/bin/ls');
+
+$g->mkdir('/boot/grub');
+$g->touch('/boot/grub/grub.conf');
+
+# Test files.
+$g->write('/etc/test1', 'abcdefg');
+$g->write('/etc/test2', '');
+$g->write('/etc/test3',
+'a
+b
+c
+d
+e
+f');
+$g->write('/bin/test1', 'abcdefg');
+$g->write('/bin/test2', 'zxcvbnm');
+$g->write('/bin/test3', '1234567');
+$g->write('/bin/test4', '');
+$g->ln_s('/bin/test1', '/bin/test5');
+$g->mkfifo(0777, '/bin/test6');
+$g->mknod(0777, 10, 10, '/bin/test7');
+
+# Other filesystems.
+# Note that these should be empty, for testing virt-df.
+$g->mkfs_opts('ext2', '/dev/VG/LV1', blocksize => 4096);
+$g->mkfs_opts('ext2', '/dev/VG/LV2', blocksize => 1024);
+$g->mkfs_opts('ext2', '/dev/VG/LV3', blocksize => 2048);
+
+# Cleanup
+unlink('fstab.tmp') or die;
+foreach my $img (@images) {
+ $img =~ /^(.*)\.tmp$/ or die;
+ rename($img, $1) or die;
+}
diff --git a/images/guest-aux/make-fedora-img.sh
b/images/guest-aux/make-fedora-img.sh
deleted file mode 100755
index b4c07e9..0000000
--- a/images/guest-aux/make-fedora-img.sh
+++ /dev/null
@@ -1,110 +0,0 @@
-#!/bin/bash -
-# libguestfs
-# Copyright (C) 2010-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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-
-# Make a standard test image which is used by all the tools test
-# scripts. This test image is supposed to look like a Fedora
-# installation, or at least enough of one to fool the inspection API
-# heuristics.
-
-export LANG=C
-set -e
-
-# fstab file.
-cat > fstab.tmp <<EOF
-LABEL=BOOT /boot ext2 default 0 0
-LABEL=ROOT / ext2 default 0 0
-EOF
-
-# Create a disk image.
-../run ../fish/guestfish <<EOF
-sparse fedora.img.tmp 512M
-run
-
-# Format the disk.
-part-init /dev/sda mbr
-part-add /dev/sda p 64 524287
-part-add /dev/sda p 524288 -64
-
-pvcreate /dev/sda2
-vgcreate VG /dev/sda2
-lvcreate Root VG 32
-lvcreate LV1 VG 32
-lvcreate LV2 VG 32
-lvcreate LV3 VG 64
-
-# Phony /boot filesystem.
-mkfs-opts ext2 /dev/sda1 blocksize:4096
-set-e2label /dev/sda1 BOOT
-set-e2uuid /dev/sda1 01234567-0123-0123-0123-012345678901
-
-# Phony root filesystem.
-mkfs-opts ext2 /dev/VG/Root blocksize:4096
-set-e2label /dev/VG/Root ROOT
-set-e2uuid /dev/VG/Root 01234567-0123-0123-0123-012345678902
-
-# Enough to fool inspection API.
-mount-options "" /dev/VG/Root /
-mkdir /boot
-mount-options "" /dev/sda1 /boot
-mkdir /bin
-mkdir /etc
-mkdir /etc/sysconfig
-mkdir /usr
-mkdir-p /var/lib/rpm
-
-upload fstab.tmp /etc/fstab
-write /etc/redhat-release "Fedora release 14 (Phony)"
-write /etc/fedora-release "Fedora release 14 (Phony)"
-write /etc/sysconfig/network "HOSTNAME=fedora.invalid"
-
-upload guest-aux/fedora-name.db /var/lib/rpm/Name
-upload guest-aux/fedora-packages.db /var/lib/rpm/Packages
-
-upload ${SRCDIR}/bin-x86_64-dynamic /bin/ls
-
-mkdir /boot/grub
-touch /boot/grub/grub.conf
-
-# Test files.
-write /etc/test1 "abcdefg"
-write /etc/test2 ""
-upload -<<__end /etc/test3
-a
-b
-c
-d
-e
-f
-__end
-write /bin/test1 "abcdefg"
-write /bin/test2 "zxcvbnm"
-write /bin/test3 "1234567"
-write /bin/test4 ""
-ln-s /bin/test1 /bin/test5
-mkfifo 0777 /bin/test6
-mknod 0777 10 10 /bin/test7
-
-# Other filesystems.
-# Note that these should be empty, for testing virt-df.
-mkfs-opts ext2 /dev/VG/LV1 blocksize:4096
-mkfs-opts ext2 /dev/VG/LV2 blocksize:1024
-mkfs-opts ext2 /dev/VG/LV3 blocksize:2048
-EOF
-
-rm fstab.tmp
-mv fedora.img.tmp fedora.img
--
1.7.6.4
Matthew Booth
2011-Nov-23 14:00 UTC
[Libguestfs] [PATCH 2/8] build: Nothing under images/ should be translated
--- Makefile.am | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Makefile.am b/Makefile.am index a223600..8394ac4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -228,6 +228,7 @@ all-local: grep -v '/rc_protocol.c' | \ grep -v 'appliance/debian/root' | \ grep -v '^po-docs/' | \ + grep -v '^images/' | \ LC_ALL=C sort | \ sed 's,^\./,,' > po/POTFILES.in -- 1.7.6.4
Matthew Booth
2011-Nov-23 14:00 UTC
[Libguestfs] [PATCH 3/8] NFC: Declare and use variables on the same line in inspect.c
---
src/inspect.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/inspect.c b/src/inspect.c
index 1b41be5..e49cb1f 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -61,8 +61,7 @@ guestfs__inspect_os (guestfs_h *g)
* information to the handle.
*/
/* Look to see if any devices directly contain filesystems (RHBZ#590167). */
- char **devices;
- devices = guestfs_list_devices (g);
+ char **devices = guestfs_list_devices (g);
if (devices == NULL)
return NULL;
@@ -77,8 +76,7 @@ guestfs__inspect_os (guestfs_h *g)
guestfs___free_string_list (devices);
/* Look at all partitions. */
- char **partitions;
- partitions = guestfs_list_partitions (g);
+ char **partitions = guestfs_list_partitions (g);
if (partitions == NULL) {
guestfs___free_inspect_info (g);
return NULL;
--
1.7.6.4
---
src/inspect.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/src/inspect.c b/src/inspect.c
index e49cb1f..f9b8298 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -91,6 +91,22 @@ guestfs__inspect_os (guestfs_h *g)
}
guestfs___free_string_list (partitions);
+ /* Look at MD devices. */
+ char **mds = guestfs_list_md_devices (g);
+ if (mds == NULL) {
+ guestfs___free_inspect_info (g);
+ return NULL;
+ }
+
+ for (i = 0; mds[i] != NULL; ++i) {
+ if (guestfs___check_for_filesystem_on (g, mds[i], 0, i+1) == -1) {
+ guestfs___free_string_list (mds);
+ guestfs___free_inspect_info (g);
+ return NULL;
+ }
+ }
+ guestfs___free_string_list (mds);
+
/* Look at all LVs. */
if (guestfs___feature_available (g, "lvm2")) {
char **lvs;
--
1.7.6.4
Matthew Booth
2011-Nov-23 14:00 UTC
[Libguestfs] [PATCH 5/8] NFC: Consolidate the error path in check_fstab in inspect_fs_unix.c
---
src/inspect_fs_unix.c | 23 +++++++++--------------
1 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/src/inspect_fs_unix.c b/src/inspect_fs_unix.c
index 6b1a05b..51dfa67 100644
--- a/src/inspect_fs_unix.c
+++ b/src/inspect_fs_unix.c
@@ -685,13 +685,11 @@ static int
check_fstab (guestfs_h *g, struct inspect_fs *fs)
{
char **lines = guestfs_aug_ls (g, "/files/etc/fstab");
- if (lines == NULL)
- return -1;
+ if (lines == NULL) goto error;
if (lines[0] == NULL) {
error (g, _("could not parse /etc/fstab or empty file"));
- guestfs___free_string_list (lines);
- return -1;
+ goto error;
}
size_t i;
@@ -703,32 +701,29 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs)
if (match (g, lines[i], re_aug_seq)) {
snprintf (augpath, sizeof augpath, "%s/spec", lines[i]);
char *spec = guestfs_aug_get (g, augpath);
- if (spec == NULL) {
- guestfs___free_string_list (lines);
- return -1;
- }
+ if (spec == NULL) goto error;
snprintf (augpath, sizeof augpath, "%s/file", lines[i]);
char *mp = guestfs_aug_get (g, augpath);
if (mp == NULL) {
- guestfs___free_string_list (lines);
free (spec);
- return -1;
+ goto error;
}
int r = add_fstab_entry (g, fs, spec, mp);
free (spec);
free (mp);
- if (r == -1) {
- guestfs___free_string_list (lines);
- return -1;
- }
+ if (r == -1) goto error;
}
}
guestfs___free_string_list (lines);
return 0;
+
+error:
+ if (lines) guestfs___free_string_list (lines);
+ return -1;
}
/* Add a filesystem and possibly a mountpoint entry for
--
1.7.6.4
Matthew Booth
2011-Nov-23 14:00 UTC
[Libguestfs] [PATCH 6/8] NFC: Allow multiple config files in inspect_with_augeas in inspect_fs_unix.c
This change is in support of the addition of MD support to fstab inspection.
---
src/inspect_fs_unix.c | 78 +++++++++++++++++++++++++++++++++++++------------
1 files changed, 59 insertions(+), 19 deletions(-)
diff --git a/src/inspect_fs_unix.c b/src/inspect_fs_unix.c
index 51dfa67..3046e02 100644
--- a/src/inspect_fs_unix.c
+++ b/src/inspect_fs_unix.c
@@ -143,7 +143,7 @@ static int check_fstab (guestfs_h *g, struct inspect_fs
*fs);
static int add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
const char *spec, const char *mp);
static char *resolve_fstab_device (guestfs_h *g, const char *spec);
-static int inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs, const char
*filename, int (*f) (guestfs_h *, struct inspect_fs *));
+static int inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs, const char
**configfiles, int (*f) (guestfs_h *, struct inspect_fs *));
/* Set fs->product_name to the first line of the release file. */
static int
@@ -446,7 +446,8 @@ guestfs___check_linux_root (guestfs_h *g, struct inspect_fs
*fs)
* which filesystems are used by the operating system and how they
* are mounted.
*/
- if (inspect_with_augeas (g, fs, "/etc/fstab", check_fstab) == -1)
+ const char *configfiles[] = { "/etc/fstab", NULL };
+ if (inspect_with_augeas (g, fs, configfiles, check_fstab) == -1)
return -1;
/* Determine hostname. */
@@ -479,7 +480,8 @@ guestfs___check_freebsd_root (guestfs_h *g, struct
inspect_fs *fs)
check_architecture (g, fs);
/* We already know /etc/fstab exists because it's part of the test above.
*/
- if (inspect_with_augeas (g, fs, "/etc/fstab", check_fstab) == -1)
+ const char *configfiles[] = { "/etc/fstab", NULL };
+ if (inspect_with_augeas (g, fs, configfiles, check_fstab) == -1)
return -1;
/* Determine hostname. */
@@ -520,7 +522,8 @@ guestfs___check_netbsd_root (guestfs_h *g, struct inspect_fs
*fs)
check_architecture (g, fs);
/* We already know /etc/fstab exists because it's part of the test above.
*/
- if (inspect_with_augeas (g, fs, "/etc/fstab", check_fstab) == -1)
+ const char *configfiles[] = { "/etc/fstab", NULL };
+ if (inspect_with_augeas (g, fs, configfiles, check_fstab) == -1)
return -1;
/* Determine hostname. */
@@ -583,7 +586,8 @@ check_hostname_unix (guestfs_h *g, struct inspect_fs *fs)
return -1;
}
else if (guestfs_is_file (g, "/etc/sysconfig/network")) {
- if (inspect_with_augeas (g, fs, "/etc/sysconfig/network",
+ const char *configfiles[] = { "/etc/sysconfig/network", NULL };
+ if (inspect_with_augeas (g, fs, configfiles,
check_hostname_redhat) == -1)
return -1;
}
@@ -941,18 +945,23 @@ resolve_fstab_device (guestfs_h *g, const char *spec)
* Augeas is closed.
*/
static int
-inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs, const char *filename,
+inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs,
+ const char **configfiles,
int (*f) (guestfs_h *, struct inspect_fs *))
{
- /* Security: Refuse to do this if filename is too large. */
- int64_t size = guestfs_filesize (g, filename);
- if (size == -1)
- /* guestfs_filesize failed and has already set error in handle */
- return -1;
- if (size > MAX_AUGEAS_FILE_SIZE) {
- error (g, _("size of %s is unreasonably large (%" PRIi64 "
bytes)"),
- filename, size);
- return -1;
+ /* Security: Refuse to do this if a config file is too large. */
+ for (const char **i = configfiles; *i != NULL; i++) {
+ if (guestfs_exists(g, *i) == 0) continue;
+
+ int64_t size = guestfs_filesize (g, *i);
+ if (size == -1)
+ /* guestfs_filesize failed and has already set error in handle */
+ return -1;
+ if (size > MAX_AUGEAS_FILE_SIZE) {
+ error (g, _("size of %s is unreasonably large (%" PRIi64 "
bytes)"),
+ *i, size);
+ return -1;
+ }
}
/* If !feature_available (g, "augeas") then the next call will
fail.
@@ -965,11 +974,42 @@ inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs,
const char *filename,
int r = -1;
/* Tell Augeas to only load one file (thanks Rapha?l Pinson). */
- char buf[strlen (filename) + 64];
- snprintf (buf, strlen (filename) + 64, "/augeas/load//incl[. !=
\"%s\"]",
- filename);
- if (guestfs_aug_rm (g, buf) == -1)
+#define AUGEAS_LOAD "/augeas/load//incl[. != \""
+#define AUGEAS_LOAD_LEN (strlen(AUGEAS_LOAD))
+ size_t conflen = strlen(configfiles[0]);
+ size_t buflen = AUGEAS_LOAD_LEN + conflen + 1 /* Closing " */;
+ char *buf = safe_malloc(g, buflen + 2 /* Closing ] + null terminator */);
+
+ memcpy(buf, AUGEAS_LOAD, AUGEAS_LOAD_LEN);
+ memcpy(buf + AUGEAS_LOAD_LEN, configfiles[0], conflen);
+ buf[buflen - 1] = '"';
+#undef AUGEAS_LOAD_LEN
+#undef AUGEAS_LOAD
+
+#define EXCL " and . != \""
+#define EXCL_LEN (strlen(EXCL))
+ for (const char **i = &configfiles[1]; *i != NULL; i++) {
+ size_t orig_buflen = buflen;
+ conflen = strlen(*i);
+ buflen += EXCL_LEN + conflen + 1 /* Closing " */;
+ buf = safe_realloc(g, buf, buflen + 2 /* Closing ] + null terminator */);
+ char *s = buf + orig_buflen;
+
+ memcpy(s, EXCL, EXCL_LEN);
+ memcpy(s + EXCL_LEN, *i, conflen);
+ buf[buflen - 1] = '"';
+ }
+#undef EXCL_LEN
+#undef EXCL
+
+ buf[buflen] = ']';
+ buf[buflen + 1] = '\0';
+
+ if (guestfs_aug_rm (g, buf) == -1) {
+ free(buf);
goto out;
+ }
+ free(buf);
if (guestfs_aug_load (g) == -1)
goto out;
--
1.7.6.4
Matthew Booth
2011-Nov-23 14:00 UTC
[Libguestfs] [PATCH 7/8] inspection: Handle MD devices in fstab
This patch fixes inspection when fstab contains devices md devices
specified as /dev/mdN. The appliance creates these devices without reference to
the guest's mdadm.conf so, for e.g. /dev/md0 in the guest will often be
created
as /dev/md127 in the appliance. With this patch, we match the uuids of detected
md devices against uuids specified in mdadm.conf, and map them appropriately
when we encounter them in fstab.
---
src/inspect_fs_unix.c | 317 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 310 insertions(+), 7 deletions(-)
diff --git a/src/inspect_fs_unix.c b/src/inspect_fs_unix.c
index 3046e02..3be49d5 100644
--- a/src/inspect_fs_unix.c
+++ b/src/inspect_fs_unix.c
@@ -38,6 +38,8 @@
#include "c-ctype.h"
#include "ignore-value.h"
#include "xstrtol.h"
+#include "hash.h"
+#include "hash-pjw.h"
#include "guestfs.h"
#include "guestfs-internal.h"
@@ -65,6 +67,7 @@ static pcre *re_major_minor;
static pcre *re_aug_seq;
static pcre *re_xdev;
static pcre *re_cciss;
+static pcre *re_mdN;
static pcre *re_first_partition;
static pcre *re_freebsd;
static pcre *re_netbsd;
@@ -110,6 +113,7 @@ compile_regexps (void)
COMPILE (re_aug_seq, "/\\d+$", 0);
COMPILE (re_xdev, "^/dev/(h|s|v|xv)d([a-z]+)(\\d*)$", 0);
COMPILE (re_cciss, "^/dev/(cciss/c\\d+d\\d+)(?:p(\\d+))?$", 0);
+ COMPILE (re_mdN, "^(/dev/md\\d+)$", 0);
COMPILE (re_freebsd, "^/dev/ad(\\d+)s(\\d+)([a-z])$", 0);
COMPILE (re_netbsd, "^NetBSD (\\d+)\\.(\\d+)", 0);
}
@@ -131,6 +135,7 @@ free_regexps (void)
pcre_free (re_aug_seq);
pcre_free (re_xdev);
pcre_free (re_cciss);
+ pcre_free (re_mdN);
pcre_free (re_freebsd);
pcre_free (re_netbsd);
}
@@ -141,10 +146,37 @@ static int check_hostname_redhat (guestfs_h *g, struct
inspect_fs *fs);
static int check_hostname_freebsd (guestfs_h *g, struct inspect_fs *fs);
static int check_fstab (guestfs_h *g, struct inspect_fs *fs);
static int add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
- const char *spec, const char *mp);
-static char *resolve_fstab_device (guestfs_h *g, const char *spec);
+ const char *spec, const char *mp,
+ Hash_table *md_map);
+static char *resolve_fstab_device (guestfs_h *g, const char *spec,
+ Hash_table *md_map);
static int inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs, const char
**configfiles, int (*f) (guestfs_h *, struct inspect_fs *));
+/* Hash structure for uuid->path lookups */
+typedef struct md_uuid {
+ uint32_t uuid[4];
+ char *path;
+} md_uuid;
+
+static size_t uuid_hash(const void *x, size_t table_size);
+static bool uuid_cmp(const void *x, const void *y);
+static void md_uuid_free(void *x);
+
+static bool parse_uuid(const char *str, uint32_t *uuid);
+
+/* Hash structure for path(mdadm)->path(appliance) lookup */
+typedef struct {
+ char *mdadm;
+ char *app;
+} mdadm_app;
+
+static size_t mdadm_app_hash(const void *x, size_t table_size);
+static bool mdadm_app_cmp(const void *x, const void *y);
+static void mdadm_app_free(void *x);
+
+static bool map_app_md_devices (guestfs_h *g, Hash_table **map);
+static bool map_md_devices(guestfs_h *g, Hash_table **map);
+
/* Set fs->product_name to the first line of the release file. */
static int
parse_release_file (guestfs_h *g, struct inspect_fs *fs,
@@ -446,7 +478,7 @@ guestfs___check_linux_root (guestfs_h *g, struct inspect_fs
*fs)
* which filesystems are used by the operating system and how they
* are mounted.
*/
- const char *configfiles[] = { "/etc/fstab", NULL };
+ const char *configfiles[] = { "/etc/fstab",
"/etc/mdadm.conf", NULL };
if (inspect_with_augeas (g, fs, configfiles, check_fstab) == -1)
return -1;
@@ -688,6 +720,11 @@ check_hostname_freebsd (guestfs_h *g, struct inspect_fs
*fs)
static int
check_fstab (guestfs_h *g, struct inspect_fs *fs)
{
+ /* Generate a map of MD device paths listed in /etc/mdadm.conf to MD device
+ * paths in the guestfs appliance */
+ Hash_table *md_map;
+ if (!map_md_devices (g, &md_map)) return -1;
+
char **lines = guestfs_aug_ls (g, "/files/etc/fstab");
if (lines == NULL) goto error;
@@ -714,7 +751,7 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs)
goto error;
}
- int r = add_fstab_entry (g, fs, spec, mp);
+ int r = add_fstab_entry (g, fs, spec, mp, md_map);
free (spec);
free (mp);
@@ -722,10 +759,12 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs)
}
}
+ hash_free (md_map);
guestfs___free_string_list (lines);
return 0;
error:
+ hash_free (md_map);
if (lines) guestfs___free_string_list (lines);
return -1;
}
@@ -740,7 +779,7 @@ error:
*/
static int
add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
- const char *spec, const char *mp)
+ const char *spec, const char *mp, Hash_table *md_map)
{
/* Ignore certain mountpoints. */
if (STRPREFIX (mp, "/dev/") ||
@@ -772,7 +811,7 @@ add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
device = safe_strdup (g, fs->device);
else if (STRPREFIX (spec, "/dev/"))
/* Resolve guest block device names. */
- device = resolve_fstab_device (g, spec);
+ device = resolve_fstab_device (g, spec, md_map);
/* If we haven't resolved the device successfully by this point,
* we don't care, just ignore it.
@@ -809,6 +848,263 @@ add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
return 0;
}
+/* Compute a uuid hash as a simple xor of of its 4 32bit components */
+static size_t
+uuid_hash(const void *x, size_t table_size)
+{
+ const md_uuid *a = x;
+
+ size_t h = a->uuid[0];
+ for (size_t i = 1; i < 4; i++) {
+ h ^= a->uuid[i];
+ }
+
+ return h % table_size;
+}
+
+static bool
+uuid_cmp(const void *x, const void *y)
+{
+ const md_uuid *a = x;
+ const md_uuid *b = y;
+
+ for (size_t i = 0; i < 1; i++) {
+ if (a->uuid[i] != b->uuid[i]) return false;
+ }
+
+ return true;
+}
+
+static void
+md_uuid_free(void *x)
+{
+ md_uuid *a = x;
+ free(a->path);
+ free(a);
+}
+
+/* Taken from parse_uuid in mdadm */
+static bool
+parse_uuid(const char *str, uint32_t *uuid)
+{
+ for (size_t i = 0; i < 4; i++) uuid[i] = 0;
+
+ int hit = 0; /* number of Hex digIT */
+ char c;
+ while ((c = *str++)) {
+ int n;
+ if (c >= '0' && c <= '9')
+ n = c - '0';
+ else if (c >= 'a' && c <= 'f')
+ n = 10 + c - 'a';
+ else if (c >= 'A' && c <= 'F')
+ n = 10 + c - 'A';
+ else if (strchr(":. -", c))
+ continue;
+ else return false;
+
+ if (hit < 32) {
+ uuid[hit / 8] <<= 4;
+ uuid[hit / 8] += n;
+ }
+ hit++;
+ }
+ if (hit == 32) return true;
+
+ return false;
+}
+
+/* Create a mapping of uuids to appliance md device names */
+static bool
+map_app_md_devices (guestfs_h *g, Hash_table **map)
+{
+ int ret = false;
+
+ char **mds = NULL;
+
+ /* A hash mapping uuids to md device names */
+ *map = hash_initialize(16, NULL, uuid_hash, uuid_cmp, md_uuid_free);
+ if (*map == NULL) g->abort_cb();
+
+ mds = guestfs_list_md_devices(g);
+ if (mds == NULL) goto error;
+
+ for (char **md = mds; *md != NULL; md++) {
+ char **detail = guestfs_mdadm_detail(g, *md);
+ if (detail == NULL) goto error;
+
+ /* Iterate over keys until we find uuid */
+ char **i;
+ for (i = detail; *i != NULL; i += 2) {
+ if (STREQ(*i, "uuid")) break;
+ }
+
+ /* We found it */
+ if (*i) {
+ /* Next item is the uuid value */
+ i++;
+
+ md_uuid *entry = safe_malloc(g, sizeof(md_uuid));
+ entry->path = safe_strdup(g, *md);
+
+ if (!parse_uuid(*i, entry->uuid)) {
+ /* Invalid UUID is weird, but not fatal. */
+ debug(g, "inspect-os: guestfs_mdadm_detail returned invalid "
+ "uuid for %s: %s", *md, *i);
+ guestfs___free_string_list(detail);
+ md_uuid_free(entry);
+ continue;
+ }
+
+ const void *matched = NULL;
+ switch (hash_insert_if_absent(*map, entry, &matched)) {
+ case -1:
+ g->abort_cb();
+
+ case 0:
+ /* Duplicate uuid in for md device is weird, but not fatal. */
+ debug(g, "inspect-os: md devices %s and %s have the same
uuid",
+ ((md_uuid *)matched)->path, entry->path);
+ md_uuid_free(entry);
+
+ default:
+ ;;
+ }
+ }
+
+ guestfs___free_string_list(detail);
+ }
+
+ guestfs___free_string_list(mds);
+
+ return true;
+
+error:
+ hash_free(*map); *map = NULL;
+ guestfs___free_string_list(mds);
+
+ return false;
+}
+
+static size_t
+mdadm_app_hash(const void *x, size_t table_size)
+{
+ const mdadm_app *a = x;
+ return hash_pjw(a->mdadm, table_size);
+}
+
+static bool
+mdadm_app_cmp(const void *x, const void *y)
+{
+ const mdadm_app *a = x;
+ const mdadm_app *b = y;
+
+ return strcmp(a->mdadm, b->mdadm) == 0;
+}
+
+static void
+mdadm_app_free(void *x)
+{
+ mdadm_app *a = x;
+ free(a->mdadm);
+ free(a->app);
+ free(a);
+}
+
+/* Get a map of md device names in mdadm.conf to their device names in the
+ * appliance */
+static bool
+map_md_devices(guestfs_h *g, Hash_table **map)
+{
+ Hash_table *app_map = NULL;
+ char **matches = NULL;
+ char *dev_path = NULL;
+ char *uuid_path = NULL;
+ *map = NULL;
+
+ /* Get a map of md device uuids to their device names in the appliance */
+ if (!map_app_md_devices(g, &app_map)) goto error;
+
+ *map = hash_initialize(16, NULL, mdadm_app_hash, mdadm_app_cmp,
+ mdadm_app_free);
+ if (!*map) g->abort_cb();
+
+ /* Get all arrays listed in mdadm.conf */
+ matches = guestfs_aug_match(g, "/files/etc/mdadm.conf/array");
+ if (!matches) goto error;
+
+ for (char **match = matches; *match != NULL; match++) {
+ /* Get device name and uuid for each array */
+ dev_path = safe_asprintf(g, "%s/devicename", *match);
+ uuid_path = safe_asprintf(g, "%s/uuid", *match);
+
+ char *dev = guestfs_aug_get(g, dev_path);
+ if (!dev) goto error;
+
+ char *uuid = guestfs_aug_get(g, uuid_path);
+ if (!uuid) {
+ free(dev);
+ goto error;
+ }
+
+ /* Parse the uuid into an md_uuid structure so we can look it up in the
+ * uuid->appliance device map */
+ md_uuid mdadm;
+ mdadm.path = dev;
+ if (!parse_uuid(uuid, mdadm.uuid)) {
+ /* Invalid uuid. Weird, but not fatal. */
+ debug(g, "inspect-os: mdadm.conf contains invalid uuid for %s:
%s",
+ dev, uuid);
+ free(dev);
+ free(uuid);
+ continue;
+ }
+ free(uuid);
+
+ /* If there's a corresponding uuid in the appliance, create a new
+ * entry in the transitive map */
+ md_uuid *app = hash_lookup(app_map, &mdadm);
+ if (app) {
+ mdadm_app *entry = safe_malloc(g, sizeof(mdadm_app));
+ entry->mdadm = dev;
+ entry->app = safe_strdup(g, app->path);
+
+ switch (hash_insert_if_absent(*map, entry, NULL)) {
+ case -1:
+ g->abort_cb();
+
+ case 0:
+ /* Duplicate uuid in for md device is weird, but not fatal. */
+ debug(g, "inspect-os: mdadm.conf contains multiple entries for
%s",
+ app->path);
+ mdadm_app_free(entry);
+ continue;
+
+ default:
+ ;;
+ }
+ } else {
+ free(dev);
+ }
+ }
+
+ hash_free(app_map);
+ guestfs___free_string_list(matches);
+ free(dev_path);
+ free(uuid_path);
+
+ return true;
+
+error:
+ if (app_map) hash_free(app_map);
+ if (matches) guestfs___free_string_list(matches);
+ free(dev_path);
+ free(uuid_path);
+ if (*map) hash_free(*map);
+
+ return false;
+}
+
/* Resolve block device name to the libguestfs device name, eg.
* /dev/xvdb1 => /dev/vdb1; and /dev/mapper/VG-LV => /dev/VG/LV. This
* assumes that disks were added in the same order as they appear to
@@ -816,7 +1112,7 @@ add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
* anything we don't recognize unchanged.
*/
static char *
-resolve_fstab_device (guestfs_h *g, const char *spec)
+resolve_fstab_device (guestfs_h *g, const char *spec, Hash_table *md_map)
{
char *device = NULL;
char *type, *slice, *disk, *part;
@@ -911,6 +1207,13 @@ resolve_fstab_device (guestfs_h *g, const char *spec)
free (part);
guestfs___free_string_list (devices);
}
+ else if ((disk = match1 (g, spec, re_mdN)) != NULL) {
+ mdadm_app entry;
+ entry.mdadm = disk;
+
+ mdadm_app *app = hash_lookup (md_map, &entry);
+ if (app) device = safe_strdup (g, app->app);
+ }
else if (match3 (g, spec, re_freebsd, &disk, &slice, &part)) {
/* FreeBSD disks are organized quite differently. See:
* http://www.freebsd.org/doc/handbook/disk-organization.html
--
1.7.6.4
Matthew Booth
2011-Nov-23 14:00 UTC
[Libguestfs] [PATCH 8/8] inspection: Add a test for MD device mapping in fstab
Check that we properly handle fstab entries of the form /dev/md0 and /dev/md/foo. --- regressions/Makefile.am | 1 + regressions/test-inspect-fstab-md.sh | 65 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 0 deletions(-) create mode 100755 regressions/test-inspect-fstab-md.sh diff --git a/regressions/Makefile.am b/regressions/Makefile.am index c75d54a..e7ebc6f 100644 --- a/regressions/Makefile.am +++ b/regressions/Makefile.am @@ -44,6 +44,7 @@ TESTS = \ test-guestfish-escapes.sh \ test-guestfish-tilde.sh \ test-inspect-fstab.sh \ + test-inspect-fstab-md.sh \ test-launch-race.pl \ test-list-filesystems.sh \ test-list-md-devices.sh \ diff --git a/regressions/test-inspect-fstab-md.sh b/regressions/test-inspect-fstab-md.sh new file mode 100755 index 0000000..d541ef4 --- /dev/null +++ b/regressions/test-inspect-fstab-md.sh @@ -0,0 +1,65 @@ +#!/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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +# Test the handling of MD devices specified in /etc/fstab + +set -e +export LANG=C + +guestfish=../fish/guestfish + +rm -f test1.img test.fstab test.output + +# First, test the regular fedora image, which specifies /boot as /dev/md0 +cp ../images/fedora-md1.img test1.img +cp ../images/fedora-md2.img test2.img + +$guestfish -i test[12].img <<'EOF' | sort > test.output + exists /boot/grub/grub.conf +EOF + +if [ "$(cat test.output)" != "true" ]; then + echo "$0: /boot not correctly mounted (/dev/md0)" + exit 1 +fi + +# Test inspection when /boot is specfied as /dev/md/boot +cat <<'EOF' > test.fstab +/dev/VG/Root / ext2 default 0 0 +/dev/md/boot /boot ext2 default 0 0 +EOF + +$guestfish -a test1.img -a test2.img <<'EOF' + run + mount-options "" /dev/VG/Root / + upload test.fstab /etc/fstab +EOF + +$guestfish -i test[12].img <<'EOF' | sort > test.output + exists /boot/grub/grub.conf +EOF + +if [ "$(cat test.output)" != "true" ]; then + echo "$0: error: /boot not correctly mounted (/dev/md/boot)" + cat test.output + exit 1 +fi + +rm test.fstab +rm test[12].img +rm test.output -- 1.7.6.4