[Libguestfs] [PATCH v4 1/5] launch: Make g->drives into an array (was a linked list).

From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; 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