[Libguestfs] [PATCH for discussion only] launch: Add add_drive 'serial' option.
Richard W.M. Jones
rjones at redhat.com
Thu Oct 4 14:43:21 UTC 2012
These two patches are for discussion of the proposed API only (in
fact, it doesn't compile).
virtio-scsi allows you to name drives (using the qemu serial=...
option or libvirt <serial/>). Let's allow users to specify a serial
when adding a drive, and then map that to a well-known name in the
libguestfs API (/dev/disk/guests/SERIAL[PARTNUM]).
Note that I chose not to change list-devices / list-partitions. These
still return raw device names (which still exist, even if the serial
is used). Instead there is a new API called list-serial-names which
returns a mapping of the serial names to raw device names.
This patch is in preparation for hotplugging. The plan would be to
remove the restriction that add_drive{_opts} has to be called before
launch. If it's called after launch (ie. hotplugging) then we
strongly recommend that users specify the serial option so the disk
has a predictable name.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
-------------- next part --------------
>From 6614d4f1d66921bf002067539b42c126d4bbfdb7 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Wed, 3 Oct 2012 11:46:07 +0100
Subject: [PATCH 1/2] build: Use 'tmp-d' as name of temporary directory
instead of 'tmp'.
When building supermin.d/daemon.img, use 'tmp-d' instead of 'tmp'
as the name of the temporary directory.
This is just code motion.
---
appliance/Makefile.am | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/appliance/Makefile.am b/appliance/Makefile.am
index fb1f676..6d8b74a 100644
--- a/appliance/Makefile.am
+++ b/appliance/Makefile.am
@@ -76,12 +76,12 @@ stamp-supermin: make.sh packagelist excludelist
supermin.d/daemon.img: ../daemon/guestfsd guestfsd.suppressions
mkdir -p supermin.d
rm -f $@ $@-t
- rm -rf tmp
- mkdir -p tmp$(DAEMON_SUPERMIN_DIR) tmp/etc
- ln ../daemon/guestfsd tmp$(DAEMON_SUPERMIN_DIR)/guestfsd
- ln $(srcdir)/guestfsd.suppressions tmp/etc/guestfsd.suppressions
- ( cd tmp && find | cpio --quiet -o -H newc ) > $@-t
- rm -rf tmp
+ rm -rf tmp-d
+ mkdir -p tmp-d$(DAEMON_SUPERMIN_DIR) tmp-d/etc
+ ln ../daemon/guestfsd tmp-d$(DAEMON_SUPERMIN_DIR)/guestfsd
+ ln $(srcdir)/guestfsd.suppressions tmp-d/etc/guestfsd.suppressions
+ ( cd tmp-d && find | cpio --quiet -o -H newc ) > $@-t
+ rm -rf tmp-d
mv $@-t $@
supermin.d/init.img: init
--
1.7.11.4
-------------- next part --------------
>From 98d956d57c3d9df12a39c6abe5e639ee3b54f8a0 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Wed, 3 Oct 2012 10:50:51 +0100
Subject: [PATCH 2/2] launch: Add add_drive 'serial' option. New API:
list-serial-names.
Allow the user to pass an optional serial name when adding a drive.
This is passed through to qemu / libvirt, and from there to the
appliance which exposes it through udev, creating a special alias of
the device /dev/disk/guestfs/<serial>. Partitions are named
/dev/disk/guestfs/<serial><partnum>.
virtio-blk and virtio-scsi limit the serial name to 20 bytes. We
further limit the name to maximum 20 alphabetic characters.
list-devices and list-partitions are not changed: these calls still
return raw block device names. However a new call, list-serial-names,
returns a hash table allowing callers to map between serial names, and
block device and partition names.
---
appliance/99-guestfs-serial.rules | 8 ++++++++
appliance/Makefile.am | 22 ++++++++++++++++++++-
daemon/devsparts.c | 6 ++++++
generator/actions.ml | 28 ++++++++++++++++++++++++++-
src/MAX_PROC_NR | 2 +-
src/guestfs-internal.h | 1 +
src/guestfs.pod | 18 ++++++++++++++++++
src/launch-appliance.c | 6 +++++-
src/launch-libvirt.c | 6 ++++++
src/launch.c | 40 ++++++++++++++++++++++++++++++++++-----
10 files changed, 128 insertions(+), 9 deletions(-)
create mode 100644 appliance/99-guestfs-serial.rules
diff --git a/appliance/99-guestfs-serial.rules b/appliance/99-guestfs-serial.rules
new file mode 100644
index 0000000..87784f3
--- /dev/null
+++ b/appliance/99-guestfs-serial.rules
@@ -0,0 +1,8 @@
+# For libguestfs, create /dev/disk/guestfs/<serial>
+#
+# As written, it's likely that this only works with virtio-scsi
+# because ID_SCSI_SERIAL is specific to the output of the 'scsi_id'
+# program.
+
+KERNEL=="sd*|vd*", ENV{DEVTYPE}=="disk", ENV{ID_SCSI_SERIAL}=="?*", \
+ SYMLINK+="disk/guestfs/$env{ID_SCSI_SERIAL}"
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/daemon/devsparts.c b/daemon/devsparts.c
index 8f6c507..74684ae 100644
--- a/daemon/devsparts.c
+++ b/daemon/devsparts.c
@@ -282,3 +282,9 @@ do_nr_devices (void)
return (int) i;
}
+
+char **
+do_list_serial_names (void)
+{
+ XXX partition names? XXX
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 4e13855..6595993 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 "serial"];
once_had_no_optargs = true;
fish_alias = ["add"]; config_only = true;
shortdesc = "add an image to examine or modify";
@@ -1237,6 +1237,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<serial>
+
+Give the disk a serial name. This name should be a unique, short
+string using only alphabetic ASCII characters.
+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<serial>>.
+
+See L<guestfs(3)/SERIAL NAMES>.
+
=back
C<filename> can have the special value C</dev/null>, which means
@@ -9928,6 +9937,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_serial_names";
+ style = RHashtable "names", [], [];
+ proc_nr = Some 369;
+ tests = [];
+ shortdesc = "mapping of serial names to devices";
+ longdesc = "\
+If you add drives using the optional C<serial> parameter
+of C</guestfs_add_drive_opts>, you can use this call to
+map between serial names, and raw block device and partition
+names (like C</dev/sda> and C</dev/sda1>).
+
+This returns a hashtable, where keys are the serial names
+(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 42bf05d..4b70a33 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -145,6 +145,7 @@ struct drive {
char *format;
char *iface;
char *name;
+ char *serial;
bool use_cache_none;
};
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 2b33bf3..8b3b915 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -1191,6 +1191,24 @@ 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 SERIAL NAMES
+
+In libguestfs E<ge> 1.20, you can give a name to a disk when you add
+it, using the optional C<serial> parameter to L</guestfs_add_drive_opts>.
+
+Not all versions of libguestfs support setting a serial name, and when
+it is supported, it is limited to 20 alphabetic ASCII characters.
+
+When you add a disk with a serial name, it can either be addressed
+using C</dev/sd*>, or using C</dev/disk/guestfs/I<serial>>.
+Partitions on the disk can be addressed using
+C</dev/disk/guestfs/I<serial>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_serial_names> to map serial names
+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..3fb6223 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->serial)
+ len += strlen (drv->serial);
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->serial ? ",serial=" : "",
+ drv->serial ? drv->serial : "",
index,
iface);
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index d678266..9b15d3e 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -890,6 +890,12 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr xo,
}
XMLERROR (-1, xmlTextWriterEndElement (xo));
+ if (drv->serial) {
+ XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "serial"));
+ XMLERROR (-1, xmlTextWriterWriteString (xo, BAD_CAST drv->serial));
+ 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 a0d6c12..06a79e1 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -110,10 +110,31 @@ valid_format_iface (const char *str)
return 1;
}
+/* Check the serial parameter 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_serial (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;
+}
+
static void
add_drive (guestfs_h *g, const char *path,
int readonly, const char *format,
- const char *iface, const char *name,
+ const char *iface, const char *name, const char *serial,
int use_cache_none)
{
struct drive **drv = &(g->drives);
@@ -128,6 +149,7 @@ add_drive (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)->serial = serial ? safe_strdup (g, serial) : NULL;
(*drv)->use_cache_none = use_cache_none;
}
@@ -141,7 +163,7 @@ add_drive (guestfs_h *g, const char *path,
*/
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 *serial)
{
char *tmpfile = NULL;
int fd = -1;
@@ -169,7 +191,7 @@ add_null_drive (guestfs_h *g, int readonly, const char *format,
goto err;
}
- add_drive (g, tmpfile, readonly, format, iface, name, 0);
+ add_drive (g, tmpfile, readonly, format, iface, name, serial, 0);
free (tmpfile);
return 0;
@@ -189,6 +211,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
const char *format;
const char *iface;
const char *name;
+ const char *serial;
int use_cache_none;
if (strchr (filename, ':') != NULL) {
@@ -205,6 +228,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;
+ serial = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_SERIAL_BITMASK
+ ? optargs->serial : NULL;
if (format && !valid_format_iface (format)) {
error (g, _("%s parameter is empty or contains disallowed characters"),
@@ -216,9 +241,13 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
"iface");
return -1;
}
+ if (serial && !valid_serial (serial)) {
+ error (g, _("serial 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, serial);
/* For writable files, see if we can use cache=none. This also
* checks for the existence of the file. For readonly we have
@@ -235,7 +264,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
}
}
- add_drive (g, filename, readonly, format, iface, name, use_cache_none);
+ add_drive (g, filename, readonly, format, iface, name, serial,
+ use_cache_none);
return 0;
}
--
1.7.11.4
More information about the Libguestfs
mailing list