launch: Make g->drives into an array (was a linked list).

Using an array simplifies the implementation of hotplugging.

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);

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;

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 */

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:

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,

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;
 
-  while (i != NULL) {
-    struct drive *next = i->next;
-    free_drive_struct (i);
-    i = next;
+  drv = create_drive_struct (g, "", 0, NULL, NULL, NULL, 0);
+  add_drive_to_handle (g, drv);
+}
+
+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;

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);