Libguestfs - Oct 2010 - [PATCH 0/2] /dev/mapper paths should not be returned from C inspection APIs

This is a fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=638899

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
-------------- next part --------------
>From 6ea93c0fd208a60d14306e76f2ca7ebc2a73f49f Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Mon, 25 Oct 2010 12:52:49 +0100
Subject: [PATCH 1/2] New API: lvm-canonical-lv-name: make LV name canonical.

When logical volume names appear in places like /etc/fstab
files they can have the form "/dev/mapper/foo-bar".  This
function takes such names and makes them canonical.

Note that this operation cannot be performed using the current
API, because 'guestfs_stat' does not work on device names, and
we don't really want to make a 'stat-device' call since that
exposes too much non-useful detail about the appliance.

With this patch you can do this:
><fs> debug ll /dev/mapper
total 8
drwxrwxr-x  2 root root   4096 Oct 25 12:51 .
drwxr-xr-x 16 root root   4096 Oct 25 12:51 ..
crw-------  1 root root 10, 62 Oct 25 12:51 control
lrwxrwxrwx  1 root root      7 Oct 25 12:51 vg_f13x64-lv_root -> ../dm-0
lrwxrwxrwx  1 root root      7 Oct 25 12:51 vg_f13x64-lv_swap -> ../dm-1
><fs> lvm-canonical-lv-name /dev/mapper/vg_f13x64-lv_root
/dev/vg_f13x64/lv_root
><fs> lvm-canonical-lv-name /dev/mapper/vg_f13x64-lv_swap
/dev/vg_f13x64/lv_swap
><fs> lvm-canonical-lv-name /dev/mapper/foo
libguestfs: error: lvm_canonical_lv_name: lvm_canonical_lv_name_stub:
/dev/mapper/foo: No such file or directory
><fs> lvm-canonical-lv-name /dev/mapper/control
libguestfs: error: lvm_canonical_lv_name: /dev/mapper/control: not a logical
volume
><fs> lvm-canonical-lv-name /dev/vg_f13x64/lv_root
/dev/vg_f13x64/lv_root
---
 daemon/lvm.c                   |   42 ++++++++++++++++++++++++++++++++++++++++
 generator/generator_actions.ml |   16 +++++++++++++++
 src/MAX_PROC_NR                |    2 +-
 3 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/daemon/lvm.c b/daemon/lvm.c
index 0df27e2..2691daa 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -709,3 +709,45 @@ do_is_lv (const char *device)
   free_strings (lvs);
   return 0;
 }
+
+/* Similar to is_lv above (RHBZ#638899). */
+char *
+do_lvm_canonical_lv_name (const char *device)
+{
+  struct stat stat1, stat2;
+
+  int r = stat (device, &stat1);
+  if (r == -1) {
+    reply_with_perror ("stat: %s", device);
+    return NULL;
+  }
+
+  char **lvs = do_lvs ();
+  if (lvs == NULL)
+    return NULL;
+
+  size_t i;
+  for (i = 0; lvs[i] != NULL; ++i) {
+    r = stat (lvs[i], &stat2);
+    if (r == -1) {
+      reply_with_perror ("stat: %s", lvs[i]);
+      free_strings (lvs);
+      return NULL;
+    }
+    if (stat1.st_rdev == stat2.st_rdev) { /* found it */
+      char *r = strdup (lvs[i]);
+      if (r == NULL) {
+        reply_with_perror ("strdup");
+        free_strings (lvs);
+      }
+      free_strings (lvs);
+      return r;
+    }
+  }
+
+  free_strings (lvs);
+
+  /* not found */
+  reply_with_error ("%s: not a logical volume", device);
+  return NULL;
+}
diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
index 695a73d..38f1db3 100644
--- a/generator/generator_actions.ml
+++ b/generator/generator_actions.ml
@@ -5208,6 +5208,22 @@ see the L<pread(2)> system call.
 
 See also C<guestfs_pread>.");
 
