Richard W.M. Jones
2012-Oct-08 19:05 UTC
[Libguestfs] [PATCH v4 0/5] Finish hotplugging support.
This rounds off hotplugging support by allowing you to add and remove drives at any stage (before and after launch). Rich.
Richard W.M. Jones
2012-Oct-08 19:05 UTC
[Libguestfs] [PATCH v4 1/5] launch: Make g->drives into an array (was a linked list).
From: "Richard W.M. Jones" <rjones at redhat.com>
Using an array simplifies the implementation of hotplugging.
---
src/guestfs-internal.h | 37 ++++++++++++++++-----
src/guestfs.c | 4 ++-
src/inspect-fs-unix.c | 20 ++++-------
src/launch-appliance.c | 20 +++++------
src/launch-libvirt.c | 33 +++++++------------
src/launch.c | 86 +++++++++++++++++++++++++++++-------------------
src/libvirtdomain.c | 2 +-
7 files changed, 113 insertions(+), 89 deletions(-)
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index afc3be4..652cf31 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -135,10 +135,8 @@ struct event {
void *opaque2;
};
-/* Linked list of drives added to the handle. */
+/* Drives added to the handle. */
struct drive {
- struct drive *next;
-
char *path;
int readonly;
@@ -194,10 +192,32 @@ struct guestfs_h
char *qemu; /* Qemu binary. */
char *append; /* Append to kernel command line. */
- struct drive *drives; /* Drives added by add-drive* APIs. */
-
struct qemu_param *qemu_params; /* Extra qemu parameters. */
+ /* Array of drives added by add-drive* APIs.
+ *
+ * Before launch this list can be empty or contain some drives.
+ *
+ * During launch, a dummy slot may be added which represents the
+ * slot taken up by the appliance drive.
+ *
+ * When hotplugging is supported by the attach method, drives can be
+ * added to the end of this list after launch. Also hot-removing a
+ * drive causes a NULL slot to appear in the list.
+ *
+ * During shutdown, this list is deleted, so that each launch gets a
+ * fresh set of drives (however callers: don't do this, create a new
+ * handle each time).
+ *
+ * Always use ITER_DRIVES macro to iterate over this list!
+ */
+ struct drive **drives;
+ size_t nr_drives;
+
+#define ITER_DRIVES(g,i,drv) \
+ for (i = 0; i < (g)->nr_drives; ++i) \
+ if (((drv) = (g)->drives[i]) != NULL)
+
/* Attach method, and associated backend operations. */
enum attach_method attach_method;
char *attach_method_arg;
@@ -433,7 +453,6 @@ extern void guestfs___debug (guestfs_h *g, const char *fs,
...)
extern void guestfs___trace (guestfs_h *g, const char *fs, ...)
__attribute__((format (printf,2,3)));
-extern void guestfs___free_drives (struct drive **drives);
extern void guestfs___free_string_list (char **);
extern void guestfs___print_BufferIn (FILE *out, const char *buf, size_t
buf_size);
@@ -489,9 +508,11 @@ extern void guestfs___remove_tmpdir (const char *dir);
extern int64_t guestfs___timeval_diff (const struct timeval *x, const struct
timeval *y);
extern void guestfs___print_timestamped_message (guestfs_h *g, const char *fs,
...);
extern void guestfs___launch_send_progress (guestfs_h *g, int perdozen);
-extern struct drive ** guestfs___checkpoint_drives (guestfs_h *g);
-extern void guestfs___rollback_drives (guestfs_h *g, struct drive **i);
+extern size_t guestfs___checkpoint_drives (guestfs_h *g);
+extern void guestfs___rollback_drives (guestfs_h *g, size_t);
extern void guestfs___launch_failed_error (guestfs_h *g);
+extern void guestfs___add_dummy_appliance_drive (guestfs_h *g);
+extern void guestfs___free_drives (guestfs_h *g);
/* launch-appliance.c */
extern char *guestfs___drive_name (size_t index, char *ret);
diff --git a/src/guestfs.c b/src/guestfs.c
index cb9ee6b..ee523eb 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -268,7 +268,7 @@ guestfs_close (guestfs_h *g)
#endif
guestfs___free_inspect_info (g);
- guestfs___free_drives (&g->drives);
+ guestfs___free_drives (g);
for (qp = g->qemu_params; qp; qp = qp_next) {
free (qp->qemu_param);
@@ -328,6 +328,8 @@ shutdown_backend (guestfs_h *g, int check_for_errors)
if (g->attach_ops->shutdown (g, check_for_errors) == -1)
ret = -1;
+ guestfs___free_drives (g);
+
g->state = CONFIG;
return ret;
diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index 2152484..33c5ee9 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -1285,7 +1285,7 @@ resolve_fstab_device_xdev (guestfs_h *g, const char *type,
const char *disk,
char *name, *device;
char **devices;
size_t i, count;
- struct drive *drive;
+ struct drive *drv;
const char *p;
/* type: (h|s|v|xv)
@@ -1299,10 +1299,8 @@ resolve_fstab_device_xdev (guestfs_h *g, const char
*type, const char *disk,
/* Check any hints we were passed for a non-heuristic mapping */
name = safe_asprintf (g, "%sd%s", type, disk);
- i = 0;
- drive = g->drives;
- while (drive) {
- if (drive->name && STREQ (drive->name, name)) {
+ ITER_DRIVES (g, i, drv) {
+ if (drv->name && STREQ (drv->name, name)) {
device = safe_asprintf (g, "%s%s", devices[i], part);
if (!is_partition (g, device)) {
free (device);
@@ -1311,8 +1309,6 @@ resolve_fstab_device_xdev (guestfs_h *g, const char *type,
const char *disk,
*device_ret = device;
break;
}
-
- i++; drive = drive->next;
}
free (name);
@@ -1354,7 +1350,7 @@ resolve_fstab_device_cciss (guestfs_h *g, const char
*disk, const char *part,
char *device;
char **devices;
size_t i;
- struct drive *drive;
+ struct drive *drv;
/* disk: (cciss/c\d+d\d+)
* part: (\d+)?
@@ -1365,10 +1361,8 @@ resolve_fstab_device_cciss (guestfs_h *g, const char
*disk, const char *part,
return -1;
/* Check any hints we were passed for a non-heuristic mapping */
- i = 0;
- drive = g->drives;
- while (drive) {
- if (drive->name && STREQ(drive->name, disk)) {
+ ITER_DRIVES (g, i, drv) {
+ if (drv->name && STREQ (drv->name, disk)) {
if (part) {
device = safe_asprintf (g, "%s%s", devices[i], part);
if (!is_partition (g, device)) {
@@ -1381,8 +1375,6 @@ resolve_fstab_device_cciss (guestfs_h *g, const char
*disk, const char *part,
*device_ret = safe_strdup (g, devices[i]);
break;
}
-
- i++; drive = drive->next;
}
/* We don't try to guess mappings for cciss devices */
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index e353e05..531faef 100644
--- a/src/launch-appliance.c
+++ b/src/launch-appliance.c
@@ -135,7 +135,7 @@ launch_appliance (guestfs_h *g, const char *arg)
/* At present you must add drives before starting the appliance. In
* future when we enable hotplugging you won't need to do this.
*/
- if (!g->drives) {
+ if (!g->nr_drives) {
error (g, _("you must call guestfs_add_drive before
guestfs_launch"));
return -1;
}
@@ -256,8 +256,8 @@ launch_appliance (guestfs_h *g, const char *arg)
add_cmdline (g, "-nographic");
/* Add drives */
- struct drive *drv = g->drives;
- size_t drv_index = 0;
+ struct drive *drv;
+ size_t i;
if (virtio_scsi) {
/* Create the virtio-scsi bus. */
@@ -265,9 +265,9 @@ launch_appliance (guestfs_h *g, const char *arg)
add_cmdline (g, "virtio-scsi-pci,id=scsi");
}
- while (drv != NULL) {
+ ITER_DRIVES (g, i, drv) {
/* Construct the final -drive parameter. */
- char *buf = qemu_drive_param (g, drv, drv_index);
+ char *buf = qemu_drive_param (g, drv, i);
add_cmdline (g, "-drive");
add_cmdline (g, buf);
@@ -275,13 +275,10 @@ launch_appliance (guestfs_h *g, const char *arg)
if (virtio_scsi && drv->iface == NULL) {
char buf2[64];
- snprintf (buf2, sizeof buf2, "scsi-hd,drive=hd%zu",
drv_index);
+ snprintf (buf2, sizeof buf2, "scsi-hd,drive=hd%zu", i);
add_cmdline (g, "-device");
add_cmdline (g, buf2);
}
-
- drv = drv->next;
- drv_index++;
}
char appliance_root[64] = "";
@@ -310,7 +307,7 @@ launch_appliance (guestfs_h *g, const char *arg)
snprintf (appliance_root, sizeof appliance_root,
"root=/dev/%cd",
virtio_scsi ? 's' : 'v');
- guestfs___drive_name (drv_index, &appliance_root[12]);
+ guestfs___drive_name (g->nr_drives, &appliance_root[12]);
}
if (STRNEQ (QEMU_OPTIONS, "")) {
@@ -667,6 +664,9 @@ launch_appliance (guestfs_h *g, const char *arg)
guestfs___launch_send_progress (g, 12);
+ if (appliance)
+ guestfs___add_dummy_appliance_drive (g);
+
return 0;
cleanup1:
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index d678266..7ae37e3 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -120,7 +120,7 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
/* At present you must add drives before starting the appliance. In
* future when we enable hotplugging you won't need to do this.
*/
- if (!g->drives) {
+ if (!g->nr_drives) {
error (g, _("you must call guestfs_add_drive before
guestfs_launch"));
return -1;
}
@@ -369,6 +369,8 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
goto cleanup;
}
+ guestfs___add_dummy_appliance_drive (g);
+
TRACE0 (launch_libvirt_end);
guestfs___launch_send_progress (g, 12);
@@ -453,18 +455,9 @@ construct_libvirt_xml (guestfs_h *g, const char
*capabilities_xml,
xmlBufferPtr xb = NULL;
xmlOutputBufferPtr ob;
xmlTextWriterPtr xo = NULL;
- struct drive *drv = g->drives;
- size_t appliance_index = 0;
+ size_t appliance_index = g->nr_drives;
const char *type;
- /* Count the number of disks added, in order to get the offset
- * of the appliance disk.
- */
- while (drv != NULL) {
- drv = drv->next;
- appliance_index++;
- }
-
/* Big hack, instead of actually parsing the capabilities XML (XXX). */
type = strstr (capabilities_xml, "'kvm'") != NULL ?
"kvm" : "qemu";
@@ -686,8 +679,8 @@ construct_libvirt_xml_devices (guestfs_h *g,
xmlTextWriterPtr xo,
const char *guestfsd_sock,
const char *console_sock)
{
- struct drive *drv = g->drives;
- size_t drv_index = 0;
+ struct drive *drv;
+ size_t i;
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "devices"));
@@ -714,11 +707,9 @@ construct_libvirt_xml_devices (guestfs_h *g,
xmlTextWriterPtr xo,
XMLERROR (-1, xmlTextWriterEndElement (xo));
/* Disks. */
- while (drv != NULL) {
- if (construct_libvirt_xml_disk (g, xo, drv, drv_index) == -1)
+ ITER_DRIVES (g, i, drv) {
+ if (construct_libvirt_xml_disk (g, xo, drv, i) == -1)
goto err;
- drv = drv->next;
- drv_index++;
}
/* Appliance disk. */
@@ -1014,7 +1005,7 @@ static int
construct_libvirt_xml_qemu_cmdline (guestfs_h *g, xmlTextWriterPtr xo)
{
struct drive *drv;
- size_t drv_index;
+ size_t i;
char attr[256];
struct qemu_param *qp;
char *p;
@@ -1025,10 +1016,10 @@ construct_libvirt_xml_qemu_cmdline (guestfs_h *g,
xmlTextWriterPtr xo)
* by Stefan Hajnoczi's post here:
*
http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html
*/
- for (drv = g->drives, drv_index = 0; drv; drv = drv->next, drv_index++)
{
+ ITER_DRIVES (g, i, drv) {
if (drv->readonly) {
snprintf (attr, sizeof attr,
- "drive.drive-scsi0-0-%zu-0.snapshot=on", drv_index);
+ "drive.drive-scsi0-0-%zu-0.snapshot=on", i);
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"qemu:arg"));
XMLERROR (-1,
@@ -1045,7 +1036,7 @@ construct_libvirt_xml_qemu_cmdline (guestfs_h *g,
xmlTextWriterPtr xo)
}
snprintf (attr, sizeof attr,
- "drive.drive-scsi0-0-%zu-0.snapshot=on", drv_index);
+ "drive.drive-scsi0-0-%zu-0.snapshot=on",
g->nr_drives);
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "qemu:arg"));
XMLERROR (-1,
diff --git a/src/launch.c b/src/launch.c
index f0cda5e..6e11111 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -39,30 +39,32 @@
static void free_drive_struct (struct drive *drv);
-struct drive **
+size_t
guestfs___checkpoint_drives (guestfs_h *g)
{
- struct drive **i = &g->drives;
- while (*i != NULL) i = &((*i)->next);
- return i;
+ return g->nr_drives;
}
void
-guestfs___rollback_drives (guestfs_h *g, struct drive **i)
+guestfs___rollback_drives (guestfs_h *g, size_t old_i)
{
- guestfs___free_drives(i);
+ size_t i;
+
+ for (i = old_i; i < g->nr_drives; ++i) {
+ if (g->drives[i])
+ free_drive_struct (g->drives[i]);
+ }
+ g->nr_drives = old_i;
}
-/* Add struct drive to the g->drives list in the handle. */
+/* Add struct drive to the end of the g->drives vector in the handle. */
static void
add_drive_to_handle (guestfs_h *g, struct drive *d)
{
- struct drive **drv = &(g->drives);
-
- while (*drv != NULL)
- drv = &((*drv)->next);
-
- *drv = d;
+ g->nr_drives++;
+ g->drives = safe_realloc (g, g->drives,
+ sizeof (struct drive *) * g->nr_drives);
+ g->drives[g->nr_drives-1] = d;
}
static struct drive *
@@ -73,7 +75,6 @@ create_drive_struct (guestfs_h *g, const char *path,
{
struct drive *drv = safe_malloc (g, sizeof (struct drive));
- drv->next = NULL;
drv->path = safe_strdup (g, path);
drv->readonly = readonly;
drv->format = format ? safe_strdup (g, format) : NULL;
@@ -85,17 +86,30 @@ create_drive_struct (guestfs_h *g, const char *path,
return drv;
}
+/* Called during launch to add a dummy slot to g->drives. */
void
-guestfs___free_drives (struct drive **drives)
+guestfs___add_dummy_appliance_drive (guestfs_h *g)
{
- struct drive *i = *drives;
- *drives = NULL;
+ struct drive *drv;
+
+ drv = create_drive_struct (g, "", 0, NULL, NULL, NULL, 0);
+ add_drive_to_handle (g, drv);
+}
- while (i != NULL) {
- struct drive *next = i->next;
- free_drive_struct (i);
- i = next;
+void
+guestfs___free_drives (guestfs_h *g)
+{
+ struct drive *drv;
+ size_t i;
+
+ ITER_DRIVES (g, i, drv) {
+ free_drive_struct (drv);
}
+
+ free (g->drives);
+
+ g->drives = NULL;
+ g->nr_drives = 0;
}
static void
@@ -383,22 +397,26 @@ guestfs__debug_drives (guestfs_h *g)
char **ret;
struct drive *drv;
- for (count = 0, drv = g->drives; drv; count++, drv = drv->next)
- ;
+ count = 0;
+ ITER_DRIVES (g, i, drv) {
+ count++;
+ }
ret = safe_malloc (g, sizeof (char *) * (count + 1));
- for (i = 0, drv = g->drives; drv; i++, drv = drv->next) {
- ret[i] = safe_asprintf (g, "path=%s%s%s%s%s%s%s%s%s",
- drv->path,
- drv->readonly ? " readonly" :
"",
- drv->format ? " format=" :
"",
- drv->format ? : "",
- drv->iface ? " iface=" : "",
- drv->iface ? : "",
- drv->name ? " name=" : "",
- drv->name ? : "",
- drv->use_cache_none ? " cache=none" :
"");
+ count = 0;
+ ITER_DRIVES (g, i, drv) {
+ ret[count++] + safe_asprintf (g, "path=%s%s%s%s%s%s%s%s%s",
+ drv->path,
+ drv->readonly ? " readonly" : "",
+ drv->format ? " format=" : "",
+ drv->format ? : "",
+ drv->iface ? " iface=" : "",
+ drv->iface ? : "",
+ drv->name ? " name=" : "",
+ drv->name ? : "",
+ drv->use_cache_none ? " cache=none" :
"");
}
ret[count] = NULL;
diff --git a/src/libvirtdomain.c b/src/libvirtdomain.c
index 5a05514..eecea26 100644
--- a/src/libvirtdomain.c
+++ b/src/libvirtdomain.c
@@ -435,7 +435,7 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
/* Checkpoint the command line around the operation so that either
* all disks are added or none are added.
*/
- struct drive **cp = guestfs___checkpoint_drives (g);
+ size_t cp = guestfs___checkpoint_drives (g);
r = guestfs___for_each_disk (g, dom, add_disk, &data);
if (r == -1)
guestfs___rollback_drives (g, cp);
--
1.7.10.4
Richard W.M. Jones
2012-Oct-08 19:05 UTC
[Libguestfs] [PATCH v4 2/5] launch: libvirt: Create qcow2 overlays for read-only drives and the appliance.
From: "Richard W.M. Jones" <rjones at redhat.com>
Instead of adding the snapshot=on option via <qemu:arg>, create qcow2
overlays for any read-only drives and the appliance using 'qemu-img
create' + a temporary file.
This is a workaround for missing support for <transient/> in libvirt's
qemu driver. Also for the unpredictable way that libvirtd handles
$TMPDIR: we want to control where the temporary disk is created.
Currently it is also much slower, because qemu-img is slow. However
we hope to fix qemu upstream.
---
src/launch-libvirt.c | 298 +++++++++++++++++++++++++++++++++++++-------------
1 file changed, 219 insertions(+), 79 deletions(-)
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 7ae37e3..d33693e 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -28,6 +28,7 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/wait.h>
+#include <assert.h>
#ifdef HAVE_LIBVIRT
#include <libvirt/libvirt.h>
@@ -92,12 +93,26 @@ xmlBufferDetach (xmlBufferPtr buf)
}
#endif
-static xmlChar *construct_libvirt_xml (guestfs_h *g, const char
*capabilities_xml, const char *kernel, const char *initrd, const char
*appliance, const char *guestfsd_sock, const char *console_sock, int
disable_svirt);
+/* Pointed to by 'struct drive *' -> priv field. */
+struct drive_libvirt {
+ /* This is either the original path, made absolute. Or for readonly
+ * drives, it is an (absolute path to) the overlay file that we
+ * create. This is always non-NULL.
+ */
+ char *path;
+ /* The format of priv->path. */
+ char *format;
+};
+
+static xmlChar *construct_libvirt_xml (guestfs_h *g, const char
*capabilities_xml, const char *kernel, const char *initrd, const char
*appliance_overlay, const char *guestfsd_sock, const char *console_sock, int
disable_svirt);
static void libvirt_error (guestfs_h *g, const char *fs, ...);
static int is_custom_qemu (guestfs_h *g);
static int is_blk (const char *path);
static int random_chars (char *ret, size_t len);
static void ignore_errors (void *ignore, virErrorPtr ignore2);
+static char *make_qcow2_overlay (guestfs_h *g, const char *path, const char
*format);
+static int make_qcow2_overlay_for_drive (guestfs_h *g, struct drive *drv);
+static void drive_free_priv (void *);
static int
launch_libvirt (guestfs_h *g, const char *libvirt_uri)
@@ -109,6 +124,7 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
char *capabilities = NULL;
xmlChar *xml = NULL;
char *kernel = NULL, *initrd = NULL, *appliance = NULL;
+ char *appliance_overlay = NULL;
char guestfsd_sock[256];
char console_sock[256];
struct sockaddr_un addr;
@@ -116,6 +132,8 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
uint32_t size;
void *buf = NULL;
int disable_svirt = is_custom_qemu (g);
+ struct drive *drv;
+ size_t i;
/* At present you must add drives before starting the appliance. In
* future when we enable hotplugging you won't need to do this.
@@ -182,6 +200,20 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
guestfs___launch_send_progress (g, 3);
TRACE0 (launch_build_libvirt_appliance_end);
+ /* Create overlays for read-only drives and the appliance. This
+ * works around lack of support for <transient/> disks in libvirt.
+ */
+ appliance_overlay = make_qcow2_overlay (g, appliance, "raw");
+ if (!appliance_overlay)
+ goto cleanup;
+
+ ITER_DRIVES (g, i, drv) {
+ if (make_qcow2_overlay_for_drive (g, drv) == -1)
+ goto cleanup;
+ }
+
+ TRACE0 (launch_build_libvirt_qcow2_overlay_end);
+
/* Using virtio-serial, we need to create a local Unix domain socket
* for qemu to connect to.
*/
@@ -278,7 +310,7 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
guestfs___print_timestamped_message (g, "create libvirt XML");
xml = construct_libvirt_xml (g, capabilities,
- kernel, initrd, appliance,
+ kernel, initrd, appliance_overlay,
guestfsd_sock, console_sock,
disable_svirt);
if (!xml)
@@ -381,6 +413,7 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
free (kernel);
free (initrd);
free (appliance);
+ free (appliance_overlay);
free (xml);
free (capabilities);
@@ -412,6 +445,7 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
free (kernel);
free (initrd);
free (appliance);
+ free (appliance_overlay);
free (capabilities);
free (xml);
@@ -431,10 +465,10 @@ static int construct_libvirt_xml_cpu (guestfs_h *g,
xmlTextWriterPtr xo);
static int construct_libvirt_xml_boot (guestfs_h *g, xmlTextWriterPtr xo, const
char *kernel, const char *initrd, size_t appliance_index);
static int construct_libvirt_xml_seclabel (guestfs_h *g, xmlTextWriterPtr xo);
static int construct_libvirt_xml_lifecycle (guestfs_h *g, xmlTextWriterPtr xo);
-static int construct_libvirt_xml_devices (guestfs_h *g, xmlTextWriterPtr xo,
const char *appliance, size_t appliance_index, const char *guestfsd_sock, const
char *console_sock);
+static int construct_libvirt_xml_devices (guestfs_h *g, xmlTextWriterPtr xo,
const char *appliance_overlay, size_t appliance_index, const char
*guestfsd_sock, const char *console_sock);
static int construct_libvirt_xml_qemu_cmdline (guestfs_h *g, xmlTextWriterPtr
xo);
static int construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr xo,
struct drive *drv, size_t drv_index);
-static int construct_libvirt_xml_appliance (guestfs_h *g, xmlTextWriterPtr xo,
const char *appliance, size_t appliance_index);
+static int construct_libvirt_xml_appliance (guestfs_h *g, xmlTextWriterPtr xo,
const char *appliance_overlay, size_t appliance_index);
#define XMLERROR(code,e) do { \
if ((e) == (code)) { \
@@ -447,7 +481,7 @@ static int construct_libvirt_xml_appliance (guestfs_h *g,
xmlTextWriterPtr xo, c
static xmlChar *
construct_libvirt_xml (guestfs_h *g, const char *capabilities_xml,
const char *kernel, const char *initrd,
- const char *appliance,
+ const char *appliance_overlay,
const char *guestfsd_sock, const char *console_sock,
int disable_svirt)
{
@@ -490,7 +524,7 @@ construct_libvirt_xml (guestfs_h *g, const char
*capabilities_xml,
goto err;
if (construct_libvirt_xml_lifecycle (g, xo) == -1)
goto err;
- if (construct_libvirt_xml_devices (g, xo, appliance, appliance_index,
+ if (construct_libvirt_xml_devices (g, xo, appliance_overlay, appliance_index,
guestfsd_sock, console_sock) == -1)
goto err;
if (construct_libvirt_xml_qemu_cmdline (g, xo) == -1)
@@ -675,7 +709,8 @@ construct_libvirt_xml_lifecycle (guestfs_h *g,
xmlTextWriterPtr xo)
/* Devices. */
static int
construct_libvirt_xml_devices (guestfs_h *g, xmlTextWriterPtr xo,
- const char *appliance, size_t appliance_index,
+ const char *appliance_overlay,
+ size_t appliance_index,
const char *guestfsd_sock,
const char *console_sock)
{
@@ -713,7 +748,8 @@ construct_libvirt_xml_devices (guestfs_h *g,
xmlTextWriterPtr xo,
}
/* Appliance disk. */
- if (construct_libvirt_xml_appliance (g, xo, appliance, appliance_index) ==
-1)
+ if (construct_libvirt_xml_appliance (g, xo, appliance_overlay,
+ appliance_index) == -1)
goto err;
/* Console. */
@@ -773,7 +809,7 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
{
char drive_name[64] = "sd";
char scsi_target[64];
- char *path = NULL;
+ struct drive_libvirt *drv_priv;
char *format = NULL;
int is_host_device;
@@ -786,6 +822,8 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
guestfs___drive_name (drv_index, &drive_name[2]);
snprintf (scsi_target, sizeof scsi_target, "%zu", drv_index);
+ drv_priv = (struct drive_libvirt *) drv->priv;
+
/* Change the libvirt XML according to whether the host path is
* a device or a file. For devices, use:
* <disk type=block device=disk>
@@ -794,15 +832,7 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
* <disk type=file device=disk>
* <source file=[path]>
*/
- is_host_device = is_blk (drv->path);
-
- /* Path must be absolute for libvirt. */
- path = realpath (drv->path, NULL);
- if (path == NULL) {
- perrorf (g, _("realpath: could not convert '%s' to absolute
path"),
- drv->path);
- goto err;
- }
+ is_host_device = is_blk (drv_priv->path);
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "disk"));
XMLERROR (-1,
@@ -815,7 +845,8 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "source"));
XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "file",
BAD_CAST path));
+ xmlTextWriterWriteAttribute (xo, BAD_CAST "file",
+ BAD_CAST drv_priv->path));
XMLERROR (-1, xmlTextWriterEndElement (xo));
}
else {
@@ -825,7 +856,8 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "source"));
XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
BAD_CAST path));
+ xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
+ BAD_CAST drv_priv->path));
XMLERROR (-1, xmlTextWriterEndElement (xo));
}
@@ -842,10 +874,10 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
XMLERROR (-1,
xmlTextWriterWriteAttribute (xo, BAD_CAST "name",
BAD_CAST "qemu"));
- if (drv->format) {
+ if (drv_priv->format) {
XMLERROR (-1,
xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
- BAD_CAST drv->format));
+ BAD_CAST drv_priv->format));
}
else {
/* libvirt has disabled the feature of detecting the disk format,
@@ -858,7 +890,7 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
* the users pass the format to libguestfs which will faithfully pass
* that to libvirt and this function won't be used.
*/
- format = guestfs_disk_format (g, path);
+ format = guestfs_disk_format (g, drv_priv->path);
if (!format)
goto err;
@@ -866,7 +898,7 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
error (g, _("could not auto-detect the format of
'%s'\n"
"If the format is known, pass the format to libguestfs,
eg. using the\n"
"'--format' option, or via the optional
'format' argument to 'add-drive'."),
- path);
+ drv->path);
goto err;
}
@@ -899,30 +931,20 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr
xo,
BAD_CAST "0"));
XMLERROR (-1, xmlTextWriterEndElement (xo));
- /* We'd like to do this, but it's not supported by libvirt.
- * See construct_libvirt_xml_qemu_cmdline for the workaround.
- *
- * if (drv->readonly) {
- * XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"transient"));
- * XMLERROR (-1, xmlTextWriterEndElement (xo));
- * }
- */
-
XMLERROR (-1, xmlTextWriterEndElement (xo));
- free (path);
free (format);
return 0;
err:
- free (path);
free (format);
return -1;
}
static int
construct_libvirt_xml_appliance (guestfs_h *g, xmlTextWriterPtr xo,
- const char *appliance, size_t drv_index)
+ const char *appliance_overlay,
+ size_t drv_index)
{
char drive_name[64] = "sd";
char scsi_target[64];
@@ -941,7 +963,7 @@ construct_libvirt_xml_appliance (guestfs_h *g,
xmlTextWriterPtr xo,
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "source"));
XMLERROR (-1,
xmlTextWriterWriteAttribute (xo, BAD_CAST "file",
- BAD_CAST appliance));
+ BAD_CAST appliance_overlay));
XMLERROR (-1, xmlTextWriterEndElement (xo));
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "target"));
@@ -959,7 +981,7 @@ construct_libvirt_xml_appliance (guestfs_h *g,
xmlTextWriterPtr xo,
BAD_CAST "qemu"));
XMLERROR (-1,
xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
- BAD_CAST "raw"));
+ BAD_CAST "qcow2"));
XMLERROR (-1,
xmlTextWriterWriteAttribute (xo, BAD_CAST "cache",
BAD_CAST "unsafe"));
@@ -1004,52 +1026,11 @@ construct_libvirt_xml_appliance (guestfs_h *g,
xmlTextWriterPtr xo,
static int
construct_libvirt_xml_qemu_cmdline (guestfs_h *g, xmlTextWriterPtr xo)
{
- struct drive *drv;
- size_t i;
- char attr[256];
struct qemu_param *qp;
char *p;
XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"qemu:commandline"));
- /* Workaround because libvirt can't do snapshot=on yet. Idea inspired
- * by Stefan Hajnoczi's post here:
- *
http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html
- */
- ITER_DRIVES (g, i, drv) {
- if (drv->readonly) {
- snprintf (attr, sizeof attr,
- "drive.drive-scsi0-0-%zu-0.snapshot=on", i);
-
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"qemu:arg"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "value",
- BAD_CAST "-set"));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
-
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"qemu:arg"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "value",
- BAD_CAST attr));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
- }
- }
-
- snprintf (attr, sizeof attr,
- "drive.drive-scsi0-0-%zu-0.snapshot=on",
g->nr_drives);
-
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "qemu:arg"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "value",
- BAD_CAST "-set"));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
-
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "qemu:arg"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "value",
- BAD_CAST attr));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
-
/* We need to ensure the snapshots are created in $TMPDIR (RHBZ#856619). */
p = getenv ("TMPDIR");
if (p) {
@@ -1162,6 +1143,165 @@ ignore_errors (void *ignore, virErrorPtr ignore2)
/* empty */
}
+/* Create a temporary qcow2 overlay on top of 'path'. */
+static char *
+make_qcow2_overlay (guestfs_h *g, const char *path, const char *format)
+{
+ char *tmpfile = NULL;
+ int fd[2] = { -1, -1 };
+ pid_t pid = -1;
+ FILE *fp = NULL;
+ char *line = NULL;
+ size_t len;
+ int r;
+
+ /* Path must be absolute. */
+ assert (path);
+ assert (path[0] == '/');
+
+ tmpfile = safe_asprintf (g, "%s/snapshot%d", g->tmpdir,
++g->unique);
+
+ /* Because 'qemu-img create' spews junk to stdout and stderr, pass
+ * all output from it up through the event system.
+ * XXX Like libvirt, we should create a generic library for running
+ * commands.
+ */
+ if (pipe2 (fd, O_CLOEXEC) == -1) {
+ perrorf (g, "pipe2");
+ goto error;
+ }
+
+ pid = fork ();
+ if (pid == -1) {
+ perrorf (g, "fork");
+ goto error;
+ }
+
+ if (pid == 0) { /* child: qemu-img create command */
+ /* Capture stdout and stderr. */
+ close (fd[0]);
+ dup2 (fd[1], 1);
+ dup2 (fd[1], 2);
+ close (fd[1]);
+
+ setenv ("LC_ALL", "C", 1);
+
+ if (!format)
+ execlp ("qemu-img", "qemu-img", "create",
"-f", "qcow2",
+ "-b", path, tmpfile, NULL);
+ else {
+ size_t len = strlen (format);
+ char backing_fmt[len+64];
+
+ snprintf (backing_fmt, len+64, "backing_fmt=%s", format);
+
+ execlp ("qemu-img", "qemu-img", "create",
"-f", "qcow2",
+ "-b", path, "-o", backing_fmt, tmpfile,
NULL);
+ }
+
+ perror ("could not execute 'qemu-img create' command");
+ _exit (EXIT_FAILURE);
+ }
+
+ close (fd[1]);
+ fd[1] = -1;
+
+ fp = fdopen (fd[0], "r");
+ if (fp == NULL) {
+ perrorf (g, "fdopen: qemu-img create");
+ goto error;
+ }
+ fd[0] = -1;
+
+ while (getline (&line, &len, fp) != -1) {
+ guestfs___call_callbacks_message (g, GUESTFS_EVENT_LIBRARY, line, len);
+ }
+
+ if (fclose (fp) == -1) { /* also closes fd[0] */
+ perrorf (g, "fclose");
+ fp = NULL;
+ goto error;
+ }
+ fp = NULL;
+
+ free (line);
+ line = NULL;
+
+ if (waitpid (pid, &r, 0) == -1) {
+ perrorf (g, "waitpid");
+ pid = 0;
+ goto error;
+ }
+ pid = 0;
+
+ if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
+ error (g, _("qemu-img create: could not create snapshot over
%s"), path);
+ goto error;
+ }
+
+ return tmpfile; /* caller frees */
+
+ error:
+ if (fd[0] >= 0)
+ close (fd[0]);
+ if (fd[1] >= 0)
+ close (fd[1]);
+ if (fp != NULL)
+ fclose (fp);
+ if (pid > 0)
+ waitpid (pid, NULL, 0);
+
+ free (tmpfile);
+ free (line);
+
+ return NULL;
+}
+
+static int
+make_qcow2_overlay_for_drive (guestfs_h *g, struct drive *drv)
+{
+ char *path;
+ struct drive_libvirt *drv_priv;
+
+ if (drv->priv && drv->free_priv)
+ drv->free_priv (drv->priv);
+
+ drv->priv = drv_priv = safe_calloc (g, 1, sizeof (struct drive_libvirt));
+ drv->free_priv = drive_free_priv;
+
+ /* Even for non-readonly paths, we need to make the paths absolute here. */
+ path = realpath (drv->path, NULL);
+ if (path == NULL) {
+ perrorf (g, _("realpath: could not convert '%s' to absolute
path"),
+ drv->path);
+ return -1;
+ }
+
+ if (!drv->readonly) {
+ drv_priv->path = path;
+ drv_priv->format = drv->format ? safe_strdup (g, drv->format) :
NULL;
+ }
+ else {
+ drv_priv->path = make_qcow2_overlay (g, path, drv->format);
+ free (path);
+ if (!drv_priv->path)
+ return -1;
+ drv_priv->format = safe_strdup (g, "qcow2");
+ }
+
+ return 0;
+}
+
+static void
+drive_free_priv (void *priv)
+{
+ struct drive_libvirt *drv_priv = priv;
+
+ free (drv_priv->path);
+ free (drv_priv->format);
+ free (drv_priv);
+}
+
static int
shutdown_libvirt (guestfs_h *g, int check_for_errors)
{
--
1.7.10.4
Richard W.M. Jones
2012-Oct-08 19:05 UTC
[Libguestfs] [PATCH v4 3/5] launch: Add add_drive 'label' option. New API: list-disk-labels
From: "Richard W.M. Jones" <rjones at 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 | 42 +++++++++++++++---
tests/disk-labels/Makefile.am | 26 +++++++++++
tests/disk-labels/test-disk-labels.pl | 72 +++++++++++++++++++++++++++++++
14 files changed, 311 insertions(+), 9 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..25d2809 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 652cf31..ed9ada4 100644
--- a/src/guestfs-internal.h
+++ b/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. */
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 531faef..378f121 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 d33693e..aa1c39f 100644
--- a/src/launch-libvirt.c
+++ b/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",
diff --git a/src/launch.c b/src/launch.c
index 6e11111..1b6cf4b 100644
--- a/src/launch.c
+++ b/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;
}
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
Richard W.M. Jones
2012-Oct-08 19:05 UTC
[Libguestfs] [PATCH v4 4/5] Add support for hotplugging (adding disks) to the libvirt attach-method.
From: "Richard W.M. Jones" <rjones at redhat.com>
When libvirt is used, we can allow disks to be hotplugged.
guestfs_add_drive can be called after launch to hot-add a disk.
When a disk is hot-added, we first ask libvirt to add the disk to the
appliance, then we make an internal call into the appliance to get it
to wait for the disk to appear (ie. udev_settle ()).
Hot-added disks are tracked in the g->drives array.
This also adds a test.
---
Makefile.am | 1 +
configure.ac | 1 +
daemon/Makefile.am | 1 +
daemon/hotplug.c | 67 ++++++++++++++++++++++++++++
fish/alloc.c | 5 ---
generator/actions.ml | 24 ++++++++--
po/POTFILES | 1 +
src/MAX_PROC_NR | 2 +-
src/guestfs-internal.h | 3 ++
src/guestfs.pod | 38 ++++++++++++++--
src/launch-libvirt.c | 99 +++++++++++++++++++++++++++++++++++++----
src/launch.c | 80 ++++++++++++++++++++++++++++++---
tests/hotplug/Makefile.am | 26 +++++++++++
tests/hotplug/test-hot-add.pl | 66 +++++++++++++++++++++++++++
14 files changed, 386 insertions(+), 28 deletions(-)
create mode 100644 daemon/hotplug.c
create mode 100644 tests/hotplug/Makefile.am
create mode 100755 tests/hotplug/test-hot-add.pl
diff --git a/Makefile.am b/Makefile.am
index 4e476ea..50bfb32 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -52,6 +52,7 @@ SUBDIRS += tests/9p
SUBDIRS += tests/rsync
SUBDIRS += tests/bigdirs
SUBDIRS += tests/disk-labels
+SUBDIRS += tests/hotplug
SUBDIRS += tests/regressions
endif
diff --git a/configure.ac b/configure.ac
index 0422bcb..b4720bc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1379,6 +1379,7 @@ AC_CONFIG_FILES([Makefile
tests/disk-labels/Makefile
tests/extra/Makefile
tests/guests/Makefile
+ tests/hotplug/Makefile
tests/luks/Makefile
tests/lvm/Makefile
tests/md/Makefile
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 2a25c69..9ffff15 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -125,6 +125,7 @@ guestfsd_SOURCES = \
guestfsd.c \
headtail.c \
hexdump.c \
+ hotplug.c \
hivex.c \
htonl.c \
initrd.c \
diff --git a/daemon/hotplug.c b/daemon/hotplug.c
new file mode 100644
index 0000000..aae638e
--- /dev/null
+++ b/daemon/hotplug.c
@@ -0,0 +1,67 @@
+/* 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 <unistd.h>
+#include <errno.h>
+#include <time.h>
+
+#include "guestfs_protocol.h"
+#include "daemon.h"
+#include "actions.h"
+
+#define HOT_ADD_TIMEOUT 30 /* seconds */
+
+/* Wait for /dev/disk/guestfs/<label> to appear. Timeout (and error)
+ * if it doesn't appear after a reasonable length of time.
+ */
+int
+do_internal_hot_add_drive (const char *label)
+{
+ time_t start_t, now_t;
+ size_t len = strlen (label);
+ char path[len+64];
+ int r;
+
+ snprintf (path, len+64, "/dev/disk/guestfs/%s", label);
+
+ time (&start_t);
+
+ while (time (&now_t) - start_t <= HOT_ADD_TIMEOUT) {
+ udev_settle ();
+
+ r = access (path, F_OK);
+ if (r == -1 && errno != ENOENT) {
+ reply_with_perror ("%s", path);
+ return -1;
+ }
+ if (r == 0)
+ return 0;
+
+ sleep (1);
+ }
+
+ reply_with_error ("hot-add drive: '%s' did not appear after %d
seconds: "
+ "this could mean that virtio-scsi (in qemu or kernel)
"
+ "or udev is not working",
+ path, HOT_ADD_TIMEOUT);
+ return -1;
+}
diff --git a/fish/alloc.c b/fish/alloc.c
index f6e5b8f..7454fb7 100644
--- a/fish/alloc.c
+++ b/fish/alloc.c
@@ -72,11 +72,6 @@ alloc_disk (const char *filename, const char *size_str, int
add, int sparse)
if (parse_size (size_str, &size) == -1)
return -1;
- if (!guestfs_is_config (g)) {
- fprintf (stderr, _("can't allocate or add disks after
launching\n"));
- return -1;
- }
-
fd = open (filename, O_WRONLY|O_CREAT|O_NOCTTY|O_TRUNC|O_CLOEXEC, 0666);
if (fd == -1) {
perror (filename);
diff --git a/generator/actions.ml b/generator/actions.ml
index 25d2809..39c71ab 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1186,14 +1186,22 @@ not all belong to a single logical operating system
name = "add_drive";
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;
+ fish_alias = ["add"];
shortdesc = "add an image to examine or modify";
longdesc = "\
This function adds a disk image called C<filename> to the handle.
C<filename> may be a regular host file or a host device.
-The first time you call this function, the disk appears as
-C</dev/sda>, the second time as C</dev/sdb>, and so on.
+When this function is called before C<guestfs_launch> (the
+usual case) then the first time you call this function,
+the disk appears in the API as C</dev/sda>, the second time
+as C</dev/sdb>, and so on.
+
+In libguestfs E<ge> 1.20 you can also call this function
+after launch (with some restrictions). This is called
+\"hotplugging\". When hotplugging, you must specify a
+C<label> so that the new disk gets a predictable name.
+For more information see L<guestfs(3)/HOTPLUGGING>.
You don't necessarily need to be root when using libguestfs. However
you obviously do need sufficient permissions to access the filename
@@ -9951,6 +9959,16 @@ This returns a hashtable, where keys are the disk labels
are the full raw block device and partition names
(eg. C</dev/sda> and C</dev/sda1>)." };
+ { defaults with
+ name = "internal_hot_add_drive";
+ style = RErr, [String "label"], [];
+ proc_nr = Some 370;
+ in_fish = false; in_docs = false;
+ tests = [];
+ shortdesc = "internal hotplugging operation";
+ longdesc = "\
+This function is used internally when hotplugging drives." };
+
]
(* Non-API meta-commands available only in guestfish.
diff --git a/po/POTFILES b/po/POTFILES
index 548156c..22cd148 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -42,6 +42,7 @@ daemon/guestfsd.c
daemon/headtail.c
daemon/hexdump.c
daemon/hivex.c
+daemon/hotplug.c
daemon/htonl.c
daemon/initrd.c
daemon/inotify.c
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 446dfcc..5b0cffb 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-369
+370
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index ed9ada4..db9818c 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -166,6 +166,9 @@ struct attach_ops {
int (*get_pid) (guestfs_h *g); /* get-pid API. */
int (*max_disks) (guestfs_h *g); /* max-disks API. */
+
+ /* Hotplugging drives. */
+ int (*hot_add_drive) (guestfs_h *g, struct drive *drv, size_t drv_index);
};
extern struct attach_ops attach_ops_appliance;
extern struct attach_ops attach_ops_libvirt;
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 48d810b..624e743 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -143,8 +143,11 @@ one you added), etc.
Once L</guestfs_launch> has been called you cannot add any more images.
You can call L</guestfs_list_devices> to get a list of the device
-names, in the order that you added them. See also L</BLOCK DEVICE
-NAMING> below.
+names, in the order that you added them.
+See also L</BLOCK DEVICE NAMING> below.
+
+There are slightly different rules when hotplugging disks (in
+libguestfs E<ge> 1.20). See L</HOTPLUGGING> below.
=head2 MOUNTING
@@ -601,6 +604,35 @@ Libguestfs on top of FUSE performs quite poorly. For best
performance
do not use it. Use ordinary libguestfs filesystem calls, upload,
download etc. instead.
+=head2 HOTPLUGGING
+
+In libguestfs E<ge> 1.20, you may add drives after calling
+L</guestfs_launch>. There are some restrictions, see below.
+This is called I<hotplugging>.
+
+Only a subset of the attach-method backends support hotplugging
+(currently only the libvirt attach-method has support). It also
+requires that you use libvirt E<ge> 0.10.3 and qemu E<ge> 1.2.
+
+To hot-add a disk, simply call L</guestfs_add_drive_opts> after
+L</guestfs_launch>. It is mandatory to specify the C<label>
parameter
+so that the newly added disk has a predictable name. For example:
+
+ if (guestfs_launch (g) == -1)
+ error ("launch failed");
+
+ if (guestfs_add_drive_opts (g, filename,
+ GUESTFS_ADD_DRIVE_OPTS_LABEL, "newdisk",
+ -1) == -1)
+ error ("hot-add of disk failed");
+
+ if (guestfs_part_disk ("/dev/disk/guestfs/newdisk", "mbr")
== -1)
+ error ("partitioning of hot-added disk failed");
+
+Backends that support hotplugging do not require that you add
+E<ge> 1 disk before calling launch. When hotplugging is supported
+you don't need to add any disks.
+
=head2 INSPECTION
Libguestfs has APIs for inspecting an unknown disk image to find out
@@ -2639,7 +2671,7 @@ The guest may be killed by
L</guestfs_kill_subprocess>, or may die
asynchronously at any time (eg. due to some internal error), and that
causes the state to transition back to CONFIG.
-Configuration commands for qemu such as L</guestfs_add_drive> can only
+Configuration commands for qemu such as L</guestfs_set_path> can only
be issued when in the CONFIG state.
The API offers one call that goes from CONFIG through LAUNCHING to
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index aa1c39f..3eab567 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -135,14 +135,6 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
struct drive *drv;
size_t i;
- /* At present you must add drives before starting the appliance. In
- * future when we enable hotplugging you won't need to do this.
- */
- if (!g->nr_drives) {
- error (g, _("you must call guestfs_add_drive before
guestfs_launch"));
- return -1;
- }
-
/* XXX: It should be possible to make this work. */
if (g->direct) {
error (g, _("direct mode flag is not supported yet for libvirt attach
method"));
@@ -1376,6 +1368,89 @@ max_disks_libvirt (guestfs_h *g)
return 255;
}
+static xmlChar *construct_libvirt_xml_hot_add_disk (guestfs_h *g, struct drive
*drv, size_t drv_index);
+
+/* Hot-add a drive. Note the appliance is up when this is called. */
+static int
+hot_add_drive_libvirt (guestfs_h *g, struct drive *drv, size_t drv_index)
+{
+ virConnectPtr conn = g->virt.connv;
+ virDomainPtr dom = g->virt.domv;
+ xmlChar *xml = NULL;
+
+ if (!conn || !dom) {
+ /* This is essentially an internal error if it happens. */
+ error (g, "%s: conn == NULL or dom == NULL", __func__);
+ return -1;
+ }
+
+ /* Create overlay for read-only drive. This works around lack of
+ * support for <transient/> disks in libvirt.
+ */
+ if (make_qcow2_overlay_for_drive (g, drv) == -1)
+ return -1;
+
+ /* Create the XML for the new disk. */
+ xml = construct_libvirt_xml_hot_add_disk (g, drv, drv_index);
+ if (xml == NULL)
+ return -1;
+
+ /* Attach it. */
+ if (virDomainAttachDeviceFlags (dom, (char *) xml,
+ VIR_DOMAIN_DEVICE_MODIFY_LIVE) == -1) {
+ libvirt_error (g, _("could not attach disk to libvirt domain"));
+ goto error;
+ }
+
+ free (xml);
+ return 0;
+
+ error:
+ free (xml);
+ return -1;
+}
+
+static xmlChar *
+construct_libvirt_xml_hot_add_disk (guestfs_h *g, struct drive *drv,
+ size_t drv_index)
+{
+ xmlChar *ret = NULL;
+ xmlBufferPtr xb = NULL;
+ xmlOutputBufferPtr ob;
+ xmlTextWriterPtr xo = NULL;
+
+ XMLERROR (NULL, xb = xmlBufferCreate ());
+ XMLERROR (NULL, ob = xmlOutputBufferCreateBuffer (xb, NULL));
+ XMLERROR (NULL, xo = xmlNewTextWriter (ob));
+
+ XMLERROR (-1, xmlTextWriterSetIndent (xo, 1));
+ XMLERROR (-1, xmlTextWriterSetIndentString (xo, BAD_CAST " "));
+ XMLERROR (-1, xmlTextWriterStartDocument (xo, NULL, NULL, NULL));
+
+ if (construct_libvirt_xml_disk (g, xo, drv, drv_index) == -1)
+ goto err;
+
+ XMLERROR (-1, xmlTextWriterEndDocument (xo));
+ XMLERROR (NULL, ret = xmlBufferDetach (xb)); /* caller frees ret */
+
+ debug (g, "hot-add disk XML:\n%s", ret);
+
+ err:
+ if (xo)
+ xmlFreeTextWriter (xo); /* frees 'ob' too */
+ if (xb)
+ xmlBufferFree (xb);
+
+ return ret;
+}
+
+struct attach_ops attach_ops_libvirt = {
+ .launch = launch_libvirt,
+ .shutdown = shutdown_libvirt,
+ .max_disks = max_disks_libvirt,
+ .hot_add_drive = hot_add_drive_libvirt,
+};
+
#else /* no libvirt or libxml2 at compile time */
#define NOT_IMPL(r) \
@@ -1403,10 +1478,16 @@ max_disks_libvirt (guestfs_h *g)
NOT_IMPL (-1);
}
-#endif /* no libvirt or libxml2 at compile time */
+static int
+hot_add_drive_libvirt (guestfs_h *g, struct drive *drv, size_t drv_index)
+{
+ NOT_IMPL (-1);
+}
struct attach_ops attach_ops_libvirt = {
.launch = launch_libvirt,
.shutdown = shutdown_libvirt,
.max_disks = max_disks_libvirt,
};
+
+#endif /* no libvirt or libxml2 at compile time */
diff --git a/src/launch.c b/src/launch.c
index 1b6cf4b..a4e10fe 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -29,6 +29,7 @@
#include <sys/types.h>
#include <sys/wait.h>
#include <errno.h>
+#include <assert.h>
#include "c-ctype.h"
@@ -57,14 +58,29 @@ guestfs___rollback_drives (guestfs_h *g, size_t old_i)
g->nr_drives = old_i;
}
+/* Add struct drive to the g->drives vector at the given index. */
+static void
+add_drive_to_handle_at (guestfs_h *g, struct drive *d, size_t drv_index)
+{
+ if (drv_index >= g->nr_drives) {
+ g->drives = safe_realloc (g, g->drives,
+ sizeof (struct drive *) * (drv_index + 1));
+ while (g->nr_drives <= drv_index) {
+ g->drives[g->nr_drives] = NULL;
+ g->nr_drives++;
+ }
+ }
+
+ assert (g->drives[drv_index] == NULL);
+
+ g->drives[drv_index] = d;
+}
+
/* Add struct drive to the end of the g->drives vector in the handle. */
static void
add_drive_to_handle (guestfs_h *g, struct drive *d)
{
- g->nr_drives++;
- g->drives = safe_realloc (g, g->drives,
- sizeof (struct drive *) * g->nr_drives);
- g->drives[g->nr_drives-1] = d;
+ add_drive_to_handle_at (g, d, g->nr_drives);
}
static struct drive *
@@ -207,6 +223,48 @@ valid_disk_label (const char *str)
return 1;
}
+/* The low-level function that adds a drive. */
+static int
+add_drive (guestfs_h *g, struct drive *drv)
+{
+ size_t i, drv_index;
+
+ if (g->state == CONFIG) {
+ /* Not hotplugging, so just add it to the handle. */
+ add_drive_to_handle (g, drv);
+ return 0;
+ }
+
+ /* ... else, hotplugging case. */
+ if (!g->attach_ops || !g->attach_ops->hot_add_drive) {
+ error (g, _("the current attach-method does not support hotplugging
drives"));
+ return -1;
+ }
+
+ if (!drv->disk_label) {
+ error (g, _("'label' is required when hotplugging
drives"));
+ return -1;
+ }
+
+ /* Get the first free index, or add it at the end. */
+ drv_index = g->nr_drives;
+ for (i = 0; i < g->nr_drives; ++i)
+ if (g->drives[i] == NULL)
+ drv_index = i;
+
+ /* Hot-add the drive. */
+ if (g->attach_ops->hot_add_drive (g, drv, drv_index) == -1)
+ return -1;
+
+ add_drive_to_handle_at (g, drv, drv_index);
+
+ /* Call into the appliance to wait for the new drive to appear. */
+ if (guestfs_internal_hot_add_drive (g, drv->disk_label) == -1)
+ return -1;
+
+ return 0;
+}
+
/* 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
@@ -220,7 +278,7 @@ add_null_drive (guestfs_h *g, int readonly, const char
*format,
const char *iface, const char *name, const char *disk_label)
{
char *tmpfile = NULL;
- int fd = -1;
+ int fd = -1, r;
struct drive *drv;
if (format && STRNEQ (format, "raw")) {
@@ -252,8 +310,12 @@ add_null_drive (guestfs_h *g, int readonly, const char
*format,
}
drv = create_drive_struct (g, tmpfile, readonly, format, iface, name,
disk_label, 0);
- add_drive_to_handle (g, drv);
+ r = add_drive (g, drv);
free (tmpfile);
+ if (r == -1) {
+ free_drive_struct (drv);
+ return -1;
+ }
return 0;
@@ -328,7 +390,11 @@ guestfs__add_drive_opts (guestfs_h *g, const char
*filename,
drv = create_drive_struct (g, filename, readonly, format, iface, name,
disk_label,
use_cache_none);
- add_drive_to_handle (g, drv);
+ if (add_drive (g, drv) == -1) {
+ free_drive_struct (drv);
+ return -1;
+ }
+
return 0;
}
diff --git a/tests/hotplug/Makefile.am b/tests/hotplug/Makefile.am
new file mode 100644
index 0000000..6644930
--- /dev/null
+++ b/tests/hotplug/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-hot-add.pl
+
+TESTS_ENVIRONMENT = $(top_builddir)/run --test
+
+EXTRA_DIST = \
+ $(TESTS)
diff --git a/tests/hotplug/test-hot-add.pl b/tests/hotplug/test-hot-add.pl
new file mode 100755
index 0000000..b1322e8
--- /dev/null
+++ b/tests/hotplug/test-hot-add.pl
@@ -0,0 +1,66 @@
+#!/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 hot-adding disks.
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+
+my $g = Sys::Guestfs->new ();
+
+# Skip the test if the default attach-method isn't libvirt, since only
+# the libvirt backend supports hotplugging.
+my $attach_method = $g->get_attach_method ();
+unless ($attach_method eq "libvirt" || $attach_method =~ /^libvirt:/)
{
+ print "$0: test skipped because attach-method ($attach_method) is not
libvirt\n";
+ exit 77
+}
+
+# We don't need to add disks before launch.
+$g->launch ();
+
+# Create some temporary disks.
+open FILE, ">test1.img" or die "test1.img: $!";
+truncate FILE, 512 * 1024 * 1024 or die "test1.img: truncate: $!";
+close FILE;
+
+open FILE, ">test2.img" or die "test2.img: $!";
+truncate FILE, 512 * 1024 * 1024 or die "test2.img: truncate: $!";
+close FILE;
+
+die unless system ("qemu-img create -f qcow2 test3.img 1G") == 0;
+
+# Hot-add them. Labels are required.
+$g->add_drive ("test1.img", label => "a"); #
autodetect format
+$g->add_drive ("test2.img", label => "b", format
=> "raw", readonly => 1);
+$g->add_drive ("test3.img", label => "c", format
=> "qcow2");
+
+# Check we can use the disks immediately.
+$g->part_disk ("/dev/disk/guestfs/a", "mbr");
+$g->mkfs ("ext2", "/dev/disk/guestfs/c");
+$g->mkfs ("ext2", "/dev/disk/guestfs/a1");
+
+$g->shutdown ();
+$g->close ();
+
+unlink "test1.img";
+unlink "test2.img";
+unlink "test3.img";
+
+exit 0
--
1.7.10.4
Richard W.M. Jones
2012-Oct-08 19:05 UTC
[Libguestfs] [PATCH v4 5/5] Add support for hotplugging (removing disks). New API: remove-drive.
From: "Richard W.M. Jones" <rjones at redhat.com>
Note because of a bug in fuser, this only works with psmisc >= 22.20.
This also updates the hotplugging tests.
---
daemon/hotplug.c | 101 ++++++++++++++++++++++++++++++++++++--
generator/actions.ml | 39 +++++++++++++++
src/MAX_PROC_NR | 2 +-
src/guestfs-internal.h | 1 +
src/guestfs.pod | 10 ++--
src/launch-libvirt.c | 35 +++++++++++++
src/launch.c | 54 ++++++++++++++++++++
tests/hotplug/Makefile.am | 3 +-
tests/hotplug/test-hot-remove.pl | 88 +++++++++++++++++++++++++++++++++
9 files changed, 324 insertions(+), 9 deletions(-)
create mode 100755 tests/hotplug/test-hot-remove.pl
diff --git a/daemon/hotplug.c b/daemon/hotplug.c
index aae638e..c5e9dde 100644
--- a/daemon/hotplug.c
+++ b/daemon/hotplug.c
@@ -29,6 +29,17 @@
#include "actions.h"
#define HOT_ADD_TIMEOUT 30 /* seconds */
+#define HOT_REMOVE_TIMEOUT HOT_ADD_TIMEOUT
+
+static void
+hot_add_remove_error (const char *op, const char *path, const char *verb,
+ int timeout)
+{
+ reply_with_error ("%s drive: '%s' did not %s after %d seconds:
"
+ "this could mean that virtio-scsi (in qemu or kernel)
"
+ "or udev is not working",
+ op, path, verb, timeout);
+}
/* Wait for /dev/disk/guestfs/<label> to appear. Timeout (and error)
* if it doesn't appear after a reasonable length of time.
@@ -59,9 +70,91 @@ do_internal_hot_add_drive (const char *label)
sleep (1);
}
- reply_with_error ("hot-add drive: '%s' did not appear after %d
seconds: "
- "this could mean that virtio-scsi (in qemu or kernel)
"
- "or udev is not working",
- path, HOT_ADD_TIMEOUT);
+ hot_add_remove_error ("hot-add", path, "appear",
HOT_ADD_TIMEOUT);
+ return -1;
+}
+
+GUESTFSD_EXT_CMD(str_fuser, fuser);
+
+/* This function is called before a drive is hot-unplugged. */
+int
+do_internal_hot_remove_drive_precheck (const char *label)
+{
+ size_t len = strlen (label);
+ char path[len+64];
+ int r;
+ char *out, *err;
+
+ /* Ensure there are no requests in-flight (thanks Paolo Bonzini). */
+ udev_settle ();
+ sync_disks ();
+
+ snprintf (path, len+64, "/dev/disk/guestfs/%s", label);
+
+ r = commandr (&out, &err, str_fuser, "-v", "-m",
path, NULL);
+ if (r == -1) {
+ reply_with_error ("fuser: %s: %s", path, err);
+ free (out);
+ free (err);
+ return -1;
+ }
+ free (err);
+
+ /* "fuser returns a non-zero return code if none of the specified
+ * files is accessed or in case of a fatal error. If at least one
+ * access has been found, fuser returns zero."
+ */
+ if (r == 0) {
+ reply_with_error ("disk with label '%s' is in use "
+ "(eg. mounted or belongs to a volume group)",
label);
+
+ /* Useful for debugging when a drive cannot be unplugged. */
+ if (verbose)
+ fprintf (stderr, "%s\n", out);
+
+ free (out);
+
+ return -1;
+ }
+
+ free (out);
+
+ return 0;
+}
+
+/* This function is called after a drive is hot-unplugged. It checks
+ * that it has really gone and udev has finished processing the
+ * events, in case the user immediately hotplugs a drive with an
+ * identical label.
+ */
+int
+do_internal_hot_remove_drive (const char *label)
+{
+ time_t start_t, now_t;
+ size_t len = strlen (label);
+ char path[len+64];
+ int r;
+
+ snprintf (path, len+64, "/dev/disk/guestfs/%s", label);
+
+ time (&start_t);
+
+ while (time (&now_t) - start_t <= HOT_REMOVE_TIMEOUT) {
+ udev_settle ();
+
+ r = access (path, F_OK);
+ if (r == -1) {
+ if (errno != ENOENT) {
+ reply_with_perror ("%s", path);
+ return -1;
+ }
+ /* else udev has removed the file, so we can return */
+ return 0;
+ }
+
+ sleep (1);
+ }
+
+ hot_add_remove_error ("hot-remove", path, "disappear",
HOT_REMOVE_TIMEOUT);
return -1;
}
diff --git a/generator/actions.ml b/generator/actions.ml
index 39c71ab..0c1f4ab 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -2355,6 +2355,25 @@ backing file.
Note that detecting disk features can be insecure under some
circumstances. See L<guestfs(3)/CVE-2010-3851>." };
+ { defaults with
+ name = "remove_drive";
+ style = RErr, [String "label"], [];
+ tests = [];
+ shortdesc = "remove a disk image";
+ longdesc = "\
+This function is conceptually the opposite of C<guestfs_add_drive_opts>.
+It removes the drive that was previously added with label C<label>.
+
+Note that in order to remove drives, you have to add them with
+labels (see the optional C<label> argument to
C<guestfs_add_drive_opts>).
+If you didn't use a label, then they cannot be removed.
+
+You can call this function before or after launching the handle.
+If called after launch, if the attach-method supports it, we try to hot
+unplug the drive: see L<guestfs(3)/HOTPLUGGING>. The disk B<must
not>
+be in use (eg. mounted) when you do this. We try to detect if the
+disk is in use and stop you from doing this." };
+
]
(* daemon_functions are any functions which cause some action
@@ -9969,6 +9988,26 @@ are the full raw block device and partition names
longdesc = "\
This function is used internally when hotplugging drives." };
+ { defaults with
+ name = "internal_hot_remove_drive_precheck";
+ style = RErr, [String "label"], [];
+ proc_nr = Some 371;
+ in_fish = false; in_docs = false;
+ tests = [];
+ shortdesc = "internal hotplugging operation";
+ longdesc = "\
+This function is used internally when hotplugging drives." };
+
+ { defaults with
+ name = "internal_hot_remove_drive";
+ style = RErr, [String "label"], [];
+ proc_nr = Some 372;
+ in_fish = false; in_docs = false;
+ tests = [];
+ shortdesc = "internal hotplugging operation";
+ longdesc = "\
+This function is used internally when hotplugging drives." };
+
]
(* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 5b0cffb..ba30067 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-370
+372
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index db9818c..273900e 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -169,6 +169,7 @@ struct attach_ops {
/* Hotplugging drives. */
int (*hot_add_drive) (guestfs_h *g, struct drive *drv, size_t drv_index);
+ int (*hot_remove_drive) (guestfs_h *g, struct drive *drv, size_t drv_index);
};
extern struct attach_ops attach_ops_appliance;
extern struct attach_ops attach_ops_libvirt;
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 624e743..0dbc30f 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -606,9 +606,9 @@ download etc. instead.
=head2 HOTPLUGGING
-In libguestfs E<ge> 1.20, you may add drives after calling
-L</guestfs_launch>. There are some restrictions, see below.
-This is called I<hotplugging>.
+In libguestfs E<ge> 1.20, you may add drives and remove after calling
+L</guestfs_launch>. There are some restrictions, see below. This is
+called I<hotplugging>.
Only a subset of the attach-method backends support hotplugging
(currently only the libvirt attach-method has support). It also
@@ -629,6 +629,10 @@ so that the newly added disk has a predictable name. For
example:
if (guestfs_part_disk ("/dev/disk/guestfs/newdisk", "mbr")
== -1)
error ("partitioning of hot-added disk failed");
+To hot-remove a disk, call L</guestfs_remove_drive>. You can call
+this before or after L</guestfs_launch>. You can only remove disks
+that were previously added with a label.
+
Backends that support hotplugging do not require that you add
E<ge> 1 disk before calling launch. When hotplugging is supported
you don't need to add any disks.
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 3eab567..5b7897d 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -1410,6 +1410,40 @@ hot_add_drive_libvirt (guestfs_h *g, struct drive *drv,
size_t drv_index)
return -1;
}
+/* Hot-remove a drive. Note the appliance is up when this is called. */
+static int
+hot_remove_drive_libvirt (guestfs_h *g, struct drive *drv, size_t drv_index)
+{
+ virConnectPtr conn = g->virt.connv;
+ virDomainPtr dom = g->virt.domv;
+ xmlChar *xml = NULL;
+
+ if (!conn || !dom) {
+ /* This is essentially an internal error if it happens. */
+ error (g, "%s: conn == NULL or dom == NULL", __func__);
+ return -1;
+ }
+
+ /* Re-create the XML for the disk. */
+ xml = construct_libvirt_xml_hot_add_disk (g, drv, drv_index);
+ if (xml == NULL)
+ return -1;
+
+ /* Detach it. */
+ if (virDomainDetachDeviceFlags (dom, (char *) xml,
+ VIR_DOMAIN_DEVICE_MODIFY_LIVE) == -1) {
+ libvirt_error (g, _("could not detach disk from libvirt
domain"));
+ goto error;
+ }
+
+ free (xml);
+ return 0;
+
+ error:
+ free (xml);
+ return -1;
+}
+
static xmlChar *
construct_libvirt_xml_hot_add_disk (guestfs_h *g, struct drive *drv,
size_t drv_index)
@@ -1449,6 +1483,7 @@ struct attach_ops attach_ops_libvirt = {
.shutdown = shutdown_libvirt,
.max_disks = max_disks_libvirt,
.hot_add_drive = hot_add_drive_libvirt,
+ .hot_remove_drive = hot_remove_drive_libvirt,
};
#else /* no libvirt or libxml2 at compile time */
diff --git a/src/launch.c b/src/launch.c
index a4e10fe..cc45a10 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -452,6 +452,60 @@ guestfs__add_cdrom (guestfs_h *g, const char *filename)
return guestfs__config (g, "-cdrom", filename);
}
+/* Depending on whether we are hotplugging or not, this function
+ * does slightly different things: If not hotplugging, then the
+ * drive just disappears as if it had never been added. The later
+ * drives "move up" to fill the space. When hotplugging we have to
+ * do some complex stuff, and we usually end up leaving an empty
+ * (NULL) slot in the g->drives vector.
+ */
+int
+guestfs__remove_drive (guestfs_h *g, const char *label)
+{
+ size_t i;
+ struct drive *drv;
+
+ ITER_DRIVES (g, i, drv) {
+ if (drv->disk_label && STREQ (label, drv->disk_label))
+ goto found;
+ }
+ error (g, _("disk with label '%s' not found"), label);
+ return -1;
+
+ found:
+ if (g->state == CONFIG) { /* Not hotplugging. */
+ free_drive_struct (drv);
+
+ g->nr_drives--;
+ for (; i < g->nr_drives; ++i)
+ g->drives[i] = g->drives[i+1];
+
+ return 0;
+ }
+ else { /* Hotplugging. */
+ if (!g->attach_ops || !g->attach_ops->hot_remove_drive) {
+ error (g, _("the current attach-method does not support hotplugging
drives"));
+ return -1;
+ }
+
+ if (guestfs_internal_hot_remove_drive_precheck (g, label) == -1)
+ return -1;
+
+ if (g->attach_ops->hot_remove_drive (g, drv, i) == -1)
+ return -1;
+
+ free_drive_struct (drv);
+ g->drives[i] = NULL;
+ if (i == g->nr_drives-1)
+ g->nr_drives--;
+
+ if (guestfs_internal_hot_remove_drive (g, label) == -1)
+ return -1;
+
+ return 0;
+ }
+}
+
int
guestfs__config (guestfs_h *g,
const char *qemu_param, const char *qemu_value)
diff --git a/tests/hotplug/Makefile.am b/tests/hotplug/Makefile.am
index 6644930..dc5c7e1 100644
--- a/tests/hotplug/Makefile.am
+++ b/tests/hotplug/Makefile.am
@@ -18,7 +18,8 @@
include $(top_srcdir)/subdir-rules.mk
TESTS = \
- test-hot-add.pl
+ test-hot-add.pl \
+ test-hot-remove.pl
TESTS_ENVIRONMENT = $(top_builddir)/run --test
diff --git a/tests/hotplug/test-hot-remove.pl b/tests/hotplug/test-hot-remove.pl
new file mode 100755
index 0000000..af30869
--- /dev/null
+++ b/tests/hotplug/test-hot-remove.pl
@@ -0,0 +1,88 @@
+#!/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 hot-adding and -removing disks.
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+
+my $g = Sys::Guestfs->new ();
+
+# Skip the test if the default attach-method isn't libvirt, since only
+# the libvirt backend supports hotplugging.
+my $attach_method = $g->get_attach_method ();
+unless ($attach_method eq "libvirt" || $attach_method =~ /^libvirt:/)
{
+ print "$0: test skipped because attach-method ($attach_method) is not
libvirt\n";
+ exit 77
+}
+
+# Create some temporary disks.
+open FILE, ">test1.img" or die "test1.img: $!";
+truncate FILE, 512 * 1024 * 1024 or die "test1.img: truncate: $!";
+close FILE;
+
+open FILE, ">test2.img" or die "test2.img: $!";
+truncate FILE, 512 * 1024 * 1024 or die "test2.img: truncate: $!";
+close FILE;
+
+die unless system ("qemu-img create -f qcow2 test3.img 1G") == 0;
+
+# Hot-add them. Labels are required.
+$g->add_drive ("test1.img", label => "a"); #
autodetect format
+$g->add_drive ("test2.img", label => "b", format
=> "raw", readonly => 1);
+$g->add_drive ("test3.img", label => "c", format
=> "qcow2");
+
+# Remove them (before launch).
+$g->remove_drive ("a");
+$g->remove_drive ("b");
+$g->remove_drive ("c");
+
+$g->launch ();
+
+# There should be no drives yet.
+my @devices = $g->list_devices ();
+die unless 0 == @devices;
+
+# Add them again (after launch).
+$g->add_drive ("test1.img", label => "a"); #
autodetect format
+$g->add_drive ("test2.img", label => "b", format
=> "raw", readonly => 1);
+$g->add_drive ("test3.img", label => "c", format
=> "qcow2");
+
+# Check we can use the disks immediately.
+$g->part_disk ("/dev/disk/guestfs/a", "mbr");
+$g->mkfs ("ext2", "/dev/disk/guestfs/c");
+$g->mkfs ("ext2", "/dev/disk/guestfs/a1");
+
+# Remove them (hotplug this time).
+$g->remove_drive ("a");
+$g->remove_drive ("b");
+$g->remove_drive ("c");
+
+# There should be no drives remaining.
+ at devices = $g->list_devices ();
+die unless 0 == @devices;
+
+$g->shutdown ();
+$g->close ();
+
+unlink "test1.img";
+unlink "test2.img";
+unlink "test3.img";
+
+exit 0
--
1.7.10.4
Reasonably Related Threads
- [PATCH v3 0/3] Add support for disk labels and hotplugging.
- [PATCH 0/2] Don't use snapshot=on
- [PATCH v2 0/5] Fix SELinux security contexts so we can access shared disks (RHBZ#912499).
- [PATCH v2 0/4] common/utils: Move libxml2 writer macros to a common header file.
- [PATCH 0/2] Use common macros to help with libxml2 writer.