launch: Add add_drive 'label' option.

New API: list-disk-labels

Allow the user to pass an optional disk label when adding a drive.

This is passed through to qemu / libvirt using the disk serial field,
and from there to the appliance which exposes it through udev,
creating a special alias of the device /dev/disk/guestfs/<label>.
Partitions are named /dev/disk/guestfs/<label><partnum>.

virtio-blk and virtio-scsi limit the serial field to 20 bytes.  We
further limit the name to maximum 20 ASCII characters in [a-zA-Z].

list-devices and list-partitions are not changed: these calls still
return raw block device names.  However a new call, list-disk-labels,
returns a hash table allowing callers to map between disk labels, and
block device and partition names.

This commit also includes a test.

Makefile.am

@@ -51,6 +51,7 @@ SUBDIRS += tests/mount-local
 SUBDIRS += tests/9p
 SUBDIRS += tests/rsync
 SUBDIRS += tests/bigdirs
+SUBDIRS += tests/disk-labels
 SUBDIRS += tests/regressions
 endif
 

appliance/99-guestfs-serial.rules

@@ -0,0 +1,17 @@
+# For libguestfs, create /dev/disk/guestfs/<serial>
+# and /dev/disk/guestfs/<serial><partnum>
+
+KERNEL=="sd*[!0-9]", ENV{DEVTYPE}=="disk", ENV{ID_SCSI_SERIAL}=="?*", \
+  SYMLINK+="disk/guestfs/$env{ID_SCSI_SERIAL}"
+KERNEL=="sd*", ENV{DEVTYPE}=="partition", ENV{ID_SCSI_SERIAL}=="?*", \
+  SYMLINK+="disk/guestfs/$env{ID_SCSI_SERIAL}%n"
+
+# As written, it's likely the above only works with virtio-scsi
+# because ID_SCSI_SERIAL is specific to the output of the 'scsi_id'
+# program.  The following will not work because ID_SERIAL contains
+# some unwanted text.
+
+#KERNEL=="vd*[!0-9]", ATTRS{serial}=="?*", ENV{ID_SERIAL}="$attr{serial}", \
+#  SYMLINK+="disk/guestfs/$env{ID_SERIAL}"
+#KERNEL=="vd*[0-9]", ATTRS{serial}=="?*", ENV{ID_SERIAL}="$attr{serial}", \
+#  SYMLINK+="disk/guestfs/$env{ID_SERIAL}%n"

appliance/Makefile.am

@@ -37,7 +37,8 @@ superminfs_DATA = \
 	supermin.d/base.img \
 	supermin.d/daemon.img \
 	supermin.d/init.img \
-	supermin.d/hostfiles
+	supermin.d/hostfiles \
+	supermin.d/udev-rules.img
 
 # This used to be a configure-generated file (as is update.sh still).
 # However config.status always touches the destination file, which
@@ -91,6 +92,25 @@ supermin.d/init.img: init
 	echo "init" | cpio --quiet -o -H newc > $@-t
 	mv $@-t $@
 
+# We should put this file in /lib/udev/rules.d, but put it in /etc so
+# we don't have to deal with all the UsrMove crap in Fedora.
+supermin.d/udev-rules.img: 99-guestfs-serial.rules
+	mkdir -p supermin.d
+	rm -f $@ $@-t
+	rm -rf tmp-u
+	mkdir -p tmp-u/etc/udev/rules.d
+	for f in $^; do ln $$f tmp-u/etc/udev/rules.d/$$f; done
+	( cd tmp-u && find | cpio --quiet -o -H newc ) > $@-t
+	rm -rf tmp-u
+	mv $@-t $@
+
+# If installing the daemon, install the udev rules too.
+
+if INSTALL_DAEMON
+udevrulesdir = /lib/udev/rules.d
+udevrules_DATA = 99-guestfs-serial.rules
+endif
+
 # libguestfs-make-fixed-appliance script and man page.
 
 sbin_SCRIPTS = libguestfs-make-fixed-appliance

configure.ac

