From 3117b71d50ada119c70348b4640f7bb53935b112621670c230f5d481ccb299ba Mon Sep 17 00:00:00 2001 From: Olaf Hering Date: Wed, 10 Oct 2012 14:55:01 +0000 Subject: [PATCH] - change default_attach_method from appliance to libvirt if the distro kernel includes virtio-scsi. This is true for 12.2+ In 12.1 and older the attach_method remains appliance. The reason for this change is the creation of XML for libvirt, which is cumbersome for virtio-blk. OBS-URL: https://build.opensuse.org/package/show/Virtualization/libguestfs?expand=0&rev=129 --- ...force-virtio_blk-in-old-guest-kernel.patch | 350 +++++++++++++++--- libguestfs.changes | 9 + libguestfs.spec | 6 +- 3 files changed, 313 insertions(+), 52 deletions(-) diff --git a/1000-force-virtio_blk-in-old-guest-kernel.patch b/1000-force-virtio_blk-in-old-guest-kernel.patch index 40d8e13..3ab26fe 100644 --- a/1000-force-virtio_blk-in-old-guest-kernel.patch +++ b/1000-force-virtio_blk-in-old-guest-kernel.patch @@ -1,107 +1,355 @@ -From 04b36384e91cf0e6242be9daa1d06d269cb9cf3d Mon Sep 17 00:00:00 2001 +From 6d90239469c6de90c8a3fcb885ad2be30bf364f9 Mon Sep 17 00:00:00 2001 From: Olaf Hering Date: Mon, 3 Sep 2012 19:50:44 +0200 Subject: [PATCH] force virtio_blk in old guest kernel Signed-off-by: Olaf Hering --- - src/launch-appliance.c | 24 ++++++++++++++++++++++++ - test-tool/test-tool.c | 15 ++++++++++----- - 2 files changed, 34 insertions(+), 5 deletions(-) + df/output.c | 13 +++++++++---- + fish/options.c | 8 +++++++- + src/guestfs-internal.h | 2 ++ + src/guestfs.c | 16 ++++++++++++++++ + src/inspect-fs-unix.c | 4 ++-- + src/launch-appliance.c | 3 +++ + src/launch-libvirt.c | 36 +++++++++++++++++++++++++++++++----- + test-tool/test-tool.c | 16 +++++++++++++--- + 8 files changed, 83 insertions(+), 15 deletions(-) -diff --git a/src/launch-appliance.c b/src/launch-appliance.c -index e353e05..f037d15 100644 ---- a/src/launch-appliance.c -+++ b/src/launch-appliance.c -@@ -869,6 +869,28 @@ is_openable (guestfs_h *g, const char *path, int flags) - return 1; +diff --git a/df/output.c b/df/output.c +index 5729dd4..f1713bb 100644 +--- a/df/output.c ++++ b/df/output.c +@@ -42,6 +42,7 @@ + #include "virt-df.h" + + static void write_csv_field (const char *field); ++static int use_virtio_blk; + + void + print_title (void) +@@ -82,7 +83,7 @@ print_title (void) + } } +-static char *adjust_device_offset (const char *device, int offset); ++static char *adjust_device_offset (guestfs_h *g, const char *device, int offset); + + void + print_stat (const char *name, const char *uuid_param, +@@ -110,7 +111,7 @@ print_stat (const char *name, const char *uuid_param, + exit (EXIT_FAILURE); + if (offset >= 0) { + char *p = dev; +- dev = adjust_device_offset (p, offset); ++ dev = adjust_device_offset (g, p, offset); + free (p); + } + +@@ -237,8 +238,9 @@ write_csv_field (const char *field) + static char *drive_name (int index, char *ret); + + static char * +-adjust_device_offset (const char *device, int offset) ++adjust_device_offset (guestfs_h *g, const char *device, int offset) + { ++ ; + int index; + int part_num; + char *whole_device; +@@ -282,7 +284,10 @@ adjust_device_offset (const char *device, int offset) + exit (EXIT_FAILURE); + } + +- strcpy (ret, "/dev/sd"); ++#ifdef GUESTFS_QEMU_NO_VIRTIO_BLK ++ use_virtio_blk = 1; ++#endif ++ strcpy (ret, use_virtio_blk ? "/dev/vd" : "/dev/sd"); + drive_name (index, &ret[7]); + len = strlen (ret); + if (part_num > 0) +diff --git a/fish/options.c b/fish/options.c +index 0db790e..8f9aada 100644 +--- a/fish/options.c ++++ b/fish/options.c +@@ -27,6 +27,8 @@ + + #include "options.h" + ++static int use_virtio_blk; ++ + char + add_drives (struct drv *drv, char next_drive) + { +@@ -40,13 +42,17 @@ add_drives (struct drv *drv, char next_drive) + exit (EXIT_FAILURE); + } + ++#ifdef GUESTFS_QEMU_NO_VIRTIO_BLK ++ use_virtio_blk = 1; ++#endif ++ + if (drv) { + next_drive = add_drives (drv->next, next_drive); + + free (drv->device); + drv->device = NULL; + +- if (asprintf (&drv->device, "/dev/sd%c", next_drive) == -1) { ++ if (asprintf (&drv->device, "/dev/%s%c", use_virtio_blk ? "vd" : "sd", next_drive) == -1) { + perror ("asprintf"); + exit (EXIT_FAILURE); + } +diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h +index afc3be4..3ecc821 100644 +--- a/src/guestfs-internal.h ++++ b/src/guestfs-internal.h +@@ -289,6 +289,8 @@ struct guestfs_h + void *connv; /* libvirt connection (really virConnectPtr) */ + void *domv; /* libvirt domain (really virDomainPtr) */ + } virt; ++ ++ int use_virtio_blk; + }; + + /* Per-filesystem data stored for inspect_os. */ +diff --git a/src/guestfs.c b/src/guestfs.c +index cb9ee6b..be6d849 100644 +--- a/src/guestfs.c ++++ b/src/guestfs.c +@@ -170,6 +170,22 @@ guestfs_create (void) + } + } + +/* + * Currently virtio_scsi is forced if qemu in the host supports this + * feature. This test does however not take the capabilities of the started + * guest into account. As a result no disks will be found if the guest + * kernel is older than 3.4. + */ -+static void qemu_force_virtio_blk(guestfs_h *g) -+{ +#ifdef GUESTFS_QEMU_NO_VIRTIO_BLK -+ static const char env_string[] = "GUESTFS_QEMU_NO_VIRTIO_BLK"; -+ char *p = getenv(env_string); -+ if (p) { -+ if (g->verbose) -+ guestfs___print_timestamped_message (g, "SuSE: %s in environment, preserving virtio-scsi setting.", env_string); -+ } else { -+ if (g->verbose) -+ guestfs___print_timestamped_message (g, "SuSE: %s not in environment, preventing virtio-scsi usage in old guest kernel.", env_string); -+ g->app.virtio_scsi = 2; -+ } ++ static const char env_string[] = "GUESTFS_QEMU_NO_VIRTIO_BLK"; ++ str = getenv(env_string); ++ g->use_virtio_blk = str == NULL; ++ if (str) ++ debug (g, "SuSE: %s in environment, preserving virtio-scsi setting.", env_string); ++ else ++ debug (g, "SuSE: %s not in environment, preventing virtio-scsi usage in old guest kernel.", env_string); +#endif -+} + - /* Returns 1 = use virtio-scsi, or 0 = use virtio-blk. */ - static int - qemu_supports_virtio_scsi (guestfs_h *g) -@@ -891,6 +913,8 @@ qemu_supports_virtio_scsi (guestfs_h *g) + /* Start with large serial numbers so they are easy to spot + * inside the protocol. + */ +diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c +index 2152484..593e265 100644 +--- a/src/inspect-fs-unix.c ++++ b/src/inspect-fs-unix.c +@@ -1420,7 +1420,7 @@ resolve_fstab_device_diskbyid (guestfs_h *g, const char *part, + return 0; + + /* Make the partition name and check it exists. */ +- device = safe_asprintf (g, "/dev/sda%s", part); ++ device = safe_asprintf (g, "/dev/%sa%s", g->use_virtio_blk ? "vd" : "sd", part); + if (!is_partition (g, device)) { + free (device); + return 0; +@@ -1497,7 +1497,7 @@ resolve_fstab_device (guestfs_h *g, const char *spec, Hash_table *md_map) + if (disk_i != -1 && disk_i <= 26 && + slice_i > 0 && slice_i <= 1 /* > 4 .. see comment above */ && + part_i >= 0 && part_i < 26) { +- device = safe_asprintf (g, "/dev/sd%c%d", disk_i + 'a', part_i + 5); ++ device = safe_asprintf (g, "/dev/%s%c%d", g->use_virtio_blk ? "vd" : "sd", disk_i + 'a', part_i + 5); + } + } + else if ((part = match1 (g, spec, re_diskbyid)) != NULL) { +diff --git a/src/launch-appliance.c b/src/launch-appliance.c +index e353e05..1a1ddef 100644 +--- a/src/launch-appliance.c ++++ b/src/launch-appliance.c +@@ -891,6 +891,9 @@ qemu_supports_virtio_scsi (guestfs_h *g) g->app.virtio_scsi = 3; } -+ qemu_force_virtio_blk(g); ++ if (g->use_virtio_blk) ++ g->app.virtio_scsi = 2; + return g->app.virtio_scsi == 1; } +diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c +index d678266..1b3b681 100644 +--- a/src/launch-libvirt.c ++++ b/src/launch-libvirt.c +@@ -117,6 +117,12 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri) + void *buf = NULL; + int disable_svirt = is_custom_qemu (g); + ++#ifdef GUESTFS_QEMU_NO_VIRTIO_BLK ++ if (g->use_virtio_blk) { ++ error (g, "Using libvirt is not possible with this binary package due to forced virtio-blk usage."); ++ return -1; ++ } ++#endif + /* At present you must add drives before starting the appliance. In + * future when we enable hotplugging you won't need to do this. + */ +@@ -612,11 +618,12 @@ construct_libvirt_xml_boot (guestfs_h *g, xmlTextWriterPtr xo, + + snprintf (buf, sizeof buf, + LINUX_CMDLINE +- "root=/dev/sd%s " /* (root) */ ++ "root=/dev/%s%s " /* (root) */ + "%s " /* (selinux) */ + "%s " /* (verbose) */ + "TERM=%s " /* (TERM environment variable) */ + "%s", /* (append) */ ++ g->use_virtio_blk ? "vd" : "sd", + appliance_root, + g->selinux ? "selinux=1 enforcing=0" : "selinux=0", + g->verbose ? "guestfs_verbose=1" : "", +@@ -700,8 +707,17 @@ construct_libvirt_xml_devices (guestfs_h *g, xmlTextWriterPtr xo, + XMLERROR (-1, xmlTextWriterEndElement (xo)); + } + +- /* virtio-scsi controller. */ + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "controller")); ++ if (g->use_virtio_blk) { ++ /* virtio-blk controller. */ ++ XMLERROR (-1, ++ xmlTextWriterWriteAttribute (xo, BAD_CAST "type", ++ BAD_CAST "ide")); ++ XMLERROR (-1, ++ xmlTextWriterWriteAttribute (xo, BAD_CAST "index", ++ BAD_CAST "0")); ++ } else { ++ /* virtio-scsi controller. */ + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "type", + BAD_CAST "scsi")); +@@ -711,6 +727,7 @@ construct_libvirt_xml_devices (guestfs_h *g, xmlTextWriterPtr xo, + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "model", + BAD_CAST "virtio-scsi")); ++ } + XMLERROR (-1, xmlTextWriterEndElement (xo)); + + /* Disks. */ +@@ -784,6 +801,7 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr xo, + char scsi_target[64]; + char *path = NULL; + char *format = NULL; ++ char *bus = "scsi"; + int is_host_device; + + /* XXX We probably could support this if we thought about it some more. */ +@@ -792,6 +810,11 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr xo, + goto err; + } + ++ if (g->use_virtio_blk) { ++ drive_name[0] = 'v'; ++ bus = "virtio"; ++ } ++ + guestfs___drive_name (drv_index, &drive_name[2]); + snprintf (scsi_target, sizeof scsi_target, "%zu", drv_index); + +@@ -844,7 +867,7 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr xo, + BAD_CAST drive_name)); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "bus", +- BAD_CAST "scsi")); ++ BAD_CAST bus)); + XMLERROR (-1, xmlTextWriterEndElement (xo)); + + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "driver")); +@@ -1018,6 +1041,9 @@ construct_libvirt_xml_qemu_cmdline (guestfs_h *g, xmlTextWriterPtr xo) + char attr[256]; + struct qemu_param *qp; + char *p; ++ char *bus = "scsi"; ++ ++ bus = g->use_virtio_blk ? "ide" : "scsi"; + + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "qemu:commandline")); + +@@ -1028,7 +1054,7 @@ construct_libvirt_xml_qemu_cmdline (guestfs_h *g, xmlTextWriterPtr xo) + for (drv = g->drives, drv_index = 0; drv; drv = drv->next, drv_index++) { + if (drv->readonly) { + snprintf (attr, sizeof attr, +- "drive.drive-scsi0-0-%zu-0.snapshot=on", drv_index); ++ "drive.drive-%s0-0-%zu-0.snapshot=on", bus, drv_index); + + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "qemu:arg")); + XMLERROR (-1, +@@ -1045,7 +1071,7 @@ construct_libvirt_xml_qemu_cmdline (guestfs_h *g, xmlTextWriterPtr xo) + } + + snprintf (attr, sizeof attr, +- "drive.drive-scsi0-0-%zu-0.snapshot=on", drv_index); ++ "drive.drive-%s0-0-%zu-0.snapshot=on", bus, drv_index); + + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "qemu:arg")); + XMLERROR (-1, diff --git a/test-tool/test-tool.c b/test-tool/test-tool.c -index 4b764ee..2ba9977 100644 +index 6f26e33..56523b3 100644 --- a/test-tool/test-tool.c +++ b/test-tool/test-tool.c -@@ -35,6 +35,11 @@ +@@ -35,6 +35,7 @@ #include -+#ifdef GUESTFS_QEMU_NO_VIRTIO_BLK -+#define VDISK_KERNEL_NAME "vda" -+#else -+#define VDISK_KERNEL_NAME "sda" -+#endif ++static int use_virtio_blk; #define _(str) dgettext(PACKAGE, (str)) //#define N_(str) dgettext(PACKAGE, (str)) -@@ -55,7 +60,7 @@ - #define DEFAULT_TIMEOUT 600 +@@ -103,7 +104,11 @@ main (int argc, char *argv[]) + int i; + struct guestfs_version *vers; + char *p; ++ char *disk_name, *partition_name; - static int timeout = DEFAULT_TIMEOUT; --static char tmpf[] = P_tmpdir "/libguestfs-test-tool-sda-XXXXXX"; -+static char tmpf[] = P_tmpdir "/libguestfs-test-tool-" VDISK_KERNEL_NAME "-XXXXXX"; - static guestfs_h *g; ++#ifdef GUESTFS_QEMU_NO_VIRTIO_BLK ++ use_virtio_blk = 1; ++#endif + /* Everyone ignores the documentation, so ... */ + printf (" ************************************************************\n" + " * IMPORTANT NOTICE\n" +@@ -129,6 +134,11 @@ main (int argc, char *argv[]) + exit (EXIT_FAILURE); + } - static void make_files (void); -@@ -240,21 +245,21 @@ main (int argc, char *argv[]) ++ if (asprintf(&disk_name, "/dev/%s", use_virtio_blk ? "vda" : "sda") < 0) ++ exit (EXIT_FAILURE); ++ if (asprintf(&partition_name, "%s1", disk_name) < 0) ++ exit (EXIT_FAILURE); ++ + for (;;) { + c = getopt_long (argc, argv, options, long_options, &option_index); + if (c == -1) break; +@@ -251,19 +261,19 @@ main (int argc, char *argv[]) fflush (stdout); /* Create the filesystem and mount everything. */ - if (guestfs_part_disk (g, "/dev/sda", "mbr") == -1) { -+ if (guestfs_part_disk (g, "/dev/" VDISK_KERNEL_NAME "", "mbr") == -1) { ++ if (guestfs_part_disk (g, disk_name, "mbr") == -1) { fprintf (stderr, _("libguestfs-test-tool: failed to run part-disk\n")); exit (EXIT_FAILURE); } - if (guestfs_mkfs (g, "ext2", "/dev/sda1") == -1) { -+ if (guestfs_mkfs (g, "ext2", "/dev/" VDISK_KERNEL_NAME "1") == -1) { ++ if (guestfs_mkfs (g, "ext2", partition_name) == -1) { fprintf (stderr, _("libguestfs-test-tool: failed to mkfs.ext2\n")); exit (EXIT_FAILURE); } - if (guestfs_mount (g, "/dev/sda1", "/") == -1) { -+ if (guestfs_mount (g, "/dev/" VDISK_KERNEL_NAME "1", "/") == -1) { ++ if (guestfs_mount (g, partition_name, "/") == -1) { fprintf (stderr, -- _("libguestfs-test-tool: failed to mount /dev/sda1 on /\n")); -+ _("libguestfs-test-tool: failed to mount /dev/" VDISK_KERNEL_NAME "1 on /\n")); + _("libguestfs-test-tool: failed to mount /dev/sda1 on /\n")); exit (EXIT_FAILURE); - } - -- -1.7.12 +1.7.12.2 diff --git a/libguestfs.changes b/libguestfs.changes index cd67e26..7c197fb 100644 --- a/libguestfs.changes +++ b/libguestfs.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Wed Oct 10 16:51:32 CEST 2012 - ohering@suse.de + +- change default_attach_method from appliance to libvirt if the + distro kernel includes virtio-scsi. This is true for 12.2+ + In 12.1 and older the attach_method remains appliance. + The reason for this change is the creation of XML for libvirt, + which is cumbersome for virtio-blk. + ------------------------------------------------------------------- Mon Oct 8 20:17:39 CEST 2012 - ohering@suse.de diff --git a/libguestfs.spec b/libguestfs.spec index 673c2fa..29458ff 100644 --- a/libguestfs.spec +++ b/libguestfs.spec @@ -55,7 +55,7 @@ BuildRequires: gperf BuildRequires: libconfig-devel %endif BuildRequires: libtool -BuildRequires: libvirt-devel +BuildRequires: libvirt-devel >= 0.10.2 BuildRequires: ncurses-devel BuildRequires: pcre-devel BuildRequires: pkg-config @@ -369,12 +369,15 @@ CFLAGS="$RPM_OPT_FLAGS -Wno-unused" CXXFLAGS="$RPM_OPT_FLAGS -Wno-unused" # If the kernel happens to have no virtio-scsi force virtio-blk usage in the tools # This is true for kernel.rpm included in 12.1 and older +# Force also attach method "appliance" because constructing libvirt xml is cumbersome for virtio-blk if /sbin/modinfo -k `/sbin/get_kernel_version /boot/vmlinuz` virtio-scsi then : use virtio-scsi, which is the default in libguestfs + default_attach_method="libvirt" else CFLAGS="$CFLAGS -DGUESTFS_QEMU_NO_VIRTIO_BLK" CXXFLAGS="$CXXFLAGS -DGUESTFS_QEMU_NO_VIRTIO_BLK" + default_attach_method="appliance" fi # autoreconf -fi @@ -385,6 +388,7 @@ autoreconf -fi --docdir=%{_defaultdocdir}/guestfs-doc \ --enable-daemon \ --enable-install-daemon \ + --with-default-attach-method="${default_attach_method}" \ --with-qemu=$QEMU \ --without-java \ --disable-appliance \