+  ("lvm_canonical_lv_name", (RString "lv", [Device
"lvname"], []), 277, [],
+   [InitBasicFSonLVM, IfAvailable "lvm2", TestOutput (
+    [["lvm_canonical_lv_name"; "/dev/mapper/VG-LV"]],
"/dev/VG/LV");
+    InitBasicFSonLVM, IfAvailable "lvm2", TestOutput (
+    [["lvm_canonical_lv_name"; "/dev/VG/LV"]],
"/dev/VG/LV")],
+   "get canonical name of an LV",
+   "\
+This converts alternative naming schemes for LVs that you
+might find to the canonical name.  For example, C</dev/mapper/VG-LV>
+is converted to C</dev/VG/LV>.
+
+This command returns an error if the C<lvname> parameter does
+not refer to a logical volume.
+
+See also C<guestfs_is_lv>.");
+
 ]
 
 let all_functions = non_daemon_functions @ daemon_functions
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 15007f1..2681747 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-276
+277
-- 
1.7.3.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
-------------- next part --------------
>From d88daf9d6baf70457b60c1eb99a5d530b2ff80ab Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Mon, 25 Oct 2010 12:59:50 +0100
Subject: [PATCH 2/2] /dev/mapper paths should not be returned from C inspection
APIs (RHBZ#638899).

With this patch, /dev/mapper paths do not appear in the output
of guestfs_inspect_os, as you can see from this example:

Welcome to guestfish, the libguestfs filesystem interactive shell for
editing virtual machine filesystems.

Type: 'help' for a list of commands
      'man' to read the manual
      'quit' to quit the shell

Operating system: Fedora release 13 (Goddard)
/dev/vg_f13x64/lv_root mounted on /              <--- NB
/dev/vda1 mounted on /boot
---
 src/inspect.c |   49 ++++++++++++++++++++++++++++++++-----------------
 1 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/src/inspect.c b/src/inspect.c
index 1f4096d..3ffb2bd 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -878,39 +878,54 @@ add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
 }
 
 /* Resolve block device name to the libguestfs device name, eg.
- * /dev/xvdb1 => /dev/vdb1.  This assumes that disks were added in the
- * same order as they appear to the real VM, which is a reasonable
- * assumption to make.  Return things like LV names unchanged (or
- * anything we don't recognize).
+ * /dev/xvdb1 => /dev/vdb1; and /dev/mapper/VG-LV => /dev/VG/LV.  This
+ * assumes that disks were added in the same order as they appear to
+ * the real VM, which is a reasonable assumption to make.  Return
+ * anything we don't recognize unchanged.
  */
 static char *
 resolve_fstab_device (guestfs_h *g, const char *spec)
 {
-  char **devices = guestfs_list_devices (g);
-  if (devices == NULL)
-    return NULL;
+  char *a1;
+  char *device = NULL;
 
-  size_t count;
-  for (count = 0; devices[count] != NULL; count++)
-    ;
+  if (STRPREFIX (spec, "/dev/mapper/")) {
+    /* LVM2 does some strange munging on /dev/mapper paths for VGs and
+     * LVs which contain '-' character:
+     *
+     * ><fs> lvcreate LV--test VG--test 32
+     * ><fs> debug ls /dev/mapper
+     * VG----test-LV----test
+     *
+     * This makes it impossible to reverse those paths directly, so
+     * we have implemented lvm_canonical_lv_name in the daemon.
+     */
+    device = guestfs_lvm_canonical_lv_name (g, spec);
+  }
+  else if ((a1 = match1 (g, spec, re_xdev)) != NULL) {
+    char **devices = guestfs_list_devices (g);
+    if (devices == NULL)
+      return NULL;
+
+    size_t count;
+    for (count = 0; devices[count] != NULL; count++)
+      ;
 
-  char *device = NULL;
-  char *a1 = match1 (g, spec, re_xdev);
-  if (a1) {
     size_t i = a1[0] - 'a'; /* a1[0] is always [a-z] because of regex.
*/
     if (i < count) {
       size_t len = strlen (devices[i]) + strlen (a1) + 16;
       device = safe_malloc (g, len);
       snprintf (device, len, "%s%s", devices[i], &a1[1]);
     }
-  } else {
+
+    free (a1);
+    free_string_list (devices);
+  }
+  else {
     /* Didn't match device pattern, return original spec unchanged. */
     device = safe_strdup (g, spec);
   }
 
-  free (a1);
-  free_string_list (devices);
-
   return device;
 }
 
-- 
1.7.3.1