@@ -1376,6 +1376,7 @@ AC_CONFIG_FILES([Makefile
                  tests/charsets/Makefile
                  tests/data/Makefile
                  tests/disks/Makefile
+                 tests/disk-labels/Makefile
                  tests/extra/Makefile
                  tests/guests/Makefile
                  tests/luks/Makefile

daemon/devsparts.c

@@ -24,6 +24,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <dirent.h>
+#include <limits.h>
 #include <sys/stat.h>
 
 #include "c-ctype.h"
@@ -282,3 +283,78 @@ do_nr_devices (void)
 
   return (int) i;
 }
+
+#define GUESTFSDIR "/dev/disk/guestfs"
+
+char **
+do_list_disk_labels (void)
+{
+  DIR *dir = NULL;
+  struct dirent *d;
+  char *path = NULL, *rawdev = NULL;
+  DECLARE_STRINGSBUF (ret);
+
+  dir = opendir (GUESTFSDIR);
+  if (!dir) {
+    reply_with_perror ("opendir: %s", GUESTFSDIR);
+    return NULL;
+  }
+
+  errno = 0;
+  while ((d = readdir (dir)) != NULL) {
+    if (d->d_name[0] == '.')
+      continue;
+
+    if (asprintf (&path, "%s/%s", GUESTFSDIR, d->d_name) == -1) {
+      reply_with_perror ("asprintf");
+      free_stringslen (ret.argv, ret.size);
+      goto error;
+    }
+
+    rawdev = realpath (path, NULL);
+    if (rawdev == NULL) {
+      reply_with_perror ("realpath: %s", path);
+      free_stringslen (ret.argv, ret.size);
+      goto error;
+    }
+
+    free (path);
+    path = NULL;
+
+    if (add_string (&ret, d->d_name) == -1)
+      goto error;
+
+    if (add_string_nodup (&ret, rawdev) == -1)
+      goto error;
+    rawdev = NULL;            /* buffer now owned by the stringsbuf */
+  }
+
+  /* Check readdir didn't fail */
+  if (errno != 0) {
+    reply_with_perror ("readdir: %s", GUESTFSDIR);
+    free_stringslen (ret.argv, ret.size);
+    goto error;
+  }
+
+  /* Close the directory handle */
+  if (closedir (dir) == -1) {
+    reply_with_perror ("closedir: %s", GUESTFSDIR);
+    free_stringslen (ret.argv, ret.size);
+    dir = NULL;
+    goto error;
+  }
+
+  dir = NULL;
+
+  if (end_stringsbuf (&ret) == -1)
+    goto error;
+
+  return ret.argv;              /* caller frees */
+
+ error:
+  if (dir)
+    closedir (dir);
+  free (path);
+  free (rawdev);
+  return NULL;
+}

generator/actions.ml

@@ -1184,7 +1184,7 @@ not all belong to a single logical operating system
 
   { defaults with
     name = "add_drive";
-    style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"];
+    style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"];
     once_had_no_optargs = true;
     fish_alias = ["add"]; config_only = true;
     shortdesc = "add an image to examine or modify";
@@ -1238,6 +1238,15 @@ The name the drive had in the original guest, e.g. C</dev/sdb>.
 This is used as a hint to the guest inspection process if
 it is available.
 
+=item C<label>
+
+Give the disk a label.  The label should be a unique, short
+string using I<only> ASCII characters C<[a-zA-Z]>.
+As well as its usual name in the API (such as C</dev/sda>),
+the drive will also be named C</dev/disk/guestfs/I<label>>.
+
+See L<guestfs(3)/DISK LABELS>.
+
 =back" };
 
   { defaults with
@@ -9925,6 +9934,23 @@ on C<device>.  The optional C<blockscount> is the size of the
 filesystem in blocks.  If omitted it defaults to the size of
 C<device>." (* XXX document optional args properly *) };
 
+  { defaults with
+    name = "list_disk_labels";
+    style = RHashtable "labels", [], [];
+    proc_nr = Some 369;
+    tests = [];
+    shortdesc = "mapping of disk labels to devices";
+    longdesc = "\
+If you add drives using the optional C<label> parameter
+of C<guestfs_add_drive_opts>, you can use this call to
+map between disk labels, and raw block device and partition
+names (like C</dev/sda> and C</dev/sda1>).
+
+This returns a hashtable, where keys are the disk labels
+(I<without> the C</dev/disk/guestfs> prefix), and the values
+are the full raw block device and partition names
+(eg. C</dev/sda> and C</dev/sda1>)." };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.

src/MAX_PROC_NR

@@ -1 +1 @@
-368
+369

src/guestfs-internal.h

@@ -143,6 +143,7 @@ struct drive {
   char *format;
   char *iface;
   char *name;
+  char *disk_label;
   bool use_cache_none;
 
   void *priv;                   /* Data used by attach method. */

src/guestfs.pod

@@ -1191,6 +1191,26 @@ L</guestfs_list_devices>, L</guestfs_list_partitions> and similar calls
 return the true names of the devices and partitions as known to the
 appliance, but see L</guestfs_canonical_device_name>.
 
+=head3 DISK LABELS
+
+In libguestfs E<ge> 1.20, you can give a label to a disk when you add
+it, using the optional C<label> parameter to L</guestfs_add_drive_opts>.
+(Note that disk labels are different from and not related to
+filesystem labels).
+
+Not all versions of libguestfs support setting a disk label, and when
+it is supported, it is limited to 20 ASCII characters C<[a-zA-Z]>.
+
+When you add a disk with a label, it can either be addressed
+using C</dev/sd*>, or using C</dev/disk/guestfs/I<label>>.
+Partitions on the disk can be addressed using
+C</dev/disk/guestfs/I<label>I<partnum>>.
+
+Listing devices (L</guestfs_list_devices>) and partitions
+(L</guestfs_list_partitions>) returns the raw block device name.
+However you can use L</guestfs_list_disk_labels> to map disk labels
+to raw block device and partition names.
+
 =head3 ALGORITHM FOR BLOCK DEVICE NAME TRANSLATION
 
 Usually this translation is transparent.  However in some (very rare)

src/launch-appliance.c

@@ -908,6 +908,8 @@ qemu_drive_param (guestfs_h *g, const struct drive *drv, size_t index)
     len += strlen (drv->iface);
   if (drv->format)
     len += strlen (drv->format);
+  if (drv->disk_label)
+    len += strlen (drv->disk_label);
 
   r = safe_malloc (g, len);
 
@@ -930,11 +932,13 @@ qemu_drive_param (guestfs_h *g, const struct drive *drv, size_t index)
   else
     iface = "virtio";
 
-  snprintf (&r[i], len-i, "%s%s%s%s,id=hd%zu,if=%s",
+  snprintf (&r[i], len-i, "%s%s%s%s%s%s,id=hd%zu,if=%s",
             drv->readonly ? ",snapshot=on" : "",
             drv->use_cache_none ? ",cache=none" : "",
             drv->format ? ",format=" : "",
             drv->format ? drv->format : "",
+            drv->disk_label ? ",serial=" : "",
+            drv->disk_label ? drv->disk_label : "",
             index,
             iface);
 

src/launch-libvirt.c

@@ -913,6 +913,12 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr xo,
   }
   XMLERROR (-1, xmlTextWriterEndElement (xo));
 
+  if (drv->disk_label) {
+    XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "serial"));
+    XMLERROR (-1, xmlTextWriterWriteString (xo, BAD_CAST drv->disk_label));
+    XMLERROR (-1, xmlTextWriterEndElement (xo));
+  }
+
   XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "address"));
   XMLERROR (-1,
             xmlTextWriterWriteAttribute (xo, BAD_CAST "type",

src/launch.c

@@ -71,6 +71,7 @@ static struct drive *
 create_drive_struct (guestfs_h *g, const char *path,
                      int readonly, const char *format,
                      const char *iface, const char *name,
+                     const char *disk_label,
                      int use_cache_none)
 {
   struct drive *drv = safe_malloc (g, sizeof (struct drive));
@@ -80,6 +81,7 @@ create_drive_struct (guestfs_h *g, const char *path,
   drv->format = format ? safe_strdup (g, format) : NULL;
   drv->iface = iface ? safe_strdup (g, iface) : NULL;
   drv->name = name ? safe_strdup (g, name) : NULL;
+  drv->disk_label = disk_label ? safe_strdup (g, disk_label) : NULL;
   drv->use_cache_none = use_cache_none;
   drv->priv = drv->free_priv = NULL;
 
@@ -92,7 +94,7 @@ guestfs___add_dummy_appliance_drive (guestfs_h *g)
 {
   struct drive *drv;
 
-  drv = create_drive_struct (g, "", 0, NULL, NULL, NULL, 0);
+  drv = create_drive_struct (g, "", 0, NULL, NULL, NULL, NULL, 0);
   add_drive_to_handle (g, drv);
 }
 
@@ -119,6 +121,7 @@ free_drive_struct (struct drive *drv)
   free (drv->format);
   free (drv->iface);
   free (drv->name);
+  free (drv->disk_label);
   if (drv->priv && drv->free_priv)
     drv->free_priv (drv->priv);
   free (drv);
@@ -183,6 +186,27 @@ valid_format_iface (const char *str)
   return 1;
 }
 
+/* Check the disk label is reasonable.  It can't contain certain
+ * characters, eg. '/', ','.  However be stricter here and ensure it's
+ * just alphabetic and <= 20 characters in length.
+ */
+static int
+valid_disk_label (const char *str)
+{
+  size_t len = strlen (str);
+
+  if (len == 0 || len > 20)
+    return 0;
+
+  while (len > 0) {
+    char c = *str++;
+    len--;
+    if (!c_isalpha (c))
+      return 0;
+  }
+  return 1;
+}
+
 /* Traditionally you have been able to use /dev/null as a filename, as
  * many times as you like.  Ancient KVM (RHEL 5) cannot handle adding
  * /dev/null readonly.  qemu 1.2 + virtio-scsi segfaults when you use
@@ -193,7 +217,7 @@ valid_format_iface (const char *str)
  */
 static int
 add_null_drive (guestfs_h *g, int readonly, const char *format,
-                const char *iface, const char *name)
+                const char *iface, const char *name, const char *disk_label)
 {
   char *tmpfile = NULL;
   int fd = -1;
@@ -227,7 +251,7 @@ add_null_drive (guestfs_h *g, int readonly, const char *format,
     goto err;
   }
 
-  drv = create_drive_struct (g, tmpfile, readonly, format, iface, name, 0);
+  drv = create_drive_struct (g, tmpfile, readonly, format, iface, name, disk_label, 0);
   add_drive_to_handle (g, drv);
   free (tmpfile);
 
@@ -248,6 +272,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   const char *format;
   const char *iface;
   const char *name;
+  const char *disk_label;
   int use_cache_none;
   struct drive *drv;
 
@@ -265,6 +290,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
           ? optargs->iface : NULL;
   name = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_NAME_BITMASK
          ? optargs->name : NULL;
+  disk_label = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_LABEL_BITMASK
+         ? optargs->label : NULL;
 
   if (format && !valid_format_iface (format)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
@@ -276,9 +303,13 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
            "iface");
     return -1;
   }
+  if (disk_label && !valid_disk_label (disk_label)) {
+    error (g, _("label parameter is empty, too long, or contains disallowed characters"));
+    return -1;
+  }
 
   if (STREQ (filename, "/dev/null"))
-    return add_null_drive (g, readonly, format, iface, name);
+    return add_null_drive (g, readonly, format, iface, name, disk_label);
 
   /* For writable files, see if we can use cache=none.  This also
    * checks for the existence of the file.  For readonly we have
@@ -295,7 +326,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
     }
   }
 
-  drv = create_drive_struct (g, filename, readonly, format, iface, name, use_cache_none);
+  drv = create_drive_struct (g, filename, readonly, format, iface, name, disk_label,
+                             use_cache_none);
   add_drive_to_handle (g, drv);
   return 0;
 }

tests/disk-labels/Makefile.am

@@ -0,0 +1,26 @@
+# libguestfs
+# 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 $(top_srcdir)/subdir-rules.mk
+
+TESTS = \
+	test-disk-labels.pl
+
+TESTS_ENVIRONMENT = $(top_builddir)/run --test
+
+EXTRA_DIST = \
+	$(TESTS)

tests/disk-labels/test-disk-labels.pl

@@ -0,0 +1,72 @@
+#!/usr/bin/perl
+# 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.
+
+# Test using the 'label' option of add_drive, and the
+# list_disk_labels call.
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+
+my $g = Sys::Guestfs->new ();
+
+# Add two drives.
+foreach (["test1.img", "a"], ["test2.img", "b"]) {
+    my ($output, $label) = @$_;
+    open FILE, ">$output" or die "$output: $!";
+    truncate FILE, 512 * 1024 * 1024 or die "$output: truncate: $!";
+    close FILE or die "$output: $!";
+    $g->add_drive ($output, readonly => 0, format => "raw", label => $label);
+}
+
+$g->launch ();
+
+# Partition the drives.
+$g->part_disk ("/dev/disk/guestfs/a", "mbr");
+$g->part_init ("/dev/disk/guestfs/b", "mbr");
+$g->part_add ("/dev/disk/guestfs/b", "p", 64, 100 * 1024 * 2 - 1);
+$g->part_add ("/dev/disk/guestfs/b", "p", 100 * 1024 * 2, -64);
+
+# Check the partitions exist using both the disk label and raw name.
+die unless
+    $g->blockdev_getsize64 ("/dev/disk/guestfs/a1") ==
+    $g->blockdev_getsize64 ("/dev/sda1");
+die unless
+    $g->blockdev_getsize64 ("/dev/disk/guestfs/b1") ==
+    $g->blockdev_getsize64 ("/dev/sdb1");
+die unless
+    $g->blockdev_getsize64 ("/dev/disk/guestfs/b2") ==
+    $g->blockdev_getsize64 ("/dev/sdb2");
+
+# Check list_disk_labels
+my %labels = $g->list_disk_labels ();
+die unless exists $labels{"a"};
+die unless $labels{"a"} eq "/dev/sda";
+die unless exists $labels{"b"};
+die unless $labels{"b"} eq "/dev/sdb";
+die unless exists $labels{"a1"};
+die unless $labels{"a1"} eq "/dev/sda1";
+die unless exists $labels{"b1"};
+die unless $labels{"b1"} eq "/dev/sdb1";
+die unless exists $labels{"b2"};
+die unless $labels{"b2"} eq "/dev/sdb2";
+
+unlink "test1.img";
+unlink "test2.img";
+
+exit 0