From 4256737227c92d3c9a3139392c2ac56afe0339f4 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 8 Mar 2022 15:23:19 +0000
Subject: [PATCH] lib: Remove drive hotplugging support

This was a feature that allowed you to add drives to the appliance
after launching it.  It was complicated to implement, and only worked
for the libvirt backend (not "direct", which is the default backend).

It also turned out to be a bad idea.  The original concept was that
appliance creation was slow, so to examine multiple guests you should
launch the handle once then hot-add the disks from each guest in turn
to manipulate them.  However this is terrible from a security point of
view, especially for multi-tenant, because the drives from one guest
might compromise the appliance and thus the filesystems/drives from
subsequent guests.

It also turns out that hotplugging is very slow.  Nowadays appliance
creation should be faster than hotplugging.

The main use case for this was virt-df, but virt-df no longer uses it
after we discovered the problems outlined above.
---
 daemon/Makefile.am                     |   1 -
 daemon/hotplug.c                       | 158 -------------------------
 docs/C_SOURCE_FILES                    |   1 -
 docs/guestfs-faq.pod                   |   4 +-
 generator/actions_core.ml              |  53 +--------
 generator/actions_core_deprecated.ml   |   9 ++
 generator/proc_nr.ml                   |   3 -
 lib/drives.c                           |  90 +-------------
 lib/guestfs-internal.h                 |  10 +-
 lib/guestfs.pod                        |  42 +------
 lib/launch-direct.c                    |   3 -
 lib/launch-libvirt.c                   | 125 -------------------
 tests/Makefile.am                      |   8 --
 tests/hotplug/test-hot-add.pl          |  63 ----------
 tests/hotplug/test-hot-remove.pl       |  85 -------------
 tests/hotplug/test-hotplug-repeated.pl |  56 ---------
 16 files changed, 18 insertions(+), 693 deletions(-)
 delete mode 100644 daemon/hotplug.c
 delete mode 100755 tests/hotplug/test-hot-add.pl
 delete mode 100755 tests/hotplug/test-hot-remove.pl
 delete mode 100755 tests/hotplug/test-hotplug-repeated.pl

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 7322bfa5d..bbd49f9ea 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -136,7 +136,6 @@ 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
deleted file mode 100644
index 193a5ffb9..000000000
--- a/daemon/hotplug.c
+++ /dev/null
@@ -1,158 +0,0 @@
-/* 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 <string.h>
-
-#include "guestfs_protocol.h"
-#include "daemon.h"
-#include "actions.h"
-
-#define HOT_ADD_TIMEOUT 30 /* seconds */
-#define HOT_REMOVE_TIMEOUT HOT_ADD_TIMEOUT
-
-static void
-hotplug_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.
- */
-int
-do_internal_hot_add_drive (const char *label)
-{
-  CLEANUP_FREE char *path = NULL;
-  time_t start_t, now_t;
-  int r;
-
-  if (asprintf (&path, "/dev/disk/guestfs/%s", label) == -1) {
-    reply_with_perror ("asprintf");
-    return -1;
-  }
-
-  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);
-  }
-
-  hotplug_error ("hot-add", path, "appear", HOT_ADD_TIMEOUT);
-  return -1;
-}
-
-/* This function is called before a drive is hot-unplugged. */
-int
-do_internal_hot_remove_drive_precheck (const char *label)
-{
-  CLEANUP_FREE char *path = NULL;
-  int r;
-  CLEANUP_FREE char *out = NULL, *err = NULL;
-
-  /* Ensure there are no requests in flight (thanks Paolo Bonzini). */
-  udev_settle ();
-  sync_disks ();
-
-  if (asprintf (&path, "/dev/disk/guestfs/%s", label) == -1) {
-    reply_with_perror ("asprintf");
-    return -1;
-  }
-
-  r = commandr (&out, &err, "fuser", "-v", "-m", path, NULL);
-  if (r == -1) {
-    reply_with_error ("fuser: %s: %s", path, err);
-    return -1;
-  }
-
-  /* "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);
-
-    return -1;
-  }
-
-  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)
-{
-  CLEANUP_FREE char *path = NULL;
-  time_t start_t, now_t;
-  int r;
-
-  if (asprintf (&path, "/dev/disk/guestfs/%s", label) == -1) {
-    reply_with_perror ("asprintf");
-    return -1;
-  }
-
-  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);
-  }
-
-  hotplug_error ("hot-remove", path, "disappear", HOT_REMOVE_TIMEOUT);
-  return -1;
-}
diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index 30147ef49..ca18afbad 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -97,7 +97,6 @@ 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/docs/guestfs-faq.pod b/docs/guestfs-faq.pod
index 15b1247b0..deb05dd97 100644
--- a/docs/guestfs-faq.pod
+++ b/docs/guestfs-faq.pod
@@ -538,8 +538,8 @@ directly:
     non-RHEL
 
 The libvirt backend is more sophisticated, supporting SELinux/sVirt
-(see above), hotplugging and more.  It is, however, more complex and
-so less robust.
+(see above) and more.  It is, however, more complex and so less
+robust.
 
 If you have permissions problems using the libvirt backend, you can
 switch to the direct backend by setting this environment variable:
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index c1176fae5..e757140aa 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -224,12 +224,6 @@ usual case) then the first time you call this function,
 the disk appears in the API as F</dev/sda>, the second time
 as F</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
 for whatever operations you want to perform (ie. read access if you
@@ -1117,25 +1111,6 @@ 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"; added = (1, 19, 49);
-    style = RErr, [String (PlainString, "label")], [];
-    blocking = false;
-    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 backend 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." };
-
   { defaults with
     name = "set_libvirt_supported_credentials"; added = (1, 19, 52);
     style = RErr, [StringList (PlainString, "creds")], [];
@@ -7807,30 +7782,6 @@ This returns a hashtable, where keys are the disk labels
 are the full raw block device and partition names
 (eg. F</dev/sda> and F</dev/sda1>)." };
 
-  { defaults with
-    name = "internal_hot_add_drive"; added = (1, 19, 49);
-    style = RErr, [String (PlainString, "label")], [];
-    visibility = VInternal;
-    shortdesc = "internal hotplugging operation";
-    longdesc = "\
-This function is used internally when hotplugging drives." };
-
-  { defaults with
-    name = "internal_hot_remove_drive_precheck"; added = (1, 19, 49);
-    style = RErr, [String (PlainString, "label")], [];
-    visibility = VInternal;
-    shortdesc = "internal hotplugging operation";
-    longdesc = "\
-This function is used internally when hotplugging drives." };
-
-  { defaults with
-    name = "internal_hot_remove_drive"; added = (1, 19, 49);
-    style = RErr, [String (PlainString, "label")], [];
-    visibility = VInternal;
-    shortdesc = "internal hotplugging operation";
-    longdesc = "\
-This function is used internally when hotplugging drives." };
-
   { defaults with
     name = "mktemp"; added = (1, 19, 53);
     style = RString (RPlainString, "path"), [String (Pathname, "tmpl")], [OString "suffix"];
@@ -8053,9 +8004,7 @@ Call C<guestfs_list_ldm_volumes> and C<guestfs_list_ldm_partitions>
 to return all devices.
 
 Note that you B<don't> normally need to call this explicitly,
-since it is done automatically at C<guestfs_launch> time.
-However you might want to call this function if you have
-hotplugged disks or have just created a Windows dynamic disk." };
+since it is done automatically at C<guestfs_launch> time." };
 
   { defaults with
     name = "ldmtool_remove_all"; added = (1, 20, 0);
diff --git a/generator/actions_core_deprecated.ml b/generator/actions_core_deprecated.ml
index 60d11bfee..00dde3d2a 100644
--- a/generator/actions_core_deprecated.ml
+++ b/generator/actions_core_deprecated.ml
@@ -137,6 +137,15 @@ refers to.
 
 This is the same as the L<lstat(2)> system call." };
 
+  { defaults with
+    name = "remove_drive"; added = (1, 19, 49);
+    style = RErr, [String (PlainString, "label")], [];
+    deprecated_by = Deprecated_no_replacement;
+    blocking = false;
+    shortdesc = "remove a disk image";
+    longdesc = "\
+This call does nothing and returns an error." };
+
 ]
 
 let daemon_functions = [
diff --git a/generator/proc_nr.ml b/generator/proc_nr.ml
index 74b95baf7..b20672ff0 100644
--- a/generator/proc_nr.ml
+++ b/generator/proc_nr.ml
@@ -379,9 +379,6 @@ let proc_nr = [
 367, "rm_f";
 368, "mke2fs";
 369, "list_disk_labels";
-370, "internal_hot_add_drive";
-371, "internal_hot_remove_drive_precheck";
-372, "internal_hot_remove_drive";
 373, "mktemp";
 374, "mklost_and_found";
 375, "acl_get_file";
diff --git a/lib/drives.c b/lib/drives.c
index 46af66db4..fd95308d2 100644
--- a/lib/drives.c
+++ b/lib/drives.c
@@ -737,7 +737,6 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename,
   struct drive_create_data data;
   const char *protocol;
   struct drive *drv;
-  size_t i, drv_index;
 
   data.nr_servers = 0;
   data.servers = NULL;
@@ -917,40 +916,9 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename,
     return 0;
   }
 
-  /* ... else, hotplugging case. */
-  if (!g->backend_ops->hot_add_drive) {
-    error (g, _("the current backend does not support hotplugging drives"));
-    free_drive_struct (drv);
-    return -1;
-  }
-
-  if (!drv->disk_label) {
-    error (g, _("‘label’ is required when hotplugging drives"));
-    free_drive_struct (drv);
-    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->backend_ops->hot_add_drive (g, g->backend_data,
-                                     drv, drv_index) == -1) {
-    free_drive_struct (drv);
-    return -1;
-  }
-
-  add_drive_to_handle_at (g, drv, drv_index);
-  /* drv is now owned by the handle */
-
-  /* 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;
+  /* ... else the old hotplugging case */
+  error (g, _("hotplugging support was removed in libguestfs 1.48"));
+  return -1;
 }
 
 int
