2:05 p.m.
From: "Richard W.M. Jones" <rjones(a)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.
+@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