Add support for Windows dynamic disks (libldm / ldmtool) (RHBZ#782167).

New APIs:

  list-ldm-volumes
  list-ldm-partitions
  ldmtool-create-all
  ldmtool-remove-all
  ldmtool-scan
  ldmtool-scan-devices
  ldmtool-diskgroup-name
  ldmtool-diskgroup-volumes
  ldmtool-diskgroup-disks
  ldmtool-volume-type
  ldmtool-volume-hint
  ldmtool-volume-partitions

README

@@ -112,6 +112,12 @@ For basic functionality and the C tools:
 
 - Linux capabilities library (libcap) (optional)
 
+- libldm and ldmtool (optional)
+  This is used to handle Windows dynamic disks.
+
+- yajl >= 2 (optional)
+  JSON parser, needed to handle the output of ldmtool.
+
 - netpbm, icoutils (optional)
   These programs are used to render icons from guests.
 

appliance/init

@@ -91,6 +91,9 @@ modprobe dm_mod ||:
 lvm vgscan --ignorelockingfailure
 lvm vgchange -ay --ignorelockingfailure
 
+# Scan for Windows dynamic disks.
+ldmtool create all
+
 # Improve virtio-blk performance (RHBZ#509383).
 for f in /sys/block/vd*/queue/rotational; do echo 1 > $f; done
 

appliance/packagelist.in

@@ -35,6 +35,7 @@
   iproute
   iputils
   kernel
+  libldm /* only Fedora has this for now, but we should add it to others later*/
   MAKEDEV
   nilfs-utils
   ntfsprogs
@@ -45,6 +46,7 @@
   systemd /* for /sbin/reboot and udevd */
   vim-minimal
   xz
+  yajl
   zfs-fuse
 #endif /* REDHAT */
 
@@ -61,6 +63,7 @@
   iproute
   libaugeas0
   libhivex0
+  libyajl2
   linux-image
   nilfs-tools
   ntfs-3g
@@ -91,8 +94,9 @@
   reiserfsprogs
   systemd
   vim
-  zfs-fuse
   xz
+  yajl
+  zfs-fuse
 #endif /* ARCHLINUX */
 
 acl

configure.ac

@@ -785,6 +785,13 @@ AS_IF([test "x$enable_fuse" != "xno"],
         ])
 AM_CONDITIONAL([HAVE_FUSE],[test "x$enable_fuse" != "xno"])
 
+dnl Check for yajl JSON library (optional).
+PKG_CHECK_MODULES([YAJL], [yajl >= 2], [
+    AC_SUBST([YAJL_CFLAGS])
+    AC_SUBST([YAJL_LIBS])
+    AC_DEFINE([HAVE_YAJL],[1],[Define to 1 if you have yajl.])
+    ],[AC_MSG_WARN([yajl not found, some features will be disabled])])
+
 dnl Check for C++ (optional, we just use this to test the header works).
 AC_PROG_CXX
 

daemon/Makefile.am

@@ -136,6 +136,7 @@ guestfsd_SOURCES = \
 	is.c \
 	isoinfo.c \
 	labels.c \
+	ldm.c \
 	link.c \
 	ls.c \
 	luks.c \
@@ -185,6 +186,7 @@ guestfsd_LDADD = \
 	libprotocol.a \
 	$(ACL_LIBS) \
 	$(CAP_LIBS) \
+	$(YAJL_LIBS) \
 	$(SELINUX_LIB) \
 	$(AUGEAS_LIBS) \
 	$(HIVEX_LIBS) \
@@ -201,7 +203,8 @@ guestfsd_CPPFLAGS = -I$(top_srcdir)/gnulib/lib -I$(top_builddir)/gnulib/lib
 guestfsd_CFLAGS = \
 	$(WARN_CFLAGS) $(WERROR_CFLAGS) \
 	$(AUGEAS_CFLAGS) \
-	$(HIVEX_CFLAGS)
+	$(HIVEX_CFLAGS) \
+	$(YAJL_CFLAGS)
 
 # Manual pages and HTML files for the website.
 man_MANS = guestfsd.8

