[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libguestfs] [PATCH 3/5] inspect: Add drive naming hints



We currently use a heuristic to guess how drive names we find referenced in the
guest map to drive names in the appliance. If this heuristic fails it can cause
inspection to fail.

This change adds a new 'name' option to add_drive_opts, which allows the user to
explicitly pass the name of a drive to libguestfs if it is known. This change
also updates the fstab-parsing inspection code to use this information if it is
available.
---
 generator/generator_actions.ml    |    7 ++++-
 regressions/test-inspect-fstab.sh |   40 ++++++++++++++++++++++++---
 src/guestfs-internal.h            |    1 +
 src/inspect_fs_unix.c             |   55 ++++++++++++++++++++++++-------------
 src/launch.c                      |    4 +++
 5 files changed, 83 insertions(+), 24 deletions(-)

diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
index f76a404..a5f116c 100644
--- a/generator/generator_actions.ml
+++ b/generator/generator_actions.ml
@@ -995,7 +995,7 @@ be mountable but require special options.  Filesystems may
 not all belong to a single logical operating system
 (use C<guestfs_inspect_os> to look for OSes).");
 
-  ("add_drive_opts", (RErr, [String "filename"], [Bool "readonly"; String "format"; String "iface"]), -1, [FishAlias "add"],
+  ("add_drive_opts", (RErr, [String "filename"], [Bool "readonly"; String "format"; String "iface"; String "name"]), -1, [FishAlias "add"],
    [],
    "add an image to examine or modify",
    "\
@@ -1038,6 +1038,11 @@ this security hole.
 This rarely-used option lets you emulate the behaviour of the
 deprecated C<guestfs_add_drive_with_if> call (q.v.)
 
+=item C<name>
+
+The name the drive had in the original guest, e.g. /dev/sdb. This is used as a
+hint to the guest inspection process if it is available.
+
 =back");
 
   ("inspect_get_windows_systemroot", (RString "systemroot", [Device "root"], []), -1, [],
diff --git a/regressions/test-inspect-fstab.sh b/regressions/test-inspect-fstab.sh
index fb28415..0f4dbc2 100755
--- a/regressions/test-inspect-fstab.sh
+++ b/regressions/test-inspect-fstab.sh
@@ -49,15 +49,11 @@ $guestfish -a test1.img <<'EOF'
   upload test.fstab /etc/fstab
 EOF
 
-rm test.fstab
-
 # This will give a warning, but should not fail.
 $guestfish -a test1.img -i <<'EOF' | sort > test.output
   inspect-get-mountpoints /dev/VG/Root
 EOF
 
-rm test1.img
-
 if [ "$(cat test.output)" != "/: /dev/VG/Root
 /boot: /dev/vda1
 /nosuchfile: /dev/VG/LV1
@@ -67,4 +63,40 @@ if [ "$(cat test.output)" != "/: /dev/VG/Root
     exit 1
 fi
 
+# Test device name hints
+
+cat <<'EOF' > test.fstab
+/dev/VG/Root / ext2 default 0 0
+
+# Device name which requires a hint
+/dev/xvdg1 /boot ext2 default 0 0
+EOF
+
+$guestfish -a test1.img <<'EOF'
+  run
+  mount-options "" /dev/VG/Root /
+  upload test.fstab /etc/fstab
+EOF
+
+$guestfish <<'EOF' | sort > test.output
+  add-drive-opts test1.img readonly:true name:xvdg
+  run
+  inspect-os
+  inspect-get-mountpoints /dev/VG/Root
+EOF
+
+# Note that the test output below isn't exactly what I expect, because the
+# output of inspect-os is *after* the output of inspect-get-mountpoints. I
+# didn't get to the bottom of why this happens, but it has no bearing on the
+# test.
+if [ "$(cat test.output)" != "/: /dev/VG/Root
+/boot: /dev/vda1
+/dev/VG/Root" ]; then
+    echo "$0: error: unexpected output from inspect-get-mountpoints command"
+    cat test.output
+    exit 1
+fi
+
+rm test.fstab
+rm test1.img
 rm test.output
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 43b811c..4f1ca16 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -155,6 +155,7 @@ struct drive {
   int readonly;
   char *format;
   char *iface;
+  char *name;
 
   struct drive *next;
 };
diff --git a/src/inspect_fs_unix.c b/src/inspect_fs_unix.c
index 1169b1e..0410cf9 100644
--- a/src/inspect_fs_unix.c
+++ b/src/inspect_fs_unix.c
@@ -106,7 +106,7 @@ compile_regexps (void)
            "Scientific Linux.*release (\\d+)", 0);
   COMPILE (re_major_minor, "(\\d+)\\.(\\d+)", 0);
   COMPILE (re_aug_seq, "/\\d+$", 0);
-  COMPILE (re_xdev, "^/dev/(?:h|s|v|xv)d([a-z]+)(\\d*)$", 0);
+  COMPILE (re_xdev, "^/dev/(h|s|v|xv)d([a-z]+)(\\d*)$", 0);
   COMPILE (re_freebsd, "^/dev/ad(\\d+)s(\\d+)([a-z])$", 0);
 }
 
@@ -756,7 +756,7 @@ static char *
 resolve_fstab_device (guestfs_h *g, const char *spec)
 {
   char *device = NULL;
-  char *slice, *disk, *part;
+  char *type, *slice, *disk, *part;
 
   if (STRPREFIX (spec, "/dev/mapper/")) {
     /* LVM2 does some strange munging on /dev/mapper paths for VGs and
@@ -771,33 +771,50 @@ resolve_fstab_device (guestfs_h *g, const char *spec)
      */
     device = guestfs_lvm_canonical_lv_name (g, spec);
   }
-  else if (match2 (g, spec, re_xdev, &disk, &part)) {
-    /* disk: ([a-z]+)
+  else if (match3 (g, spec, re_xdev, &type, &disk, &part)) {
+    /* type: (h|s|v|xv)
+     * disk: ([a-z]+)
      * part: (\d*) */
     char **devices = guestfs_list_devices (g);
     if (devices == NULL)
       return NULL;
 
-    /* Count how many disks the libguestfs appliance has */
-    size_t count;
-    for (count = 0; devices[count] != NULL; count++)
-      ;
+    /* Check any hints we were passed for a non-heuristic mapping */
+    char *name = safe_asprintf (g, "%sd%s", type, disk);
+    size_t i = 0;
+    struct drive *drive = g->drives;
+    while (drive) {
+      if (drive->name && STREQ(drive->name, name)) {
+        device = safe_asprintf (g, "%s%s", devices[i], part);
+        break;
+      }
 
-    /* Calculate the numerical index of the disk */
-    size_t i = disk[0] - 'a';
-    for (char *p = disk + 1; *p != '\0'; p++) {
-      i += 1; i *= 26;
-      i += *p - 'a';
+      i++; drive = drive->next;
     }
+    free (name);
+
+    /* Guess the appliance device name if we didn't find a matching hint */
+    if (!device) {
+      /* Count how many disks the libguestfs appliance has */
+      size_t count;
+      for (count = 0; devices[count] != NULL; count++)
+        ;
+
+      /* Calculate the numerical index of the disk */
+      i = disk[0] - 'a';
+      for (char *p = disk + 1; *p != '\0'; p++) {
+        i += 1; i *= 26;
+        i += *p - 'a';
+      }
 
-    /* Check the index makes sense wrt the number of disks the appliance has.
-     * If it does, map it to an appliance disk. */
-    if (i < count) {
-      size_t len = strlen (devices[i]) + strlen (part) + 1;
-      device = safe_malloc (g, len);
-      snprintf (device, len, "%s%s", devices[i], part);
+      /* Check the index makes sense wrt the number of disks the appliance has.
+       * If it does, map it to an appliance disk. */
+      if (i < count) {
+        device = safe_asprintf (g, "%s%s", devices[i], part);
+      }
     }
 
+    free (type);
     free (disk);
     free (part);
     guestfs___free_string_list (devices);
diff --git a/src/launch.c b/src/launch.c
index 6462adc..bf5b14c 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -255,6 +255,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   int readonly;
   char *format;
   char *iface;
+  char *name;
 
   if (strchr (filename, ',') != NULL) {
     error (g, _("filename cannot contain ',' (comma) character"));
@@ -267,6 +268,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
            ? safe_strdup (g, optargs->format) : NULL;
   iface = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_IFACE_BITMASK
           ? safe_strdup (g, optargs->iface) : safe_strdup (g, DRIVE_IF);
+  name = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_NAME_BITMASK
+          ? safe_strdup (g, optargs->name) : NULL;
 
   if (format && !valid_format_iface (format)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
@@ -287,6 +290,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   (*i)->readonly = readonly;
   (*i)->format = format;
   (*i)->iface = iface;
+  (*i)->name = name;
   (*i)->next = NULL;
 
   return 0;

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]