[Libguestfs] [PATCH v4 1/5] launch: Make g->drives into an array (was a linked list).
Richard W.M. Jones
rjones at redhat.com
Mon Oct 8 19:05:50 UTC 2012
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
More information about the Libguestfs
mailing list