[Libguestfs] [PATCH] appliance: Pass root=UUID=... to supermin.
Richard W.M. Jones
rjones at redhat.com
Wed Apr 19 16:50:21 UTC 2017
Instead of mounting the appliance by device name (eg. root=/dev/sdb),
mount the appliance filesystem by filesystem UUID. This allows the
appliance to be robust against the non-determinism of SCSI device
enumeration, but it does require a corresponding change to be added to
supermin, and therefore a new version of supermin.
I did not make a corresponding change to UML as it is unlikely to have
a problem with enumerating ubd devices.
---
docs/guestfs-building.pod | 2 +-
lib/Makefile.am | 1 +
lib/appliance-kcmdline.c | 10 ++---
lib/appliance-uuid.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
lib/guestfs-internal.h | 3 ++
lib/launch-direct.c | 36 +++---------------
lib/launch-libvirt.c | 19 +++++++---
m4/guestfs_appliance.m4 | 2 +-
8 files changed, 124 insertions(+), 44 deletions(-)
create mode 100644 lib/appliance-uuid.c
diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod
index bfb46a02f..775d590f3 100644
--- a/docs/guestfs-building.pod
+++ b/docs/guestfs-building.pod
@@ -80,7 +80,7 @@ I<Required>. Virt-p2v and virt-v2v requires qemu-img E<ge> 2.2.0.
I<Required>. The following features must be enabled:
C<virtio-pci>, C<virtio-serial>, C<virtio-block>, C<virtio-net>.
-=item supermin E<ge> 5.1.0
+=item supermin E<ge> 5.1.18
I<Required>. For alternatives, see L</USING A PREBUILT BINARY APPLIANCE>
below.
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 063706f8f..01f499e6d 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -75,6 +75,7 @@ libguestfs_la_SOURCES = \
appliance-cpu.c \
appliance-kcmdline.c \
appliance-uefi.c \
+ appliance-uuid.c \
available.c \
bindtests.c \
canonical-name.c \
diff --git a/lib/appliance-kcmdline.c b/lib/appliance-kcmdline.c
index 4dde7a865..efb21e829 100644
--- a/lib/appliance-kcmdline.c
+++ b/lib/appliance-kcmdline.c
@@ -58,9 +58,7 @@
* located in this file because it's a convenient place for this
* common code.
*
- * The C<appliance_dev> parameter must be the full device name of the
- * appliance disk and must have already been adjusted to take into
- * account virtio-blk or virtio-scsi; eg C</dev/sdb>.
+ * The C<appliance_uuid> is the UUID of the appliance filesystem.
*
* The C<flags> parameter can contain the following flags logically
* or'd together (or 0):
@@ -78,7 +76,7 @@
* be freed by the caller.
*/
char *
-guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev,
+guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_uuid,
int flags)
{
CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (argv);
@@ -162,8 +160,8 @@ guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev,
guestfs_int_add_string (g, &argv, "8250.nr_uarts=1");
/* Tell supermin about the appliance device. */
- if (appliance_dev)
- guestfs_int_add_sprintf (g, &argv, "root=%s", appliance_dev);
+ if (appliance_uuid)
+ guestfs_int_add_sprintf (g, &argv, "root=UUID=%s", appliance_uuid);
/* SELinux - deprecated setting, never worked and should not be enabled. */
if (g->selinux)
diff --git a/lib/appliance-uuid.c b/lib/appliance-uuid.c
new file mode 100644
index 000000000..80dc319ae
--- /dev/null
+++ b/lib/appliance-uuid.c
@@ -0,0 +1,95 @@
+/* libguestfs
+ * Copyright (C) 2010-2017 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * Calculate the root=UUID=... kernel parameter for the appliance.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "guestfs.h"
+#include "guestfs-internal.h"
+
+/**
+ * Calculate the volume UUID of an ext4 filesystem (in a file). This
+ * is quite simple as it is stored at a known offset. See
+ * L<https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout#The_Super_Block>.
+ */
+char *
+guestfs_int_ext4fs_uuid (guestfs_h *g, const char *filename)
+{
+ int fd;
+ unsigned char magic[2];
+ unsigned char raw_uuid[16];
+ char uuid[37];
+
+ fd = open (filename, O_RDONLY);
+ if (fd == -1) {
+ perrorf (g, "%s", filename);
+ return NULL;
+ }
+
+ /* Check it's really an ext4 superblock at offset 0x400. */
+ if (pread (fd, magic, sizeof magic, 0x438) != sizeof magic) {
+ error (g, "ext4fs: could not read magic from ext4 file: %s", filename);
+ close (fd);
+ return NULL;
+ }
+ if (magic[0] != 0x53 || magic[1] != 0xef) {
+ error (g, "ext4fs: not an ext4 file: %s", filename);
+ close (fd);
+ return NULL;
+ }
+
+ /* Read the raw bytes of the UUID (128 bits / 8 == 16 bytes). */
+ if (pread (fd, raw_uuid, sizeof raw_uuid, 0x468) != sizeof raw_uuid) {
+ error (g, "ext4fs: could not read volume UUID from ext4 superblock: %s",
+ filename);
+ close (fd);
+ return NULL;
+ }
+ close (fd);
+
+ /* Check for sanity. */
+ if (is_zero ((char *) raw_uuid, sizeof raw_uuid)) {
+ error (g, "ext4fs: UUID is all zeroes: %s", filename);
+ return NULL;
+ }
+
+ /* Convert to a string UUID. The same format that blkid produces. */
+ snprintf (uuid, sizeof uuid,
+ "%02x%02x%02x%02x-"
+ "%02x%02x-"
+ "%02x%02x-"
+ "%02x%02x-"
+ "%02x%02x%02x%02x%02x%02x",
+ raw_uuid[0], raw_uuid[1], raw_uuid[2], raw_uuid[3],
+ raw_uuid[4], raw_uuid[5],
+ raw_uuid[6], raw_uuid[7],
+ raw_uuid[8], raw_uuid[9],
+ raw_uuid[10], raw_uuid[11], raw_uuid[12], raw_uuid[13],
+ raw_uuid[14], raw_uuid[15]);
+
+ return safe_strdup (g, uuid); /* caller frees */
+}
diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index a04ccff09..c181396ec 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -847,6 +847,9 @@ extern char *guestfs_int_appliance_command_line (guestfs_h *g, const char *appli
/* appliance-uefi.c */
extern int guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags);
+/* appliance-uuid.c */
+extern char *guestfs_int_ext4fs_uuid (guestfs_h *g, const char *filename);
+
/* launch.c */
extern int64_t guestfs_int_timeval_diff (const struct timeval *x, const struct timeval *y);
extern void guestfs_int_launch_send_progress (guestfs_h *g, int perdozen);
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index b0038c6a9..30ed6b1f9 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -60,7 +60,6 @@ struct backend_direct_data {
};
static int is_openable (guestfs_h *g, const char *path, int flags);
-static char *make_appliance_dev (guestfs_h *g);
static void print_qemu_command_line (guestfs_h *g, char **argv);
static char *
@@ -229,7 +228,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
int uefi_flags;
CLEANUP_FREE char *kernel = NULL, *initrd = NULL, *appliance = NULL;
int has_appliance_drive;
- CLEANUP_FREE char *appliance_dev = NULL;
+ CLEANUP_FREE char *appliance_uuid = NULL;
uint32_t size;
CLEANUP_FREE void *buf = NULL;
struct drive *drv;
@@ -555,7 +554,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
ADD_CMDLINE ("-device");
ADD_CMDLINE ("scsi-hd,drive=appliance");
- appliance_dev = make_appliance_dev (g);
+ appliance_uuid = guestfs_int_ext4fs_uuid (g, appliance);
+ if (!appliance_uuid)
+ goto cleanup0;
}
/* Create the virtio serial bus. */
@@ -598,7 +599,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
if (!has_kvm || force_tcg)
flags |= APPLIANCE_COMMAND_LINE_IS_TCG;
ADD_CMDLINE_STRING_NODUP
- (guestfs_int_appliance_command_line (g, appliance_dev, flags));
+ (guestfs_int_appliance_command_line (g, appliance_uuid, flags));
/* Note: custom command line parameters must come last so that
* qemu -set parameters can modify previously added options.
@@ -852,33 +853,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
return -1;
}
-/* Calculate the appliance device name.
- *
- * The easy thing would be to use g->nr_drives (indeed, that's what we
- * used to do). However this breaks if some of the drives being added
- * use the deprecated 'iface' parameter. To further add confusion,
- * the format of the 'iface' parameter has never been defined, but
- * given existing usage we can assume it has one of only three values:
- * NULL, "ide" or "virtio" (which means virtio-blk). See RHBZ#975797.
- */
-static char *
-make_appliance_dev (guestfs_h *g)
-{
- size_t i, index = 0;
- struct drive *drv;
- char dev[64] = "/dev/sd";
-
- /* Calculate the index of the drive. */
- ITER_DRIVES (g, i, drv) {
- if (drv->iface == NULL || STREQ (drv->iface, "ide"))
- index++;
- }
-
- guestfs_int_drive_name (index, &dev[7]);
-
- return safe_strdup (g, dev); /* Caller frees. */
-}
-
/* This is called from the forked subprocess just before qemu runs, so
* it can just print the message straight to stderr, where it will be
* picked up and funnelled through the usual appliance event API.
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index f66c8e0ef..8a95cbbe5 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -136,7 +136,7 @@ struct libvirt_xml_params {
char *kernel; /* paths to kernel and initrd */
char *initrd;
char *appliance_overlay; /* path to qcow2 overlay backed by appliance */
- char appliance_dev[64]; /* appliance device name */
+ char *appliance_uuid; /* appliance volume UUID */
size_t appliance_index; /* index of appliance */
bool enable_svirt; /* false if we decided to disable sVirt */
bool current_proc_is_root; /* true = euid is root */
@@ -303,6 +303,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
.kernel = NULL,
.initrd = NULL,
.appliance_overlay = NULL,
+ .appliance_uuid = NULL,
};
CLEANUP_FREE xmlChar *xml = NULL;
CLEANUP_FREE char *appliance = NULL;
@@ -582,8 +583,9 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
debug (g, "create libvirt XML");
params.appliance_index = g->nr_drives;
- strcpy (params.appliance_dev, "/dev/sd");
- guestfs_int_drive_name (params.appliance_index, ¶ms.appliance_dev[7]);
+ params.appliance_uuid = guestfs_int_ext4fs_uuid (g, appliance);
+ if (!params.appliance_uuid)
+ goto cleanup;
params.enable_svirt = ! is_custom_hv (g);
xml = construct_libvirt_xml (g, ¶ms);
@@ -690,6 +692,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
free (params.kernel);
free (params.initrd);
free (params.appliance_overlay);
+ free (params.appliance_uuid);
return 0;
@@ -715,6 +718,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
free (params.kernel);
free (params.initrd);
free (params.appliance_overlay);
+ free (params.appliance_uuid);
g->state = CONFIG;
@@ -1202,7 +1206,8 @@ construct_libvirt_xml_boot (guestfs_h *g,
flags = 0;
if (!params->data->is_kvm)
flags |= APPLIANCE_COMMAND_LINE_IS_TCG;
- cmdline = guestfs_int_appliance_command_line (g, params->appliance_dev, flags);
+ cmdline = guestfs_int_appliance_command_line (g, params->appliance_uuid,
+ flags);
start_element ("os") {
start_element ("type") {
@@ -1743,6 +1748,10 @@ construct_libvirt_xml_appliance (guestfs_h *g,
const struct libvirt_xml_params *params,
xmlTextWriterPtr xo)
{
+ char dev[64] = "sd";
+
+ guestfs_int_drive_name (params->appliance_index, &dev[2]);
+
start_element ("disk") {
attribute ("type", "file");
attribute ("device", "disk");
@@ -1752,7 +1761,7 @@ construct_libvirt_xml_appliance (guestfs_h *g,
} end_element ();
start_element ("target") {
- attribute ("dev", ¶ms->appliance_dev[5]);
+ attribute ("dev", dev);
attribute ("bus", "scsi");
} end_element ();
diff --git a/m4/guestfs_appliance.m4 b/m4/guestfs_appliance.m4
index 890b1999c..20a0957cc 100644
--- a/m4/guestfs_appliance.m4
+++ b/m4/guestfs_appliance.m4
@@ -34,7 +34,7 @@ then you have to --disable-appliance as well, since the appliance contains
the daemon inside it.])
fi
-dnl Check for supermin >= 5.1.0.
+dnl Check for supermin >= 5.1.18.
AC_PATH_PROG([SUPERMIN],[supermin],[no])
dnl Pass supermin --packager-config option.
--
2.12.0
More information about the Libguestfs
mailing list