[Libguestfs] [PATCH v2 3/4] appliance: Pass root=UUID=<uuid> instead of appliance device name (RHBZ#1804207).
Richard W.M. Jones
rjones at redhat.com
Thu Mar 5 13:15:32 UTC 2020
Appliance device names are not reliable since the kernel no longer
enumerates virtio-scsi devices serially. Instead get the UUID of the
appliance and pass this as the parameter.
Note this requires supermin >= 5.1.18 (from around July 2017).
---
docs/guestfs-building.pod | 2 +-
lib/appliance-kcmdline.c | 57 ++++++++++++++++++++++++++++++++++-----
lib/guestfs-internal.h | 2 +-
lib/launch-direct.c | 34 +----------------------
lib/launch-libvirt.c | 19 +++++++------
m4/guestfs-appliance.m4 | 3 ++-
6 files changed, 67 insertions(+), 50 deletions(-)
diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod
index 005626515..8e3da1d32 100644
--- a/docs/guestfs-building.pod
+++ b/docs/guestfs-building.pod
@@ -83,7 +83,7 @@ I<Required>.
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/appliance-kcmdline.c b/lib/appliance-kcmdline.c
index 8eb4999af..211cc4687 100644
--- a/lib/appliance-kcmdline.c
+++ b/lib/appliance-kcmdline.c
@@ -26,8 +26,10 @@
#include <stdlib.h>
#include <stdbool.h>
#include <string.h>
+#include <unistd.h>
#include "c-ctype.h"
+#include "ignore-value.h"
#include "guestfs.h"
#include "guestfs-internal.h"
@@ -54,15 +56,53 @@
#define EARLYPRINTK "earlyprintk=pl011,0x9000000"
#endif
+COMPILE_REGEXP (re_uuid, "UUID=([-0-9a-f]+)", 0)
+
+static void
+read_uuid (guestfs_h *g, void *retv, const char *line, size_t len)
+{
+ char **ret = retv;
+
+ *ret = match1 (g, line, re_uuid);
+}
+
+/**
+ * Given a disk image containing an extX filesystem, return the UUID.
+ * The L<file(1)> command does the hard work.
+ */
+static char *
+get_root_uuid (guestfs_h *g, const char *appliance)
+{
+ CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g);
+ char *ret = NULL;
+ int r;
+
+ guestfs_int_cmd_add_arg (cmd, "file");
+ guestfs_int_cmd_add_arg (cmd, "--");
+ guestfs_int_cmd_add_arg (cmd, appliance);
+ guestfs_int_cmd_set_stdout_callback (cmd, read_uuid, &ret, 0);
+ r = guestfs_int_cmd_run (cmd);
+ if (r == -1) {
+ if (ret) free (ret);
+ return NULL;
+ }
+ if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
+ guestfs_int_external_command_failed (g, r, "file", NULL);
+ if (ret) free (ret);
+ return NULL;
+ }
+
+ return ret;
+}
+
/**
* Construct the Linux command line passed to the appliance. This is
* used by the C<direct> and C<libvirt> backends, and is simply
* 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> parameter is the filename of the appliance
+ * (could be NULL) from which we obtain the root UUID.
*
* The C<flags> parameter can contain the following flags logically
* or'd together (or 0):
@@ -80,7 +120,8 @@
* 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,
int flags)
{
CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (argv);
@@ -164,8 +205,12 @@ 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) {
+ CLEANUP_FREE char *uuid = get_root_uuid (g, appliance);
+ if (!uuid)
+ return NULL;
+ guestfs_int_add_sprintf (g, &argv, "root=UUID=%s", uuid);
+ }
/* SELinux - deprecated setting, never worked and should not be enabled. */
if (g->selinux)
diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index 8c6affa54..fe761a784 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -688,7 +688,7 @@ extern int guestfs_int_build_appliance (guestfs_h *g, char **kernel, char **init
const char *guestfs_int_get_cpu_model (int kvm);
/* appliance-kcmdline.c */
-extern char *guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev, int flags);
+extern char *guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance, int flags);
#define APPLIANCE_COMMAND_LINE_IS_TCG 1
/* appliance-uefi.c */
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index 1718041a5..2de57ea32 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -61,8 +61,6 @@ struct backend_direct_data {
char guestfsd_sock[UNIX_PATH_MAX]; /* Path to daemon socket. */
};
-static char *make_appliance_dev (guestfs_h *g);
-
static char *
create_cow_overlay_direct (guestfs_h *g, void *datav, struct drive *drv)
{
@@ -382,7 +380,6 @@ 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;
uint32_t size;
CLEANUP_FREE void *buf = NULL;
struct hv_param *hp;
@@ -636,8 +633,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
append_list ("scsi-hd");
append_list ("drive=appliance");
} end_list ();
-
- appliance_dev = make_appliance_dev (g);
}
/* Create the virtio serial bus. */
@@ -697,7 +692,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
flags = 0;
if (!has_kvm || force_tcg)
flags |= APPLIANCE_COMMAND_LINE_IS_TCG;
- append = guestfs_int_appliance_command_line (g, appliance_dev, flags);
+ append = guestfs_int_appliance_command_line (g, appliance, flags);
arg ("-append", append);
/* Note: custom command line parameters must come last so that
@@ -961,33 +956,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. */
-}
-
static int
shutdown_direct (guestfs_h *g, void *datav, int check_for_errors)
{
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index 49786fdd9..cf83657a5 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -153,8 +153,9 @@ struct backend_libvirt_data {
*/
struct libvirt_xml_params {
struct backend_libvirt_data *data;
- char *kernel; /* paths to kernel and initrd */
+ char *kernel; /* paths to kernel, initrd and appliance */
char *initrd;
+ char *appliance;
char *appliance_overlay; /* path to qcow2 overlay backed by appliance */
char appliance_dev[64]; /* appliance device name */
size_t appliance_index; /* index of appliance */
@@ -323,10 +324,10 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
.data = data,
.kernel = NULL,
.initrd = NULL,
+ .appliance = NULL,
.appliance_overlay = NULL,
};
CLEANUP_FREE xmlChar *xml = NULL;
- CLEANUP_FREE char *appliance = NULL;
struct sockaddr_un addr;
struct drive *drv;
size_t i;
@@ -486,18 +487,18 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
debug (g, "build appliance");
if (guestfs_int_build_appliance (g, ¶ms.kernel, ¶ms.initrd,
- &appliance) == -1)
+ ¶ms.appliance) == -1)
goto cleanup;
guestfs_int_launch_send_progress (g, 3);
TRACE0 (launch_build_libvirt_appliance_end);
/* Note that appliance can be NULL if using the old-style appliance. */
- if (appliance) {
+ if (params.appliance) {
#ifndef APPLIANCE_FORMAT_AUTO
- params.appliance_overlay = make_qcow2_overlay (g, appliance, "raw");
+ params.appliance_overlay = make_qcow2_overlay (g, params.appliance, "raw");
#else
- params.appliance_overlay = make_qcow2_overlay (g, appliance, NULL);
+ params.appliance_overlay = make_qcow2_overlay (g, params.appliance, NULL);
#endif
if (!params.appliance_overlay)
goto cleanup;
@@ -714,7 +715,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
goto cleanup;
}
- if (appliance)
+ if (params.appliance)
guestfs_int_add_dummy_appliance_drive (g);
TRACE0 (launch_libvirt_end);
@@ -726,6 +727,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
free (params.kernel);
free (params.initrd);
+ free (params.appliance);
free (params.appliance_overlay);
return 0;
@@ -751,6 +753,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
free (params.kernel);
free (params.initrd);
+ free (params.appliance);
free (params.appliance_overlay);
g->state = CONFIG;
@@ -1215,7 +1218,7 @@ 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, flags);
start_element ("os") {
if (params->data->firmware)
diff --git a/m4/guestfs-appliance.m4 b/m4/guestfs-appliance.m4
index 827a83357..daed959af 100644
--- a/m4/guestfs-appliance.m4
+++ b/m4/guestfs-appliance.m4
@@ -34,7 +34,8 @@ 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.
+dnl We don't check it, but it must support the root=UUID=... notation.
AC_PATH_PROG([SUPERMIN],[supermin],[no])
dnl Pass supermin --packager-config option.
--
2.25.0
More information about the Libguestfs
mailing list