>From 98d956d57c3d9df12a39c6abe5e639ee3b54f8a0 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" 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/. Partitions are named /dev/disk/guestfs/. 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/ +# +# 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. This is used as a hint to the guest inspection process if it is available. +=item C + +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), +the drive will also be named C>. + +See L. + =back C can have the special value C, which means @@ -9928,6 +9937,23 @@ on C. The optional C is the size of the filesystem in blocks. If omitted it defaults to the size of C." (* 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 parameter +of C, you can use this call to +map between serial names, and raw block device and partition +names (like C and C). + +This returns a hashtable, where keys are the serial names +(I the C prefix), and the values +are the full raw block device and partition names +(eg. C and C)." }; + ] (* 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, L and similar calls return the true names of the devices and partitions as known to the appliance, but see L. +=head3 SERIAL NAMES + +In libguestfs E 1.20, you can give a name to a disk when you add +it, using the optional C parameter to L. + +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, or using C>. +Partitions on the disk can be addressed using +CI>. + +Listing devices (L) and partitions +(L) returns the raw block device name. +However you can use L 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