daemon/ldm.c

@@ -0,0 +1,467 @@
+/* libguestfs - the guestfsd daemon
+ * Copyright (C) 2012 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <glob.h>
+
+#if HAVE_YAJL
+#include <yajl/yajl_tree.h>
+#endif
+
+#include "daemon.h"
+#include "actions.h"
+#include "optgroups.h"
+
+#if HAVE_YAJL
+
+GUESTFSD_EXT_CMD(str_ldmtool, ldmtool);
+
+int
+optgroup_ldm_available (void)
+{
+  return prog_exists (str_ldmtool);
+}
+
+static int
+glob_errfunc (const char *epath, int eerrno)
+{
+  fprintf (stderr, "glob: failure reading %s: %s\n", epath, strerror (eerrno));
+  return 1;
+}
+
+static char **
+get_devices (const char *pattern)
+{
+  DECLARE_STRINGSBUF (ret);
+  glob_t devs;
+  int err;
+  size_t i;
+
+  err = glob (pattern, GLOB_ERR, glob_errfunc, &devs);
+  if (err == GLOB_NOSPACE) {
+    reply_with_error ("glob: returned GLOB_NOSPACE: "
+                      "rerun with LIBGUESTFS_DEBUG=1");
+    goto error;
+  } else if (err == GLOB_ABORTED) {
+    reply_with_error ("glob: returned GLOB_ABORTED: "
+                      "rerun with LIBGUESTFS_DEBUG=1");
+    goto error;
+  }
+
+  for (i = 0; i < devs.gl_pathc; ++i) {
+    if (add_string (&ret, devs.gl_pathv[i]) == -1)
+      goto error;
+  }
+
+  if (end_stringsbuf (&ret) == -1) goto error;
+
+  globfree (&devs);
+  return ret.argv;
+
+ error:
+  globfree (&devs);
+  if (ret.argv != NULL)
+    free_stringslen (ret.argv, ret.size);
+
+  return NULL;
+}
+
+/* All device mapper devices called /dev/mapper/ldm_vol_*.  XXX We
+ * could tighten this up in future if ldmtool had a way to read these
+ * names back after they have been created.
+ */
+char **
+do_list_ldm_volumes (void)
+{
+  return get_devices ("/dev/mapper/ldm_vol_*");
+}
+
+/* Same as above but /dev/mapper/ldm_part_*.  See comment above. */
+char **
+do_list_ldm_partitions (void)
+{
+  return get_devices ("/dev/mapper/ldm_part_*");
+}
+
+int
+do_ldmtool_create_all (void)
+{
+  int r;
+  char *err;
+
+  r = command (NULL, &err, "ldmtool", "create", "all", NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (err);
+    return -1;
+  }
+  free (err);
+  return 0;
+}
+
+int
+do_ldmtool_remove_all (void)
+{
+  int r;
+  char *err;
+
+  r = command (NULL, &err, "ldmtool", "remove", "all", NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (err);
+    return -1;
+  }
+  free (err);
+  return 0;
+}
+
+static yajl_val
+parse_json (const char *json, const char *func)
+{
+  yajl_val tree;
+  char parse_error[1024];
+
+  if (verbose)
+    fprintf (stderr, "%s: parsing json: %s\n", func, json);
+
+  tree = yajl_tree_parse (json, parse_error, sizeof parse_error);
+  if (tree == NULL) {
+    reply_with_error ("parse error: %s",
+                      strlen (parse_error) ? parse_error : "unknown error");
+    return NULL;
+  }
+
+  /* Caller should free this by doing 'yajl_tree_free (tree);'. */
+  return tree;
+}
+
+#define TYPE_ERROR ((char **) -1)
+
+static char **
+json_value_to_string_list (yajl_val node)
+{
+  DECLARE_STRINGSBUF (strs);
+  yajl_val n;
+  size_t i, len;
+
+  if (! YAJL_IS_ARRAY (node))
+    return TYPE_ERROR;
+
+  len = YAJL_GET_ARRAY(node)->len;
+  for (i = 0; i < len; ++i) {
+    n = YAJL_GET_ARRAY(node)->values[i];
+    if (! YAJL_IS_STRING (n))
+      return TYPE_ERROR;
+    if (add_string (&strs, YAJL_GET_STRING (n)) == -1)
+      return NULL;
+  }
+  if (end_stringsbuf (&strs) == -1)
+    return NULL;
+
+  return strs.argv;
+}
+
+static char **
+parse_json_get_string_list (const char *json,
+                            const char *func, const char *cmd)
+{
+  char **ret;
+  yajl_val tree = NULL;
+
+  tree = parse_json (json, func);
+  if (tree == NULL)
+    return NULL;
+
+  ret = json_value_to_string_list (tree);
+  yajl_tree_free (tree);
+  if (ret == TYPE_ERROR) {
+    reply_with_error ("output of '%s' was not a JSON array of strings", cmd);
+    return NULL;
+  }
+  return ret;
+}
+
+#define GET_STRING_NULL_TO_EMPTY 1
+
+static char *
+parse_json_get_object_string (const char *json, const char *key, int flags,
+                              const char *func, const char *cmd)
+{
+  char *str, *ret;
+  yajl_val tree = NULL, node;
+  size_t i, len;
+
+  tree = parse_json (json, func);
+  if (tree == NULL)
+    return NULL;
+
+  if (! YAJL_IS_OBJECT (tree))
+    goto bad_type;
+
+  len = YAJL_GET_OBJECT(tree)->len;
+  for (i = 0; i < len; ++i) {
+    if (STREQ (YAJL_GET_OBJECT(tree)->keys[i], key)) {
+      node = YAJL_GET_OBJECT(tree)->values[i];
+
+      if ((flags & GET_STRING_NULL_TO_EMPTY) && YAJL_IS_NULL (node))
+        ret = strdup ("");
+      else {
+        str = YAJL_GET_STRING (node);
+        if (str == NULL)
+          goto bad_type;
+        ret = strdup (str);
+      }
+      if (ret == NULL)
+        reply_with_perror ("strdup");
+
+      yajl_tree_free (tree);
+
+      return ret;
+    }
+  }
+
+ bad_type:
+  reply_with_error ("output of '%s' was not a JSON object "
+                    "containing a key '%s' of type string", cmd, key);
+  yajl_tree_free (tree);
+  return NULL;
+}
+
+static char **
+parse_json_get_object_string_list (const char *json, const char *key,
+                                   const char *func, const char *cmd)
+{
+  char **ret;
+  yajl_val tree, node;
+  size_t i, len;
+
+  tree = parse_json (json, func);
+  if (tree == NULL)
+    return NULL;
+
+  if (! YAJL_IS_OBJECT (tree))
+    goto bad_type;
+
+  len = YAJL_GET_OBJECT(tree)->len;
+  for (i = 0; i < len; ++i) {
+    if (STREQ (YAJL_GET_OBJECT(tree)->keys[i], key)) {
+      node = YAJL_GET_OBJECT(tree)->values[i];
+      ret = json_value_to_string_list (node);
+      if (ret == TYPE_ERROR)
+        goto bad_type;
+      yajl_tree_free (tree);
+      return ret;
+    }
+  }
+
+ bad_type:
+  reply_with_error ("output of '%s' was not a JSON object "
+                    "containing a key '%s' of type array of strings",
+                    cmd, key);
+  yajl_tree_free (tree);
+  return NULL;
+}
+
+char **
+do_ldmtool_scan (void)
+{
+  const char *empty_list[] = { NULL };
+
+  return do_ldmtool_scan_devices ((char * const *) empty_list);
+}
+
+char **
+do_ldmtool_scan_devices (char * const * devices)
+{
+  char **ret;
+  size_t i, nr_devices;
+  const char **argv;
+  int r;
+  char *out, *err;
+
+  nr_devices = count_strings (devices);
+  argv = malloc ((3 + nr_devices) * sizeof (char *));
+  if (argv == NULL) {
+    reply_with_perror ("malloc");
+    return NULL;
+  }
+
+  argv[0] = str_ldmtool;
+  argv[1] = "scan";
+  for (i = 0; i < nr_devices; ++i)
+    argv[2+i] = devices[i];
+  argv[2+i] = NULL;
+
+  r = commandv (&out, &err, argv);
+  free (argv);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (out);
+    free (err);
+    return NULL;
+  }
+  free (err);
+
+  ret = parse_json_get_string_list (out, __func__, "ldmtool scan");
+  free (out);
+  return ret;
+}
+
+char *
+do_ldmtool_diskgroup_name (const char *diskgroup)
+{
+  char *ret;
+  int r;
+  char *out, *err;
+
+  r = command (&out, &err, str_ldmtool, "show", "diskgroup", diskgroup, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (out);
+    free (err);
+    return NULL;
+  }
+  free (err);
+
+  ret = parse_json_get_object_string (out, "name", 0,
+                                      __func__, "ldmtool show diskgroup");
+  free (out);
+  return ret;
+}
+
+char **
+do_ldmtool_diskgroup_volumes (const char *diskgroup)
+{
+  char **ret;
+  int r;
+  char *out, *err;
+
+  r = command (&out, &err, str_ldmtool, "show", "diskgroup", diskgroup, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (out);
+    free (err);
+    return NULL;
+  }
+  free (err);
+
+  ret = parse_json_get_object_string_list (out, "volumes",
+                                           __func__, "ldmtool show diskgroup");
+  free (out);
+  return ret;
+}
+
+char **
+do_ldmtool_diskgroup_disks (const char *diskgroup)
+{
+  char **ret;
+  int r;
+  char *out, *err;
+
+  r = command (&out, &err, str_ldmtool, "show", "diskgroup", diskgroup, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (out);
+    free (err);
+    return NULL;
+  }
+  free (err);
+
+  ret = parse_json_get_object_string_list (out, "disks",
+                                           __func__, "ldmtool show diskgroup");
+  free (out);
+  return ret;
+}
+
+char *
+do_ldmtool_volume_type (const char *diskgroup, const char *volume)
+{
+  char *ret;
+  int r;
+  char *out, *err;
+
+  r = command (&out, &err,
+               str_ldmtool, "show", "volume", diskgroup, volume, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (out);
+    free (err);
+    return NULL;
+  }
+  free (err);
+
+  ret = parse_json_get_object_string (out, "type", 0,
+                                      __func__, "ldmtool show volume");
+  free (out);
+  return ret;
+}
+
+char *
+do_ldmtool_volume_hint (const char *diskgroup, const char *volume)
+{
+  char *ret;
+  int r;
+  char *out, *err;
+
+  r = command (&out, &err,
+               str_ldmtool, "show", "volume", diskgroup, volume, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (out);
+    free (err);
+    return NULL;
+  }
+  free (err);
+
+  ret = parse_json_get_object_string (out, "hint", GET_STRING_NULL_TO_EMPTY,
+                                      __func__, "ldmtool show volume");
+  free (out);
+  return ret;
+}
+
+char **
+do_ldmtool_volume_partitions (const char *diskgroup, const char *volume)
+{
+  char **ret;
+  int r;
+  char *out, *err;
+
+  r = command (&out, &err,
+               str_ldmtool, "show", "volume", diskgroup, volume, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    free (out);
+    free (err);
+    return NULL;
+  }
+  free (err);
+
+  ret = parse_json_get_object_string_list (out, "partitions",
+                                           __func__, "ldmtool show volume");
+  free (out);
+  return ret;
+}
+
+#else /* !HAVE_YAJL */
+
+OPTGROUP_LDM_NOT_AVAILABLE
+
+#endif

