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_Bl...;. + */ +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, &params.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, &params); @@ -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", &params->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