4:48 a.m.
From: "Richard W.M. Jones" <rjones(a)redhat.com>
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 | 1 +
appliance/99-guestfs-serial.rules | 17 ++++++++
appliance/Makefile.am | 22 +++++++++-
configure.ac | 1 +
daemon/devsparts.c | 76 +++++++++++++++++++++++++++++++++
generator/actions.ml | 28 +++++++++++-
src/MAX_PROC_NR | 2 +-
src/guestfs-internal.h | 1 +
src/guestfs.pod | 20 +++++++++
src/launch-appliance.c | 6 ++-
src/launch-libvirt.c | 6 +++
src/launch.c | 40 +++++++++++++++--
tests/disk-labels/Makefile.am | 26 +++++++++++
tests/disk-labels/test-disk-labels.pl | 72 +++++++++++++++++++++++++++++++
14 files changed, 310 insertions(+), 8 deletions(-)
create mode 100644 appliance/99-guestfs-serial.rules
create mode 100644 tests/disk-labels/Makefile.am
create mode 100755 tests/disk-labels/test-disk-labels.pl
diff --git a/Makefile.am b/Makefile.am
index 7a0a091..4e476ea 100644
--- a/Makefile.am
+++ b/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
diff --git a/appliance/99-guestfs-serial.rules b/appliance/99-guestfs-serial.rules
new file mode 100644
index 0000000..2438958
--- /dev/null
+++ b/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"
diff --git a/appliance/Makefile.am b/appliance/Makefile.am
index 6d8b74a..8481534 100644
--- a/appliance/Makefile.am
+++ b/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
diff --git a/configure.ac b/configure.ac
index 7e580b3..0422bcb 100644
--- a/configure.ac
+++ b/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
diff --git a/daemon/devsparts.c b/daemon/devsparts.c
index 8f6c507..7e319cb 100644
--- a/daemon/devsparts.c
+++ b/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;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 713c716..f22bca9 100644
--- a/generator/actions.ml
+++ b/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.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index cb35cf9..446dfcc 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-368
+369
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index afc3be4..16b493c 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -145,6 +145,7 @@ struct drive {
char *format;
char *iface;
char *name;
+ char *disk_label;
bool use_cache_none;
void *priv; /* Data used by attach method. */
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 2b33bf3..48d810b 100644
--- a/src/guestfs.pod
+++ b/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)
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index e353e05..46090fc 100644
--- a/src/launch-appliance.c
+++ b/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);
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index e263f16..b757619 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -921,6 +921,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",
diff --git a/src/launch.c b/src/launch.c
index f0cda5e..3d27230 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -69,6 +69,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));
@@ -79,6 +80,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;
@@ -105,6 +107,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);
@@ -169,6 +172,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
@@ -179,7 +203,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;
@@ -213,7 +237,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);
@@ -234,6 +258,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;
@@ -251,6 +276,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"),
@@ -262,9 +289,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
@@ -281,7 +312,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;
}
diff --git a/tests/disk-labels/Makefile.am b/tests/disk-labels/Makefile.am
new file mode 100644
index 0000000..cd8f0e7
--- /dev/null
+++ b/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)
diff --git a/tests/disk-labels/test-disk-labels.pl
b/tests/disk-labels/test-disk-labels.pl
new file mode 100755
index 0000000..137adac
--- /dev/null
+++ b/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
--
1.7.10.4