generator/actions.ml

@@ -10526,6 +10526,167 @@ This function sets the Linux capabilities attached to C<path>.
 The capabilities set C<cap> should be passed in text form
 (see L<cap_from_text(3)>)." };
 
+  { defaults with
+    name = "list_ldm_volumes";
+    style = RStringList "devices", [], [];
+    proc_nr = Some 380;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "list all Windows dynamic disk volumes";
+    longdesc = "\
+This function returns all Windows dynamic disk volumes
+that were found at launch time.  It returns a list of
+device names." };
+
+  { defaults with
+    name = "list_ldm_partitions";
+    style = RStringList "devices", [], [];
+    proc_nr = Some 381;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "list all Windows dynamic disk partitions";
+    longdesc = "\
+This function returns all Windows dynamic disk partitions
+that were found at launch time.  It returns a list of
+device names." };
+
+  { defaults with
+    name = "ldmtool_create_all";
+    style = RErr, [], [];
+    proc_nr = Some 382;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "scan and create Windows dynamic disk volumes";
+    longdesc = "\
+This function scans all block devices looking for Windows
+dynamic disk volumes and partitions, and creates devices
+for any that were found.
+
+Call C<guestfs_list_ldm_volumes> and C<guestfs_list_ldm_partitions>
+to return all devices.
+
+Note that you B<don't> normally need to call this explicitly,
+since it is done automatically at C<guestfs_launch> time.
+However you might want to call this function if you have
+hotplugged disks or have just created a Windows dynamic disk." };
+
+  { defaults with
+    name = "ldmtool_remove_all";
+    style = RErr, [], [];
+    proc_nr = Some 383;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "remove all Windows dynamic disk volumes";
+    longdesc = "\
+This is essentially the opposite of C<guestfs_ldmtool_create_all>.
+It removes the device mapper mappings for all Windows dynamic disk
+volumes" };
+
+  { defaults with
+    name = "ldmtool_scan";
+    style = RStringList "guids", [], [];
+    proc_nr = Some 384;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "scan for Windows dynamic disks";
+    longdesc = "\
+This function scans for Windows dynamic disks.  It returns a list
+of identifiers (GUIDs) for all disk groups that were found.  These
+identifiers can be passed to other C<guestfs_ldmtool_*> functions.
+
+This function scans all block devices.  To scan a subset of
+block devices, call C<guestfs_ldmtool_scan_devices> instead." };
+
+  { defaults with
+    name = "ldmtool_scan_devices";
+    style = RStringList "guids", [DeviceList "devices"], [];
+    proc_nr = Some 385;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "scan for Windows dynamic disks";
+    longdesc = "\
+This function scans for Windows dynamic disks.  It returns a list
+of identifiers (GUIDs) for all disk groups that were found.  These
+identifiers can be passed to other C<guestfs_ldmtool_*> functions.
+
+The parameter C<devices> is a list of block devices which are
+scanned.  If this list is empty, all block devices are scanned." };
+
+  { defaults with
+    name = "ldmtool_diskgroup_name";
+    style = RString "name", [String "diskgroup"], [];
+    proc_nr = Some 386;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "return the name of a Windows dynamic disk group";
+    longdesc = "\
+Return the name of a Windows dynamic disk group.  The C<diskgroup>
+parameter should be the GUID of a disk group, one element from
+the list returned by C<guestfs_ldmtool_scan>." };
+
+  { defaults with
+    name = "ldmtool_diskgroup_volumes";
+    style = RStringList "volumes", [String "diskgroup"], [];
+    proc_nr = Some 387;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "return the volumes in a Windows dynamic disk group";
+    longdesc = "\
+Return the volumes in a Windows dynamic disk group.  The C<diskgroup>
+parameter should be the GUID of a disk group, one element from
+the list returned by C<guestfs_ldmtool_scan>." };
+
+  { defaults with
+    name = "ldmtool_diskgroup_disks";
+    style = RStringList "disks", [String "diskgroup"], [];
+    proc_nr = Some 388;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "return the disks in a Windows dynamic disk group";
+    longdesc = "\
+Return the disks in a Windows dynamic disk group.  The C<diskgroup>
+parameter should be the GUID of a disk group, one element from
+the list returned by C<guestfs_ldmtool_scan>." };
+
+  { defaults with
+    name = "ldmtool_volume_type";
+    style = RString "voltype", [String "diskgroup"; String "volume"], [];
+    proc_nr = Some 389;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "return the type of a Windows dynamic disk volume";
+    longdesc = "\
+Return the type of the volume named C<volume> in the disk
+group with GUID <diskgroup>.
+
+Possible volume types that can be returned here include:
+C<simple>, C<spanned>, C<striped>, C<mirrored>, C<raid5>.
+Other types may also be returned." };
+
+  { defaults with
+    name = "ldmtool_volume_hint";
+    style = RString "hint", [String "diskgroup"; String "volume"], [];
+    proc_nr = Some 390;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "return the hint field of a Windows dynamic disk volume";
+    longdesc = "\
+Return the hint field of the volume named C<volume> in the disk
+group with GUID <diskgroup>.  This may not be defined, in which
+case the empty string is returned.  The hint field is often, though
+not always, the name of a Windows drive, eg. C<E:>." };
+
+  { defaults with
+    name = "ldmtool_volume_partitions";
+    style = RStringList "partitions", [String "diskgroup"; String "volume"], [];
+    proc_nr = Some 391;
+    optional = Some "ldm";
+    tests = [];
+    shortdesc = "return the partitions in a Windows dynamic disk volume";
+    longdesc = "\
+Return the list of partitions in the volume named C<volume> in the disk
+group with GUID <diskgroup>." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.

po/POTFILES

@@ -52,6 +52,7 @@ daemon/internal.c
 daemon/is.c
 daemon/isoinfo.c
 daemon/labels.c
+daemon/ldm.c
 daemon/link.c
 daemon/ls.c
 daemon/luks.c

src/MAX_PROC_NR

@@ -1 +1 @@
-379
+391

src/listfs.c

@@ -52,6 +52,8 @@ guestfs__list_filesystems (guestfs_h *g)
   char **partitions = NULL;
   char **mds = NULL;
   char **lvs = NULL;
+  char **ldmvols = NULL;
+  char **ldmparts = NULL;
 
   /* Look to see if any devices directly contain filesystems
    * (RHBZ#590167).  However vfs-type will fail to tell us anything
@@ -94,17 +96,36 @@ guestfs__list_filesystems (guestfs_h *g)
       check_with_vfs_type (g, lvs[i], &ret, &ret_size);
   }
 
+  if (guestfs___feature_available (g, "ldm")) {
+    /* Use vfs-type to check for filesystems on Windows dynamic disks. */
+    ldmvols = guestfs_list_ldm_volumes (g);
+    if (ldmvols == NULL) goto error;
+
+    for (i = 0; ldmvols[i] != NULL; ++i)
+      check_with_vfs_type (g, ldmvols[i], &ret, &ret_size);
+
+    ldmparts = guestfs_list_ldm_partitions (g);
+    if (ldmparts == NULL) goto error;
+
+    for (i = 0; ldmparts[i] != NULL; ++i)
+      check_with_vfs_type (g, ldmparts[i], &ret, &ret_size);
+  }
+
   guestfs___free_string_list (devices);
   guestfs___free_string_list (partitions);
   guestfs___free_string_list (mds);
   if (lvs) guestfs___free_string_list (lvs);
+  if (ldmvols) guestfs___free_string_list (ldmvols);
+  if (ldmparts) guestfs___free_string_list (ldmparts);
   return ret;
 
  error:
   if (devices) guestfs___free_string_list (devices);
   if (partitions) guestfs___free_string_list (partitions);
   if (mds) guestfs___free_string_list (mds);
-  //if (lvs) guestfs___free_string_list (lvs);
+  if (lvs) guestfs___free_string_list (lvs);
+  if (ldmvols) guestfs___free_string_list (ldmvols);
+  if (ldmparts) guestfs___free_string_list (ldmparts);
   if (ret) guestfs___free_string_list (ret);
   return NULL;
 }