@@ -1039,61 +1007,11 @@ guestfs_impl_add_cdrom (guestfs_h *g, const char *filename)
   return guestfs_impl_add_drive_ro (g, filename);
 }
 
-/**
- * This function implements L<guestfs(3)/guestfs_remove_drive>.
- *
- * 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 (C<NULL>) slot in the
- * C<g-E<gt>drives> vector.
- */
 int
 guestfs_impl_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);
+  error (g, _("hotplugging support was removed in libguestfs 1.48"));
   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->backend_ops->hot_remove_drive) {
-      error (g, _("the current backend does not support hotplugging drives"));
-      return -1;
-    }
-
-    if (guestfs_internal_hot_remove_drive_precheck (g, label) == -1)
-      return -1;
-
-    if (g->backend_ops->hot_remove_drive (g, g->backend_data, 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;
-  }
 }
 
 /**
diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index 09ab3ea9e..5bb00bc10 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -280,7 +280,7 @@ enum discard {
 };
 
 /**
- * There is one C<struct drive> per drive, including hot-plugged drives.
+ * There is one C<struct drive> per drive.
  */
 struct drive {
   /* Original source of the drive, eg. file:..., http:... */
@@ -346,10 +346,6 @@ struct backend_ops {
   /* Miscellaneous. */
   int (*get_pid) (guestfs_h *g, void *data);
   int (*max_disks) (guestfs_h *g, void *data);
-
-  /* Hotplugging drives. */
-  int (*hot_add_drive) (guestfs_h *g, void *data, struct drive *drv, size_t drv_index);
-  int (*hot_remove_drive) (guestfs_h *g, void *data, struct drive *drv, size_t drv_index);
 };
 
 /**
@@ -453,10 +449,6 @@ struct guestfs_h {
    * During launch, a dummy slot may be added which represents the
    * slot taken up by the appliance drive.
    *
-   * When hotplugging is supported by the backend, 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).
diff --git a/lib/guestfs.pod b/lib/guestfs.pod
index bf4238ce9..b04c28d62 100644
--- a/lib/guestfs.pod
+++ b/lib/guestfs.pod
@@ -188,9 +188,6 @@ 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.
 
-There are slightly different rules when hotplugging disks (in
-libguestfs E<ge> 1.20).  See L</HOTPLUGGING> below.
-
 =head2 MOUNTING
 
 Before you can read or write files, create directories and so on in a
@@ -659,39 +656,6 @@ 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 and remove after calling
-L</guestfs_launch>.  There are some restrictions, see below.  This is
-called I<hotplugging>.
-
-Only a subset of the backends support hotplugging (currently only the
-libvirt backend 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");
-
-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.
-
 =head2 REMOTE STORAGE
 
 =head3 CEPH
@@ -1478,8 +1442,7 @@ C<libvirt:I<URI>> uses I<URI> as the libvirt connection URI (see
 L<http://libvirt.org/uri.html>).  The typical libvirt backend with a
 URI would be C<libvirt:qemu:///session>
 
-The libvirt backend supports more features, including
-hotplugging (see L</HOTPLUGGING>) and sVirt.
+The libvirt backend supports more features, including sVirt.
 
 =back
 
@@ -3067,9 +3030,6 @@ Before libguestfs 1.19.7, disk names had to be a single character
 that meant the limit was 25.  This has been fixed in more recent
 versions.
 
-In libguestfs E<ge> 1.20 it is possible to hot plug disks.  See
-L</HOTPLUGGING>.
-
 =head2 MAXIMUM NUMBER OF PARTITIONS PER DISK
 
 Virtio limits the maximum number of partitions per disk to B<15>.
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index 4f038f4f0..5c91822fb 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -391,9 +391,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   CLEANUP_FREE char *append = NULL;
   CLEANUP_FREE_STRING_LIST char **argv = NULL;
 
-  /* 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;
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index cc714c02e..44764f3cc 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -2298,137 +2298,12 @@ max_disks_libvirt (guestfs_h *g, void *datav)
   return 255;
 }
 
-static xmlChar *construct_libvirt_xml_hot_add_disk (guestfs_h *g, const struct backend_libvirt_data *data, 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, void *datav,
-                       struct drive *drv, size_t drv_index)
-{
-  struct backend_libvirt_data *data = datav;
-  virConnectPtr conn = data->conn;
-  virDomainPtr dom = data->dom;
-  CLEANUP_FREE 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;
-  }
-
-  /* If the drive has an associated secret, store it in libvirt. */
-  if (add_secret (g, conn, data, drv) == -1)
-    return -1;
-
-  /* Create the XML for the new disk. */
-  xml = construct_libvirt_xml_hot_add_disk (g, data, 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"));
-    return -1;
-  }
-
-  return 0;
-}
-
-/* Hot-remove a drive.  Note the appliance is up when this is called. */
-static int
-hot_remove_drive_libvirt (guestfs_h *g, void *datav,
-                          struct drive *drv, size_t drv_index)
-{
-  struct backend_libvirt_data *data = datav;
-  virConnectPtr conn = data->conn;
-  virDomainPtr dom = data->dom;
-  CLEANUP_FREE 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, data, 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"));
-    return -1;
-  }
-
-  return 0;
-}
-
-static xmlChar *
-construct_libvirt_xml_hot_add_disk (guestfs_h *g,
-                                    const struct backend_libvirt_data *data,
-                                    struct drive *drv,
-                                    size_t drv_index)
-{
-  xmlChar *ret = NULL;
-  CLEANUP_XMLBUFFERFREE xmlBufferPtr xb = NULL;
-  xmlOutputBufferPtr ob;
-  CLEANUP_XMLFREETEXTWRITER xmlTextWriterPtr xo = NULL;
-
-  xb = xmlBufferCreate ();
-  if (xb == NULL) {
-    perrorf (g, "xmlBufferCreate");
-    return NULL;
-  }
-  ob = xmlOutputBufferCreateBuffer (xb, NULL);
-  if (ob == NULL) {
-    perrorf (g, "xmlOutputBufferCreateBuffer");
-    return NULL;
-  }
-  xo = xmlNewTextWriter (ob);
-  if (xo == NULL) {
-    perrorf (g, "xmlNewTextWriter");
-    return NULL;
-  }
-
-  if (xmlTextWriterSetIndent (xo, 1) == -1 ||
-      xmlTextWriterSetIndentString (xo, BAD_CAST "  ") == -1) {
-    perrorf (g, "could not set XML indent");
-    return NULL;
-  }
-  if (xmlTextWriterStartDocument (xo, NULL, NULL, NULL) == -1) {
-    perrorf (g, "xmlTextWriterStartDocument");
-    return NULL;
-  }
-
-  if (construct_libvirt_xml_disk (g, data, xo, drv, drv_index) == -1)
-    return NULL;
-
-  if (xmlTextWriterEndDocument (xo) == -1) {
-    perrorf (g, "xmlTextWriterEndDocument");
-    return NULL;
-  }
-  ret = xmlBufferDetach (xb); /* caller frees ret */
-  if (ret == NULL) {
-    perrorf (g, "xmlBufferDetach");
-    return NULL;
-  }
-
-  debug (g, "hot-add disk XML:\n%s", ret);
-
-  return ret;
-}
-
 static struct backend_ops backend_libvirt_ops = {
   .data_size = sizeof (struct backend_libvirt_data),
   .create_cow_overlay = create_cow_overlay_libvirt,
   .launch = launch_libvirt,
   .shutdown = shutdown_libvirt,
   .max_disks = max_disks_libvirt,
-  .hot_add_drive = hot_add_drive_libvirt,
-  .hot_remove_drive = hot_remove_drive_libvirt,
 };
 
 void
diff --git a/tests/Makefile.am b/tests/Makefile.am
index c65fc2669..f864f82b2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -415,14 +415,6 @@ endif
 TESTS += gdisk/test-expand-gpt.pl
 EXTRA_DIST += gdisk/test-expand-gpt.pl
 
-TESTS += \
-	hotplug/test-hot-add.pl \
-	hotplug/test-hot-remove.pl
-EXTRA_DIST += \
-	hotplug/test-hot-add.pl \
-	hotplug/test-hot-remove.pl \
-	hotplug/test-hotplug-repeated.pl
-
 # Test uses the Python SimpleHTTPServer module which is
 # conveniently part of Python core.
 
diff --git a/tests/hotplug/test-hot-add.pl b/tests/hotplug/test-hot-add.pl
deleted file mode 100755
index 18ff7c47f..000000000
--- a/tests/hotplug/test-hot-add.pl
+++ /dev/null
@@ -1,63 +0,0 @@
-#!/usr/bin/env 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 ();
-
-exit 77 if $ENV{SKIP_TEST_HOT_ADD_PL};
-
-# Skip the test if the default backend isn't libvirt, since only
-# the libvirt backend supports hotplugging.
-my $backend = $g->get_backend ();
-unless ($backend eq "libvirt" || $backend =~ /^libvirt:/) {
-    print "$0: test skipped because backend ($backend) is not libvirt\n";
-    exit 77
-}
-
-# We don't need to add disks before launch.
-$g->launch ();
-
-# Create some temporary disks.
-$g->disk_create ("test-hot-add-1.img", "raw", 512 * 1024 * 1024);
-$g->disk_create ("test-hot-add-2.img", "raw", 512 * 1024 * 1024);
-$g->disk_create ("test-hot-add-3.img", "qcow2", 1024 * 1024 * 1024,
-                 preallocation => "metadata");
-
-# Hot-add them.  Labels are required.
-$g->add_drive ("test-hot-add-1.img", label => "a"); # autodetect format
-$g->add_drive ("test-hot-add-2.img", label => "b", format => "raw", readonly => 1);
-$g->add_drive ("test-hot-add-3.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 "test-hot-add-1.img";
-unlink "test-hot-add-2.img";
-unlink "test-hot-add-3.img";
-
-exit 0
diff --git a/tests/hotplug/test-hot-remove.pl b/tests/hotplug/test-hot-remove.pl
deleted file mode 100755
index 3e7cf72ed..000000000
--- a/tests/hotplug/test-hot-remove.pl
+++ /dev/null
@@ -1,85 +0,0 @@
-#!/usr/bin/env 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 ();
-
-exit 77 if $ENV{SKIP_TEST_HOT_REMOVE_PL};
-
-# Skip the test if the default backend isn't libvirt, since only
-# the libvirt backend supports hotplugging.
-my $backend = $g->get_backend ();
-unless ($backend eq "libvirt" || $backend =~ /^libvirt:/) {
-    print "$0: test skipped because backend ($backend) is not libvirt\n";
-    exit 77
-}
-
-# Create some temporary disks.
-$g->disk_create ("test-hot-remove-1.img", "raw", 512 * 1024 * 1024);
-$g->disk_create ("test-hot-remove-2.img", "raw", 512 * 1024 * 1024);
-$g->disk_create ("test-hot-remove-3.img", "qcow2", 1024 * 1024 * 1024,
-                 preallocation => "metadata");
-
-# Hot-add them.  Labels are required.
-$g->add_drive ("test-hot-remove-1.img", label => "a"); # autodetect format
-$g->add_drive ("test-hot-remove-2.img", label => "b", format => "raw", readonly => 1);
-$g->add_drive ("test-hot-remove-3.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 ("test-hot-remove-1.img", label => "a"); # autodetect format
-$g->add_drive ("test-hot-remove-2.img", label => "b", format => "raw", readonly => 1);
-$g->add_drive ("test-hot-remove-3.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.
-@devices = $g->list_devices ();
-die unless 0 == @devices;
-
-$g->shutdown ();
-$g->close ();
-
-unlink "test-hot-remove-1.img";
-unlink "test-hot-remove-2.img";
-unlink "test-hot-remove-3.img";
-
-exit 0
diff --git a/tests/hotplug/test-hotplug-repeated.pl b/tests/hotplug/test-hotplug-repeated.pl
deleted file mode 100755
index 142f44521..000000000
--- a/tests/hotplug/test-hotplug-repeated.pl
+++ /dev/null
@@ -1,56 +0,0 @@
-#!/usr/bin/env 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 repeatedly hotplugging a single disk.
-
-use strict;
-use warnings;
-
-use Sys::Guestfs;
-
-my $g = Sys::Guestfs->new ();
-
-# Skip the test if the default backend isn't libvirt, since only
-# the libvirt backend supports hotplugging.
-my $backend = $g->get_backend ();
-unless ($backend eq "libvirt" || $backend =~ /^libvirt:/) {
-    print "$0: test skipped because backend ($backend) is not libvirt\n";
-    exit 77
-}
-
-$g->launch ();
-
-# Create a temporary disk.
-$g->disk_create ("test-hotplug-repeated.img", "raw", 512 * 1024 * 1024);
-
-my $start_t = time ();
-while (time () - $start_t <= 60) {
-    $g->add_drive ("test-hotplug-repeated.img",
-                   label => "a", format => "raw");
-    $g->remove_drive ("a");
-}
-
-# There should be no drives remaining.
-my @devices = $g->list_devices ();
-die unless 0 == @devices;
-
-$g->shutdown ();
-$g->close ();
-
-unlink "test-hotplug-repeated.img";
-
-exit 0