Richard W.M. Jones
2022-Mar-08 15:57 UTC
[Libguestfs] [PATCH 0/4] Remove some experimental features from libguestfs 1.48
I think we should take the opportunity of libguestfs 1.48 to remove some features which are experimental, inadvisable or both. This will also reduce the delta between Fedora and RHEL. The commit messages outline why in my opinion it's right to remove each feature. This doesn't break API/ABI (as in, compiling and linking will still work). Of course if you were using one of these features then that sucks because the APIs will return errors instead of doing what you expected. None of our existing tools use any of these features. (I wouldn't necessarily bother with detailed reviews of each patch -- all the tests pass and if there's something I've overlooked we can remove that later. It's more about do we agree on this as a general direction of travel.) Rich.
Richard W.M. Jones
2022-Mar-08 15:57 UTC
[Libguestfs] [PATCH 1/4] lib: Remove libguestfs live
This experimental feature allowed you (in theory) to connect to an existing instance of the libguestfs daemon. (Again, in theory) it allowed you to attach to running guests. This didn't work well in practice. If you want to do this, install qemu-guest-agent inside your guest instead. This was never supported in RHEL. The daemon tests relied on this connection method to perform tests on a bare daemon, so this removes those tests. They were not especially valuable. See-also: https://bugzilla.redhat.com/798980 --- configure.ac | 1 - docs/C_SOURCE_FILES | 1 - lib/Makefile.am | 1 - lib/guestfs-internal.h | 1 - lib/guestfs.pod | 66 --------------- lib/launch-unix.c | 130 ------------------------------ lib/launch.c | 1 - tests/Makefile.am | 9 --- tests/daemon/captive-daemon.pm.in | 122 ---------------------------- tests/daemon/test-btrfs.pl | 82 ------------------- tests/daemon/test-daemon-start.pl | 35 -------- 11 files changed, 449 deletions(-) diff --git a/configure.ac b/configure.ac index d8383ec173..03180eeafb 100644 --- a/configure.ac +++ b/configure.ac @@ -280,7 +280,6 @@ AC_CONFIG_FILES([Makefile test-data/phony-guests/guests.xml test-tool/Makefile tests/Makefile - tests/daemon/captive-daemon.pm tests/disks/test-qemu-drive-libvirt.xml website/index.html]) diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES index 6a97d8b0e6..7837120173 100644 --- a/docs/C_SOURCE_FILES +++ b/docs/C_SOURCE_FILES @@ -314,7 +314,6 @@ lib/journal.c lib/launch-direct.c lib/launch-libvirt.c lib/launch-uml.c -lib/launch-unix.c lib/launch.c lib/libvirt-auth.c lib/libvirt-domain.c diff --git a/lib/Makefile.am b/lib/Makefile.am index 433dd530cd..5cb7c8e841 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -98,7 +98,6 @@ libguestfs_la_SOURCES = \ launch-direct.c \ launch-libvirt.c \ launch-uml.c \ - launch-unix.c \ libvirt-auth.c \ libvirt-domain.c \ lpj.c \ diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h index 4a19e5c6bb..c6432a219f 100644 --- a/lib/guestfs-internal.h +++ b/lib/guestfs-internal.h @@ -813,7 +813,6 @@ void guestfs_int_init_direct_backend (void) __attribute__((constructor)); void guestfs_int_init_libvirt_backend (void) __attribute__((constructor)); #endif void guestfs_int_init_uml_backend (void) __attribute__((constructor)); -void guestfs_int_init_unix_backend (void) __attribute__((constructor)); /* qemu.c */ struct qemu_data; diff --git a/lib/guestfs.pod b/lib/guestfs.pod index ff58aa0bb2..f9c3d4dc25 100644 --- a/lib/guestfs.pod +++ b/lib/guestfs.pod @@ -1492,14 +1492,6 @@ User-Mode Linux can be much faster, simpler and more lightweight than using a full-blown virtual machine, but it also has some shortcomings. See L</USER-MODE LINUX BACKEND> below. -=item C<unix:I<path>> - -Connect to the Unix domain socket I<path>. - -This method lets you connect to an existing daemon or (using -virtio-serial) to a live guest. For more information, see -L</ATTACHING TO RUNNING DAEMONS>. - =back C<direct> is usually the default backend. However since libguestfs @@ -1561,64 +1553,6 @@ On Fedora, install C<kernel-debuginfo> for the C<vmlinux> file (containing symbols). Make sure the symbols precisely match the kernel being used. -=head2 ATTACHING TO RUNNING DAEMONS - -I<Note (1):> This is B<highly experimental> and has a tendency to eat -babies. Use with caution. - -I<Note (2):> This section explains how to attach to a running daemon -from a low level perspective. For most users, simply using virt tools -such as L<guestfish(1)> with the I<--live> option will "just work". - -=head3 Using guestfs_set_backend - -By calling L</guestfs_set_backend> you can change how the library -connects to the C<guestfsd> daemon in L</guestfs_launch> (read -L<guestfs-internals(1)/ARCHITECTURE> for some background). - -The normal backend is C<direct>, where a small appliance is created -containing the daemon, and then the library connects to this. -C<libvirt> or C<libvirt:I<URI>> are alternatives that use libvirt to -start the appliance. - -Setting the backend to C<unix:I<path>> (where I<path> is the path of a -Unix domain socket) causes L</guestfs_launch> to connect to an -existing daemon over the Unix domain socket. - -The normal use for this is to connect to a running virtual machine -that contains a C<guestfsd> daemon, and send commands so you can read -and write files inside the live virtual machine. - -=head3 Using guestfs_add_domain with live flag - -L</guestfs_add_domain> provides some help for getting the correct -backend. If you pass the C<live> option to this function, then (if -the virtual machine is running) it will examine the libvirt XML -looking for a virtio-serial channel to connect to: - - <domain> - ... - <devices> - ... - <channel type='unix'> - <source mode='bind' path='/path/to/socket'/> - <target type='virtio' name='org.libguestfs.channel.0'/> - </channel> - ... - </devices> - </domain> - -L</guestfs_add_domain> extracts F</path/to/socket> and sets the -backend to C<unix:/path/to/socket>. - -Some of the libguestfs tools (including guestfish) support a I<--live> -option which is passed through to L</guestfs_add_domain> thus allowing -you to attach to and modify live virtual machines. - -The virtual machine needs to have been set up beforehand so that it -has the virtio-serial channel and so that guestfsd is running inside -it. - =head2 USER-MODE LINUX BACKEND Setting the following environment variables (or the equivalent in the diff --git a/lib/launch-unix.c b/lib/launch-unix.c deleted file mode 100644 index 0d344f9df9..0000000000 --- a/lib/launch-unix.c +++ /dev/null @@ -1,130 +0,0 @@ -/* libguestfs - * Copyright (C) 2009-2020 Red Hat Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2 of the License, or (at your option) any later version. - * - * This library 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 - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; 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 <sys/socket.h> -#include <sys/un.h> /* sockaddr_un */ -#include <string.h> -#include <libintl.h> - -#include "guestfs.h" -#include "guestfs-internal.h" -#include "guestfs_protocol.h" - -/* Alternate backend: instead of launching the appliance, - * connect to an existing unix socket. - */ - -static int -launch_unix (guestfs_h *g, void *datav, const char *sockpath) -{ - int r, daemon_sock = -1; - struct sockaddr_un addr; - uint32_t size; - void *buf = NULL; - - if (g->hv_params) { - error (g, _("cannot set hv parameters with the 'unix:' backend")); - return -1; - } - - if (strlen (sockpath) > UNIX_PATH_MAX-1) { - error (g, _("socket filename too long (more than %d characters): %s"), - UNIX_PATH_MAX-1, sockpath); - return -1; - } - - debug (g, "connecting to %s", sockpath); - - daemon_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); - if (daemon_sock == -1) { - perrorf (g, "socket"); - return -1; - } - - addr.sun_family = AF_UNIX; - strncpy (addr.sun_path, sockpath, UNIX_PATH_MAX); - addr.sun_path[UNIX_PATH_MAX-1] = '\0'; - - g->state = LAUNCHING; - - if (connect (daemon_sock, (struct sockaddr *) &addr, sizeof addr) == -1) { - perrorf (g, "bind"); - goto cleanup; - } - - g->conn = guestfs_int_new_conn_socket_connected (g, daemon_sock, -1); - if (!g->conn) - goto cleanup; - - /* g->conn now owns this socket. */ - daemon_sock = -1; - - r = guestfs_int_recv_from_daemon (g, &size, &buf); - free (buf); - - if (r == -1) goto cleanup; - - if (size != GUESTFS_LAUNCH_FLAG) { - error (g, _("guestfs_launch failed, unexpected initial message from guestfsd")); - goto cleanup; - } - - debug (g, "connected"); - - if (g->state != READY) { - error (g, _("contacted guestfsd, but state != READY")); - goto cleanup; - } - - return 0; - - cleanup: - if (daemon_sock >= 0) - close (daemon_sock); - if (g->conn) { - g->conn->ops->free_connection (g, g->conn); - g->conn = NULL; - } - return -1; -} - -static int -shutdown_unix (guestfs_h *g, void *datav, int check_for_errors) -{ - /* Merely closing g->daemon_sock is sufficient and that is already done - * in the calling code. - */ - return 0; -} - -static struct backend_ops backend_unix_ops = { - .data_size = 0, - .launch = launch_unix, - .shutdown = shutdown_unix, -}; - -void -guestfs_int_init_unix_backend (void) -{ - guestfs_int_register_backend ("unix", &backend_unix_ops); -} diff --git a/lib/launch.c b/lib/launch.c index 0e4adcf1c0..0675fe98be 100644 --- a/lib/launch.c +++ b/lib/launch.c @@ -432,5 +432,4 @@ guestfs_int_force_load_backends[] = { guestfs_int_init_libvirt_backend, #endif guestfs_int_init_uml_backend, - guestfs_int_init_unix_backend, }; diff --git a/tests/Makefile.am b/tests/Makefile.am index 3c7a132dfe..707a840225 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -326,15 +326,6 @@ charsets_test_charset_fidelity_LDADD = \ TESTS += create/test-disk-create.sh EXTRA_DIST += create/test-disk-create.sh -check_DATA = daemon/captive-daemon.pm - -TESTS += \ - daemon/test-daemon-start.pl \ - daemon/test-btrfs.pl -EXTRA_DIST += \ - daemon/test-daemon-start.pl \ - daemon/test-btrfs.pl - TESTS += \ discard/test-blkdiscard.pl \ discard/test-discard.pl \ diff --git a/tests/daemon/captive-daemon.pm.in b/tests/daemon/captive-daemon.pm.in deleted file mode 100644 index 77600821af..0000000000 --- a/tests/daemon/captive-daemon.pm.in +++ /dev/null @@ -1,122 +0,0 @@ -# libguestfs -# Copyright (C) 2015 Red Hat Inc. -# @configure_input@ -# -# 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. - -# Small library to help testing the daemon directly. - -package CaptiveDaemon; - -use strict; -use warnings; - -use Sys::Guestfs; - -$| = 1; - -# Filled in by autoconf. -my %var; -$var{abs_top_srcdir} = "@abs_top_srcdir@"; -$var{abs_top_builddir} = "@abs_top_builddir@"; -$var{VALGRIND} = "@VALGRIND@"; - -# Now we have to substitute the above variables into this one: -my $VG = '@VG@'; -$VG =~ s/\$\(([A-Za-z_]+)\)/ $var{"$1"} /ge; - -# Refuse to run if the user is trying to run tests as root. There's -# too much risk that things will go badly wrong. -if ($> == 0) { - print "$0: don't run the libguestfs tests as root!\n"; - exit 77 -} - -sub run_tests { - my $g = Sys::Guestfs->new(); - my $tmpdir = $g->get_tmpdir; - my $verbose = $g->get_verbose; - $g->close; - - # Choose a random name for the socket. The daemon will create the - # socket so we don't need to do that. - my @chars = ("A".."Z", "a".."z", "0".."9"); - my $sockname = $tmpdir; - $sockname .= "/"; - $sockname .= $chars[rand @chars] for 1..8; - - # Assemble the command we will run in the subprocess. - my $cmd - "$var{abs_top_builddir}/daemon/guestfsd --channel $sockname -r -t -l"; - if ($verbose) { - $cmd = $cmd . " -v" - } - if ($ENV{VG}) { # $VG env var will be set when make check-valgrind. - $cmd = $VG . " " . $cmd - } - - if ($verbose) { - print "$0: running: $cmd\n"; - } - - # Fork to run the daemon in the background. - my $pid = fork (); - die "fork: $!" unless defined $pid; - if ($pid == 0) { - # Child process: the daemon. - exec $cmd or die "guestfsd: $!"; - } - - # Wait for the daemon to create the socket, but if it doesn't - # appear after a short timeout, assume there has been a failure. - for (my $i = 0; $i < 10; ++$i) { - last if -S $sockname; - sleep 1; - } - die "subprocess did not create the socket, check earlier messages\n" - unless -S $sockname; - - # Create the libguestfs handle and connect to the daemon using - # libguestfs live. - $g = Sys::Guestfs->new (); - $g->set_backend ("unix:" . $sockname); - $g->set_autosync (0); - $g->launch; - - # Run the user tests. - my $r = ::tests ($g); - - # Tell the daemon to exit cleanly, and remove the socket. - $g->internal_exit; - $g->close; - unlink $sockname; - - waitpid ($pid, 0) or die "waitpid: $!"; - if ($? != 0) { - my $signal = $? & 127; - die "ERROR: guestfsd died on signal $signal\n" if $signal; - my $crash = $? & 128; - die "ERROR: guestfsd core dumped\n" if $crash; - my $status = $? >> 8; - die "ERROR: guestfsd died with exit code 119 (valgrind failure)\n" - if $status == 119; - die "ERROR: guestfsd died with exit code $status\n"; - } - - # Exit with failure if the user test failed. - exit 1 unless $r -} - -1; diff --git a/tests/daemon/test-btrfs.pl b/tests/daemon/test-btrfs.pl deleted file mode 100755 index b7f4214a89..0000000000 --- a/tests/daemon/test-btrfs.pl +++ /dev/null @@ -1,82 +0,0 @@ -#!/usr/bin/env perl -# libguestfs -# Copyright (C) 2015 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 that the daemon starts and stops. - -use strict; -use warnings; - -use File::Temp qw/tempdir/; - -require './daemon/captive-daemon.pm'; - -# Set $PATH to include directory that will have phony 'btrfs' binary. -my $bindir = tempdir (CLEANUP => 1); -$ENV{PATH} = $bindir . ":" . $ENV{PATH}; - -sub set_btrfs_output { - my $output = shift; - open BTRFS, ">$bindir/btrfs" or die "$bindir/btrfs: $!"; - print BTRFS "#!/bin/sh\n"; - print BTRFS "cat << '__EOF'\n"; - print BTRFS $output; - print BTRFS "__EOF\n"; - close BTRFS; - chmod 0755, "$bindir/btrfs" or die "chmod: $bindir/btrfs: $!"; -} - -sub tests { - my $g = shift; - - # Test btrfs_subvolume_list. - my $output = <<EOF; -ID 256 gen 30 top level 5 path test1 -ID 257 gen 30 top level 5 path dir/test2 -ID 258 gen 30 top level 5 path test3 -EOF - set_btrfs_output ($output); - my @r = $g->btrfs_subvolume_list ("/"); - die unless @r == 3; - die unless $r[0]->{btrfssubvolume_id} == 256; - die unless $r[0]->{btrfssubvolume_top_level_id} == 5; - die unless $r[0]->{btrfssubvolume_path} eq "test1"; - die unless $r[1]->{btrfssubvolume_id} == 257; - die unless $r[1]->{btrfssubvolume_top_level_id} == 5; - die unless $r[1]->{btrfssubvolume_path} eq "dir/test2"; - die unless $r[2]->{btrfssubvolume_id} == 258; - die unless $r[2]->{btrfssubvolume_top_level_id} == 5; - die unless $r[2]->{btrfssubvolume_path} eq "test3"; - - # Test btrfs_qgroup_show. - $output = <<EOF; -qgroupid rfer excl --------- ---- ---- -0/5 4096 4096 -EOF - set_btrfs_output ($output); - @r = $g->btrfs_qgroup_show ("/"); - die unless @r == 1; - die unless $r[0]->{btrfsqgroup_id} eq "0/5"; - die unless $r[0]->{btrfsqgroup_rfer} == 4096; - die unless $r[0]->{btrfsqgroup_excl} == 4096; - - # Return true to indicate the test succeeded. - 1; -} - -CaptiveDaemon::run_tests () diff --git a/tests/daemon/test-daemon-start.pl b/tests/daemon/test-daemon-start.pl deleted file mode 100755 index bac4d08c53..0000000000 --- a/tests/daemon/test-daemon-start.pl +++ /dev/null @@ -1,35 +0,0 @@ -#!/usr/bin/env perl -# libguestfs -# Copyright (C) 2015 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 that the daemon starts and stops. - -use strict; -use warnings; - -require './daemon/captive-daemon.pm'; - -sub tests { - my $g = shift; - - $g->ping_daemon; - - # Return true to indicate the test succeeded. - 1; -} - -CaptiveDaemon::run_tests () -- 2.35.1
Richard W.M. Jones
2022-Mar-08 15:57 UTC
[Libguestfs] [PATCH 2/4] lib: Remove User-Mode Linux
User-Mode Linux was an experimental alternative way to run the daemon instead of using qemu. It had many limitations, and UML support in Linux has been semi-broken for a long time. This was never supported in RHEL. See-also: https://bugzilla.redhat.com/1144197 --- Makefile.am | 22 - TODO | 5 - docs/C_SOURCE_FILES | 1 - docs/guestfs-building.pod | 4 - docs/guestfs-hacking.pod | 15 - docs/guestfs-performance.pod | 37 -- fuse/test-fuse-umount-race.sh | 2 - generator/actions_properties.ml | 3 +- lib/Makefile.am | 1 - lib/appliance.c | 16 +- lib/guestfs-internal.h | 3 +- lib/guestfs.pod | 89 --- lib/launch-uml.c | 607 -------------------- lib/launch.c | 9 - tests/c-api/test-add-libvirt-dom.c | 8 - tests/disk-labels/test-disk-labels.pl | 5 - tests/md/test-inspect-fstab.sh | 1 - tests/mountable/test-mountable-inspect.sh | 1 - tests/nbd/test-nbd.pl | 5 - tests/network/test-network.sh | 1 - tests/qemu/qemu-force-tcg.sh | 2 - tests/qemu/qemu-snapshot-isolation-qcow2.sh | 2 - tests/regressions/rhbz1370424.sh | 1 - tests/regressions/rhbz690819.sh | 2 - tests/regressions/rhbz975797.sh | 2 - tests/relative-paths/test-relative-paths.sh | 4 - tests/rsync/test-rsync.sh | 4 - tests/test-functions.sh | 2 +- 28 files changed, 4 insertions(+), 850 deletions(-) diff --git a/Makefile.am b/Makefile.am index e473ab1147..0260639171 100644 --- a/Makefile.am +++ b/Makefile.am @@ -302,8 +302,6 @@ check-all: check-valgrind \ check-direct \ check-valgrind-direct \ - check-uml \ - check-valgrind-uml \ check-with-upstream-qemu \ check-with-upstream-libvirt \ check-slow @@ -341,24 +339,6 @@ check-valgrind-direct: check-valgrind-with-appliance: check-valgrind-direct -# Tests which currently fail under UML: -# - blockdev --setro seems to have no effect on /dev/ubd* devices [*] -# - RHBZ#914931: test is sent a SIGTERM, apparently by UML [*] -# - tests/md/test-inspect-fstab-md.sh hangs at various places during the -# test, eg. running mdadm, mounting MD filesystem [*] -# [*] = likely to be a bug in UML itself -SKIP_TESTS_FAILING_IN_UML = \ - SKIP_TEST_BLOCKDEV_GETRO=1 \ - SKIP_TEST_BLOCKDEV_SETRO=1 \ - SKIP_TEST_RHBZ914931=1 \ - SKIP_TEST_INSPECT_FSTAB_MD_SH=1 - -check-uml: - $(MAKE) LIBGUESTFS_BACKEND=uml $(SKIP_TESTS_FAILING_IN_UML) check - -check-valgrind-uml: - $(MAKE) LIBGUESTFS_BACKEND=uml $(SKIP_TESTS_FAILING_IN_UML) check-valgrind - QEMUDIR = $(HOME)/d/qemu QEMUBINARY = $(QEMUDIR)/x86_64-softmmu/qemu-system-x86_64 @@ -499,8 +479,6 @@ help: @echo "make check-valgrind Run a subset of the tests under valgrind." @echo "make check-direct Test using direct backend." @echo "make check-valgrind-direct Test valgrind + direct backend." - @echo "make check-uml Test using User-Mode Linux." - @echo "make check-valgrind-uml Test valgrind + User-Mode Linux." @echo "make check-with-upstream-qemu Test using upstream qemu." @echo "make check-with-upstream-libvirt Test using upstream libvirt." @echo "make check-slow Slow/long-running tests." diff --git a/TODO b/TODO index 6fb11d5ecd..a50f7d73c3 100644 --- a/TODO +++ b/TODO @@ -532,11 +532,6 @@ virt-builder - /etc/resolv.conf handling works but is best described as a hack: https://github.com/libguestfs/libguestfs/commit/9521422ce60578f7196cc8b7977d998159238c19 - - let's make UML work - + SLIRP is insecure, but we could allow just a bare web proxy which - gets proxied over virtio-serial to the outside world (except - virtio-serial can't be multiplexed) - - sometimes (not always) aug_init takes ages, why? Midnight Commander (mc) extension diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES index 7837120173..30147ef493 100644 --- a/docs/C_SOURCE_FILES +++ b/docs/C_SOURCE_FILES @@ -313,7 +313,6 @@ lib/inspect-osinfo.c lib/journal.c lib/launch-direct.c lib/launch-libvirt.c -lib/launch-uml.c lib/launch.c lib/libvirt-auth.c lib/libvirt-domain.c diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod index e46a1222e6..b93a611a6a 100644 --- a/docs/guestfs-building.pod +++ b/docs/guestfs-building.pod @@ -271,10 +271,6 @@ Optional. Used only for testing. Optional. qemu-nbd is used for testing. -=item uml_mkcow - -Optional. For the L<UML backend|guestfs(3)/BACKEND>. - =item curl Optional. Used by virt-builder for downloads. diff --git a/docs/guestfs-hacking.pod b/docs/guestfs-hacking.pod index 1527c4fbd1..5a64740f18 100644 --- a/docs/guestfs-hacking.pod +++ b/docs/guestfs-hacking.pod @@ -649,21 +649,6 @@ using C<./configure --with-default-backend=...> Run a subset of the test suite under valgrind using the default appliance back-end. -=item C<make check-uml> - -Runs all tests using the User-Mode Linux backend. - -As there is no standard location for the User-Mode Linux kernel, you -I<have> to set C<LIBGUESTFS_HV> to point to the kernel image, eg: - - make check-uml LIBGUESTFS_HV=~/d/linux-um/vmlinux - -=item C<make check-valgrind-uml> - -Runs all tests using the User-Mode Linux backend, under valgrind. - -As above, you have to set C<LIBGUESTFS_HV> to point to the kernel. - =item C<make check-with-upstream-qemu> Runs all tests using a local qemu binary. It looks for the qemu diff --git a/docs/guestfs-performance.pod b/docs/guestfs-performance.pod index b26c6a89bb..acd770a824 100644 --- a/docs/guestfs-performance.pod +++ b/docs/guestfs-performance.pod @@ -365,43 +365,6 @@ L<http://rwmj.wordpress.com/2013/02/25/multiple-libguestfs-appliances-in-paralle printf ("%d %.2f\n", $nr_threads, $end_t - $start_t); } -=head1 USING USER-MODE LINUX - -Since libguestfs 1.24, it has been possible to use the User-Mode Linux -(uml) backend instead of KVM -(see L<guestfs(3)/USER-MODE LINUX BACKEND>). This section makes some -general remarks about this backend, but it is B<highly advisable> to -measure your own workload under UML rather than trusting comments or -intuition. - -=over 4 - -=item * - -UML usually performs the same or slightly slower than KVM, on baremetal. - -=item * - -However UML often performs the same under virtualization as it does on -baremetal, whereas KVM can run much slower under virtualization (since -hardware virt acceleration is not available). - -=item * - -Upload and download is as much as 10 times slower on UML than KVM. -Libguestfs sends this data over the UML emulated serial port, which is -far less efficient than KVM?s virtio-serial. - -=item * - -UML lacks some features (eg. qcow2 support), so it may not be -applicable at all. - -=back - -For some actual figures, see: -L<http://rwmj.wordpress.com/2013/08/14/performance-of-user-mode-linux-as-a-libguestfs-backend/#content> - =head1 TROUBLESHOOTING POOR PERFORMANCE =head2 Ensure hardware virtualization is available diff --git a/fuse/test-fuse-umount-race.sh b/fuse/test-fuse-umount-race.sh index cfb5fe38f4..d69ffbbf47 100755 --- a/fuse/test-fuse-umount-race.sh +++ b/fuse/test-fuse-umount-race.sh @@ -25,8 +25,6 @@ $TEST_FUNCTIONS skip_if_skipped "test-fuse.sh" skip_if_skipped skip_unless_phony_guest fedora.img -# UML backend does not support qcow2. -skip_if_backend uml skip_unless_fuse rm -f test.qcow2 test-copy.qcow2 test.pid diff --git a/generator/actions_properties.ml b/generator/actions_properties.ml index 1ef370d1be..18350a3888 100644 --- a/generator/actions_properties.ml +++ b/generator/actions_properties.ml @@ -32,8 +32,7 @@ let non_daemon_functions = [ longdesc = "\ Set the hypervisor binary that we will use. The hypervisor depends on the backend, but is usually the location of the -qemu/KVM hypervisor. For the uml backend, it is the location -of the C<linux> or C<vmlinux> binary. +qemu/KVM hypervisor. The default is chosen when the library was compiled by the configure script. diff --git a/lib/Makefile.am b/lib/Makefile.am index 5cb7c8e841..144c45588b 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -97,7 +97,6 @@ libguestfs_la_SOURCES = \ launch.c \ launch-direct.c \ launch-libvirt.c \ - launch-uml.c \ libvirt-auth.c \ libvirt-domain.c \ lpj.c \ diff --git a/lib/appliance.c b/lib/appliance.c index 6b4ee78022..583b6a260d 100644 --- a/lib/appliance.c +++ b/lib/appliance.c @@ -281,21 +281,7 @@ build_supermin_appliance (guestfs_h *g, /* Touch the files so they don't get deleted (as they are in /var/tmp). */ (void) utimes (appliance->kernel, NULL); (void) utimes (appliance->initrd, NULL); - - /* Checking backend != "uml" is a big hack. UML encodes the mtime - * of the original backing file (in this case, the appliance) in the - * COW file, and checks it when adding it to the VM. If there are - * multiple threads running and one touches the appliance here, it - * will disturb the mtime and UML will give an error. - * - * We can get rid of this hack as soon as UML fixes the - * ubdN=cow,original parsing bug, since we won't need to run - * uml_mkcow separately, so there is no possible race. - * - * XXX - */ - if (STRNEQ (g->backend, "uml")) - (void) utimes (appliance->image, NULL); + (void) utimes (appliance->image, NULL); return 0; } diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h index c6432a219f..09ab3ea9e7 100644 --- a/lib/guestfs-internal.h +++ b/lib/guestfs-internal.h @@ -292,7 +292,7 @@ struct drive { * it is non-NULL, else consult the original source above. * * Note that the overlay is in a backend-specific format, probably - * different from the source format. eg. qcow2, UML COW. + * different from the source format. eg. qcow2 */ char *overlay; @@ -812,7 +812,6 @@ void guestfs_int_init_direct_backend (void) __attribute__((constructor)); #ifdef HAVE_LIBVIRT_BACKEND void guestfs_int_init_libvirt_backend (void) __attribute__((constructor)); #endif -void guestfs_int_init_uml_backend (void) __attribute__((constructor)); /* qemu.c */ struct qemu_data; diff --git a/lib/guestfs.pod b/lib/guestfs.pod index f9c3d4dc25..bf4238ce93 100644 --- a/lib/guestfs.pod +++ b/lib/guestfs.pod @@ -1481,17 +1481,6 @@ URI would be C<libvirt:qemu:///session> The libvirt backend supports more features, including hotplugging (see L</HOTPLUGGING>) and sVirt. -=item C<uml> - -Run the User-Mode Linux kernel. The location of the kernel is set -using C<$LIBGUESTFS_HV> or using the L</guestfs_set_qemu> API (note -that qemu is not involved, we just reuse the same variable in the -handle for convenience). - -User-Mode Linux can be much faster, simpler and more lightweight than -using a full-blown virtual machine, but it also has some shortcomings. -See L</USER-MODE LINUX BACKEND> below. - =back C<direct> is usually the default backend. However since libguestfs @@ -1553,84 +1542,6 @@ On Fedora, install C<kernel-debuginfo> for the C<vmlinux> file (containing symbols). Make sure the symbols precisely match the kernel being used. -=head2 USER-MODE LINUX BACKEND - -Setting the following environment variables (or the equivalent in the -API) selects the User-Mode Linux backend: - - export LIBGUESTFS_BACKEND=uml - export LIBGUESTFS_HV=/path/to/vmlinux - -C<vmlinux> (or it may be called C<linux>) is the Linux binary, -compiled to run as a userspace process. Note that we reuse the qemu -variable in the handle for convenience; qemu is not involved. - -User-Mode Linux can be faster and more lightweight than running a -full-blown virtual machine as the backend (especially if you are -already running libguestfs in a virtual machine or cloud instance), -but it also has some shortcomings compared to the usual qemu/KVM-based -backend. - -=head3 BUILDING USER-MODE LINUX FROM SOURCE - -Your Linux distro may provide UML in which case you can ignore this -section. - -These instructions are adapted from: -L<http://user-mode-linux.sourceforge.net/source.html> - -=over 4 - -=item 1. Check out Linux sources - -Clone the Linux git repository or download the Linux source tarball. - -=item 2. Configure the kernel - -B<Note:> All ?make? commands must have C<ARCH=um> added. - - make menuconfig ARCH=um - -Make sure any filesystem drivers that you need are compiled into the -kernel. - -B<Currently, it needs a large amount of extra work to get modules -working>. It?s recommended that you disable module support in the -kernel configuration, which will cause everything to be compiled into -the image. - -=item 3. Build the kernel - - make ARCH=um - -This will leave a file called C<linux> or C<vmlinux> in the top-level -directory. This is the UML kernel. You should set C<LIBGUESTFS_HV> -to point to this file. - -=back - -=head3 USER-MODE LINUX DIFFERENCES FROM KVM - -=over 4 - -=item UML only supports raw-format images - -Only plain raw-format images will work. No qcow2, no backing files. - -=item UML does not support any remote drives - -No NBD, etc. - -=item UML only works on ix86 and x86-64 - -=item UML is experimental - -In particular, support for UML in libguestfs depends on support for -UML in the upstream kernel. If UML was ever removed from the upstream -Linux kernel, then we might remove it from libguestfs too. - -=back - =head2 ABI GUARANTEE We guarantee the libguestfs ABI (binary interface), for public, diff --git a/lib/launch-uml.c b/lib/launch-uml.c deleted file mode 100644 index 5aec50a578..0000000000 --- a/lib/launch-uml.c +++ /dev/null @@ -1,607 +0,0 @@ -/* libguestfs - * Copyright (C) 2009-2020 Red Hat Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2 of the License, or (at your option) any later version. - * - * This library 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 - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; 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 <string.h> -#include <stdbool.h> -#include <inttypes.h> -#include <unistd.h> -#include <sys/types.h> -#include <sys/socket.h> -#include <sys/signal.h> -#include <libintl.h> - -#include "cloexec.h" - -#include "guestfs.h" -#include "guestfs-internal.h" -#include "guestfs_protocol.h" - -/* Per-handle data. */ -struct backend_uml_data { - pid_t pid; /* vmlinux PID. */ - pid_t recoverypid; /* Recovery process PID. */ - -#define UML_UMID_LEN 16 - char umid[UML_UMID_LEN+1]; /* umid=<...> unique ID. */ -}; - -static void print_vmlinux_command_line (guestfs_h *g, char **argv); - -/* Run uml_mkcow to create a COW overlay. */ -static char * -make_cow_overlay (guestfs_h *g, const char *original) -{ - CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g); - char *overlay; - int r; - - overlay = guestfs_int_make_temp_path (g, "overlay", "qcow2"); - if (!overlay) - return NULL; - - guestfs_int_cmd_add_arg (cmd, "uml_mkcow"); - guestfs_int_cmd_add_arg (cmd, overlay); - guestfs_int_cmd_add_arg (cmd, original); - r = guestfs_int_cmd_run (cmd); - if (r == -1) { - free (overlay); - return NULL; - } - if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { - guestfs_int_external_command_failed (g, r, "uml_mkcow", original); - free (overlay); - return NULL; - } - - return overlay; -} - -static char * -create_cow_overlay_uml (guestfs_h *g, void *datav, struct drive *drv) -{ - return make_cow_overlay (g, drv->src.u.path); -} - -/* Test for features which are not supported by the UML backend. - * Possibly some of these should just be warnings, not errors. - */ -static bool -uml_supported (guestfs_h *g) -{ - size_t i; - struct drive *drv; - - if (g->enable_network) { - error (g, _("uml backend does not support networking")); - return false; - } - if (g->smp > 1) { - error (g, _("uml backend does not support SMP")); - return false; - } - - ITER_DRIVES (g, i, drv) { - if (drv->src.protocol != drive_protocol_file) { - error (g, _("uml backend does not support remote drives")); - return false; - } - if (drv->src.format && STRNEQ (drv->src.format, "raw")) { - error (g, _("uml backend does not support non-raw-format drives")); - return false; - } - if (drv->iface) { - error (g, - _("uml backend does not support drives with ?iface? parameter")); - return false; - } - if (drv->disk_label) { - error (g, - _("uml backend does not support drives with ?label? parameter")); - return false; - } - /* Note that discard == "besteffort" is fine. */ - if (drv->discard == discard_enable) { - error (g, - _("uml backend does not support drives with ?discard? parameter set to ?enable?")); - return false; - } - if (drv->blocksize) { - error (g, - _("uml backend does not support drives with ?blocksize? parameter")); - return false; - } - } - - return true; -} - -static int -launch_uml (guestfs_h *g, void *datav, const char *arg) -{ - struct backend_uml_data *data = datav; - CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (cmdline); - int console_sock = -1, daemon_sock = -1; - int r; - int csv[2], dsv[2]; - CLEANUP_FREE char *kernel = NULL, *initrd = NULL, *appliance = NULL; - int has_appliance_drive; - CLEANUP_FREE char *appliance_cow = NULL; - uint32_t size; - CLEANUP_FREE void *buf = NULL; - struct drive *drv; - size_t i; - struct hv_param *hp; - char *term = getenv ("TERM"); - - if (!uml_supported (g)) - return -1; - - if (!g->nr_drives) { - error (g, _("you must call guestfs_add_drive before guestfs_launch")); - return -1; - } - - /* Assign a random unique ID to this run. */ - if (guestfs_int_random_string (data->umid, UML_UMID_LEN) == -1) { - perrorf (g, "guestfs_int_random_string"); - return -1; - } - - /* Locate and/or build the appliance. */ - if (guestfs_int_build_appliance (g, &kernel, &initrd, &appliance) == -1) - return -1; - has_appliance_drive = appliance != NULL; - - /* Create COW overlays for the appliance. Note that the documented - * syntax ubd0=cow,orig does not work since kernel 3.3. See: - * http://thread.gmane.org/gmane.linux.uml.devel/13556 - */ - if (has_appliance_drive) { - appliance_cow = make_cow_overlay (g, appliance); - if (!appliance_cow) - goto cleanup0; - } - - /* The socket that the daemon will talk to us on. - */ - if (socketpair (AF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC, 0, dsv) == -1) { - perrorf (g, "socketpair"); - goto cleanup0; - } - - /* The console socket. */ - if (!g->direct_mode) { - if (socketpair (AF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC, 0, csv) == -1) { - perrorf (g, "socketpair"); - close (dsv[0]); - close (dsv[1]); - goto cleanup0; - } - } - - /* Construct the vmlinux command line. We have to do this before - * forking, because after fork we are not allowed to use - * non-signal-safe functions such as malloc. - */ -#define ADD_CMDLINE(str) \ - guestfs_int_add_string (g, &cmdline, (str)) -#define ADD_CMDLINE_PRINTF(fs,...) \ - guestfs_int_add_sprintf (g, &cmdline, (fs), ##__VA_ARGS__) - - ADD_CMDLINE (g->hv); - - /* Give this instance a unique random ID. */ - ADD_CMDLINE_PRINTF ("umid=%s", data->umid); - - /* Set memory size. */ - ADD_CMDLINE_PRINTF ("mem=%dM", g->memsize); - - /* vmlinux appears to ignore this, but let's add it anyway. */ - ADD_CMDLINE_PRINTF ("initrd=%s", initrd); - - /* Make sure our appliance init script runs first. */ - ADD_CMDLINE ("init=/init"); - - /* This tells the /init script not to reboot at the end. */ - ADD_CMDLINE ("guestfs_noreboot=1"); - - /* Root filesystem should be mounted read-write (default seems to - * be "ro"). - */ - ADD_CMDLINE ("rw"); - - /* See also guestfs_int_appliance_command_line. */ - if (g->verbose) - ADD_CMDLINE ("guestfs_verbose=1"); - - ADD_CMDLINE ("panic=1"); - - ADD_CMDLINE_PRINTF ("TERM=%s", term ? term : "linux"); - - if (g->selinux) - ADD_CMDLINE ("selinux=1 enforcing=0"); - else - ADD_CMDLINE ("selinux=0"); - - /* XXX This isn't quite right. Multiple append args won't work. */ - if (g->append) - ADD_CMDLINE (g->append); - - /* Add the drives. */ - ITER_DRIVES (g, i, drv) { - if (!drv->overlay) - ADD_CMDLINE_PRINTF ("ubd%zu=%s", i, drv->src.u.path); - else - ADD_CMDLINE_PRINTF ("ubd%zu=%s", i, drv->overlay); - } - - /* Add the ext2 appliance drive (after all the drives). */ - if (has_appliance_drive) { - char drv_name[64] = "ubd"; - guestfs_int_drive_name (g->nr_drives, &drv_name[3]); - - ADD_CMDLINE_PRINTF ("ubd%zu=%s", g->nr_drives, appliance_cow); - ADD_CMDLINE_PRINTF ("root=/dev/%s", drv_name); - } - - /* Create the daemon socket. */ - ADD_CMDLINE_PRINTF ("ssl3=fd:%d", dsv[1]); - ADD_CMDLINE ("guestfs_channel=/dev/ttyS3"); - - /* Add any vmlinux parameters. */ - for (hp = g->hv_params; hp; hp = hp->next) { - ADD_CMDLINE (hp->hv_param); - if (hp->hv_value) - ADD_CMDLINE (hp->hv_value); - } - - /* Finish off the command line. */ - guestfs_int_end_stringsbuf (g, &cmdline); - - r = fork (); - if (r == -1) { - perrorf (g, "fork"); - if (!g->direct_mode) { - close (csv[0]); - close (csv[1]); - } - close (dsv[0]); - close (dsv[1]); - goto cleanup0; - } - - if (r == 0) { /* Child (vmlinux). */ - /* Set up the daemon socket for the child. */ - close (dsv[0]); - set_cloexec_flag (dsv[1], 0); /* so it doesn't close across exec */ - - if (!g->direct_mode) { - /* Set up stdin, stdout, stderr. */ - close (0); - close (1); - close (csv[0]); - - /* We set the FD_CLOEXEC flag on the socket above, but now (in - * the child) it's safe to unset this flag so vmlinux can use the - * socket. - */ - set_cloexec_flag (csv[1], 0); - - /* Stdin. */ - if (dup (csv[1]) == -1) { - dup_failed: - perror ("dup failed"); - _exit (EXIT_FAILURE); - } - /* Stdout. */ - if (dup (csv[1]) == -1) - goto dup_failed; - - /* Send stderr to the pipe as well. */ - close (2); - if (dup (csv[1]) == -1) - goto dup_failed; - - close (csv[1]); - - /* RHBZ#1123007 */ - close_file_descriptors (fd > 2 && fd != dsv[1]); - } - - /* RHBZ#1460338. */ - guestfs_int_unblock_sigterm (); - - /* Dump the command line (after setting up stderr above). */ - if (g->verbose) - print_vmlinux_command_line (g, cmdline.argv); - - /* Put vmlinux in a new process group. */ - if (g->pgroup) - setpgid (0, 0); - - setenv ("LC_ALL", "C", 1); - - execv (g->hv, cmdline.argv); /* Run vmlinux. */ - perror (g->hv); - _exit (EXIT_FAILURE); - } - - /* Parent (library). */ - data->pid = r; - - /* Fork the recovery process off which will kill vmlinux if the - * parent process fails to do so (eg. if the parent segfaults). - */ - data->recoverypid = -1; - if (g->recovery_proc) { - r = fork (); - if (r == 0) { - struct sigaction sa; - pid_t vmlinux_pid = data->pid; - pid_t parent_pid = getppid (); - - /* Remove all signal handlers. See the justification here: - * https://www.redhat.com/archives/libvir-list/2008-August/msg00303.html - * We don't mask signal handlers yet, so this isn't completely - * race-free, but better than not doing it at all. - */ - memset (&sa, 0, sizeof sa); - sa.sa_handler = SIG_DFL; - sa.sa_flags = 0; - sigemptyset (&sa.sa_mask); - for (i = 1; i < NSIG; ++i) - sigaction (i, &sa, NULL); - - /* Close all other file descriptors. This ensures that we don't - * hold open (eg) pipes from the parent process. - */ - close_file_descriptors (1); - - /* RHBZ#1460338 */ - guestfs_int_unblock_sigterm (); - - /* It would be nice to be able to put this in the same process - * group as vmlinux (ie. setpgid (0, vmlinux_pid)). However - * this is not possible because we don't have any guarantee here - * that the vmlinux process has started yet. - */ - if (g->pgroup) - setpgid (0, 0); - - /* Writing to argv is hideously complicated and error prone. See: - * http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/utils/misc/ps_status.c;hb=HEAD - */ - - /* Loop around waiting for one or both of the other processes to - * disappear. It's fair to say this is very hairy. The PIDs that - * we are looking at might be reused by another process. We are - * effectively polling. Is the cure worse than the disease? - */ - for (;;) { - if (kill (vmlinux_pid, 0) == -1) - /* vmlinux's gone away, we aren't needed */ - _exit (EXIT_SUCCESS); - if (kill (parent_pid, 0) == -1) { - /* Parent's gone away, vmlinux still around, so kill vmlinux. */ - kill (data->pid, SIGKILL); - _exit (EXIT_SUCCESS); - } - sleep (2); - } - } - - /* Don't worry, if the fork failed, this will be -1. The recovery - * process isn't essential. - */ - data->recoverypid = r; - } - - if (!g->direct_mode) { - /* Close the other end of the console socketpair. */ - close (csv[1]); - - console_sock = csv[0]; /* stdin of child */ - csv[0] = -1; - } - - daemon_sock = dsv[0]; - close (dsv[1]); - dsv[0] = -1; - - g->state = LAUNCHING; - - /* Wait for vmlinux to start and to connect back to us via - * virtio-serial and send the GUESTFS_LAUNCH_FLAG message. - */ - g->conn - guestfs_int_new_conn_socket_connected (g, daemon_sock, console_sock); - if (!g->conn) - goto cleanup1; - - /* g->conn now owns these sockets. */ - daemon_sock = console_sock = -1; - - /* We now have to wait for vmlinux to start up, the daemon to start - * running, and for it to send the GUESTFS_LAUNCH_FLAG to us. - */ - r = guestfs_int_recv_from_daemon (g, &size, &buf); - - if (r == -1) { - guestfs_int_launch_failed_error (g); - goto cleanup1; - } - - if (size != GUESTFS_LAUNCH_FLAG) { - guestfs_int_launch_failed_error (g); - goto cleanup1; - } - - debug (g, "appliance is up"); - - /* This is possible in some really strange situations, such as - * guestfsd starts up OK but then vmlinux immediately exits. Check - * for it because the caller is probably expecting to be able to - * send commands after this function returns. - */ - if (g->state != READY) { - error (g, _("vmlinux launched and contacted daemon, but state != READY")); - goto cleanup1; - } - - if (has_appliance_drive) - guestfs_int_add_dummy_appliance_drive (g); - - return 0; - - cleanup1: - if (!g->direct_mode && csv[0] >= 0) - close (csv[0]); - if (dsv[0] >= 0) - close (dsv[0]); - if (data->pid > 0) kill (data->pid, SIGKILL); - if (data->recoverypid > 0) kill (data->recoverypid, SIGKILL); - if (data->pid > 0) guestfs_int_waitpid_noerror (data->pid); - if (data->recoverypid > 0) guestfs_int_waitpid_noerror (data->recoverypid); - data->pid = 0; - data->recoverypid = 0; - memset (&g->launch_t, 0, sizeof g->launch_t); - - cleanup0: - if (daemon_sock >= 0) - close (daemon_sock); - if (console_sock >= 0) - close (console_sock); - if (g->conn) { - g->conn->ops->free_connection (g, g->conn); - g->conn = NULL; - } - g->state = CONFIG; - return -1; -} - -/* This is called from the forked subprocess just before vmlinux runs, - * so it can just print the message straight to stderr, where it will - * be picked up and funnelled through the usual appliance event API. - */ -static void -print_vmlinux_command_line (guestfs_h *g, char **argv) -{ - size_t i = 0; - int needs_quote; - - struct timeval tv; - gettimeofday (&tv, NULL); - fprintf (stderr, "[%05" PRIi64 "ms] ", - guestfs_int_timeval_diff (&g->launch_t, &tv)); - - while (argv[i]) { - if (i > 0) fputc (' ', stderr); - - /* Does it need shell quoting? This only deals with simple cases. */ - needs_quote = strcspn (argv[i], " ") != strlen (argv[i]); - - if (needs_quote) fputc ('\'', stderr); - fprintf (stderr, "%s", argv[i]); - if (needs_quote) fputc ('\'', stderr); - i++; - } - - fputc ('\n', stderr); -} - -static int -shutdown_uml (guestfs_h *g, void *datav, int check_for_errors) -{ - struct backend_uml_data *data = datav; - int ret = 0; - int status; - - /* Signal vmlinux to shutdown cleanly, and kill the recovery process. */ - if (data->pid > 0) { - debug (g, "sending SIGTERM to process %d", data->pid); - kill (data->pid, SIGTERM); - } - if (data->recoverypid > 0) kill (data->recoverypid, 9); - - /* Wait for subprocess(es) to exit. */ - if (data->pid > 0) { - if (guestfs_int_waitpid (g, data->pid, &status, "vmlinux") == -1) - ret = -1; - /* Note it's normal for the pre-3.11 vmlinux process to exit with - * status "killed by signal 15" (where 15 == SIGTERM). Post 3.11 - * the exit status can normally be 1. - * - * So don't consider those to be an error. - */ - else if (!(WIFSIGNALED (status) && WTERMSIG (status) == SIGTERM) && - !(WIFEXITED (status) && WEXITSTATUS (status) == 0) && - !(WIFEXITED (status) && WEXITSTATUS (status) == 1)) { - guestfs_int_external_command_failed (g, status, g->hv, NULL); - ret = -1; - } - } - if (data->recoverypid > 0) guestfs_int_waitpid_noerror (data->recoverypid); - - data->pid = data->recoverypid = 0; - - return ret; -} - -static int -get_pid_uml (guestfs_h *g, void *datav) -{ - struct backend_uml_data *data = datav; - - if (data->pid > 0) - return data->pid; - else { - error (g, "get_pid: no vmlinux subprocess"); - return -1; - } -} - -/* UML appears to use a single major, and puts ubda at minor 0 with - * each partition at minors 1-15, ubdb at minor 16, etc. So the - * maximum is 256/16 = 16. However one disk is used by the appliance, - * so it's one less than this. I tested both 15 & 16 disks, and found - * that 15 worked and 16 failed. - */ -static int -max_disks_uml (guestfs_h *g, void *datav) -{ - return 15; -} - -static struct backend_ops backend_uml_ops = { - .data_size = sizeof (struct backend_uml_data), - .create_cow_overlay = create_cow_overlay_uml, - .launch = launch_uml, - .shutdown = shutdown_uml, - .get_pid = get_pid_uml, - .max_disks = max_disks_uml, -}; - -void -guestfs_int_init_uml_backend (void) -{ - guestfs_int_register_backend ("uml", &backend_uml_ops); -} diff --git a/lib/launch.c b/lib/launch.c index 0675fe98be..2970dc8c45 100644 --- a/lib/launch.c +++ b/lib/launch.c @@ -284,14 +284,6 @@ guestfs_impl_config (guestfs_h *g, { struct hv_param *hp; - /* - XXX For qemu this made sense, but not for uml. - if (hv_param[0] != '-') { - error (g, _("parameter must begin with '-' character")); - return -1; - } - */ - /* A bit fascist, but the user will probably break the extra * parameters that we add if they try to set any of these. */ @@ -431,5 +423,4 @@ guestfs_int_force_load_backends[] = { #ifdef HAVE_LIBVIRT_BACKEND guestfs_int_init_libvirt_backend, #endif - guestfs_int_init_uml_backend, }; diff --git a/tests/c-api/test-add-libvirt-dom.c b/tests/c-api/test-add-libvirt-dom.c index 17c349e985..b9669a1bc8 100644 --- a/tests/c-api/test-add-libvirt-dom.c +++ b/tests/c-api/test-add-libvirt-dom.c @@ -73,7 +73,6 @@ main (int argc, char *argv[]) virDomainPtr dom; virErrorPtr err; int r; - char *backend; char cwd[1024]; FILE *fp; char libvirt_uri[sizeof cwd + 64]; @@ -86,13 +85,6 @@ main (int argc, char *argv[]) if (g == NULL) error (EXIT_FAILURE, errno, "guestfs_create"); - backend = guestfs_get_backend (g); - if (STREQ (backend, "uml")) { - free (backend); - error (77, 0, "test skipped because UML backend does not support qcow2"); - } - free (backend); - /* Create the libvirt XML and test images in the current * directory. */ diff --git a/tests/disk-labels/test-disk-labels.pl b/tests/disk-labels/test-disk-labels.pl index 17db5e18b9..7bb94cc119 100755 --- a/tests/disk-labels/test-disk-labels.pl +++ b/tests/disk-labels/test-disk-labels.pl @@ -25,11 +25,6 @@ use Sys::Guestfs; exit 77 if $ENV{SKIP_TEST_DISK_LABELS_PL}; -if (Sys::Guestfs->new()->get_backend() eq "uml") { - print "$0: test skipped because UML backend does not support disk labels\n"; - exit 77 -} - my $g = Sys::Guestfs->new (); # Add two drives. diff --git a/tests/md/test-inspect-fstab.sh b/tests/md/test-inspect-fstab.sh index 3a8d492546..dce340d537 100755 --- a/tests/md/test-inspect-fstab.sh +++ b/tests/md/test-inspect-fstab.sh @@ -24,7 +24,6 @@ set -e $TEST_FUNCTIONS skip_because "device name hints are broken" ;# XXX Fix before 1.38 skip_if_skipped -skip_if_backend uml skip_unless_phony_guest fedora.img canonical="sed -r s,/dev/[abce-ln-z]+d,/dev/sd,g" diff --git a/tests/mountable/test-mountable-inspect.sh b/tests/mountable/test-mountable-inspect.sh index 7bbc6f1384..bd62eab7da 100755 --- a/tests/mountable/test-mountable-inspect.sh +++ b/tests/mountable/test-mountable-inspect.sh @@ -20,7 +20,6 @@ set -e $TEST_FUNCTIONS skip_if_skipped -skip_if_backend uml skip_unless_feature_available btrfs canonical="sed s,/dev/vd,/dev/sd,g" diff --git a/tests/nbd/test-nbd.pl b/tests/nbd/test-nbd.pl index 8d1ac385b1..ef10df7bc2 100755 --- a/tests/nbd/test-nbd.pl +++ b/tests/nbd/test-nbd.pl @@ -27,11 +27,6 @@ END { kill 15, $pid if $pid > 0 }; exit 77 if $ENV{SKIP_TEST_NBD_PL}; -if (Sys::Guestfs->new()->get_backend() eq "uml") { - print "$0: test skipped because UML backend does not support NBD\n"; - exit 77 -} - # Check we have qemu-nbd. if (system ("qemu-nbd --help >/dev/null 2>&1") != 0) { print "$0: test skipped because qemu-nbd program not found\n"; diff --git a/tests/network/test-network.sh b/tests/network/test-network.sh index 55e0f96b1f..144b16319d 100755 --- a/tests/network/test-network.sh +++ b/tests/network/test-network.sh @@ -24,6 +24,5 @@ set -e $TEST_FUNCTIONS skip_if_skipped -skip_if_backend uml guestfish --network -a /dev/null run diff --git a/tests/qemu/qemu-force-tcg.sh b/tests/qemu/qemu-force-tcg.sh index d540102572..f67b2a281a 100755 --- a/tests/qemu/qemu-force-tcg.sh +++ b/tests/qemu/qemu-force-tcg.sh @@ -20,8 +20,6 @@ $TEST_FUNCTIONS skip_if_skipped -# Only applicable if the backend uses qemu. -skip_if_backend uml set -e diff --git a/tests/qemu/qemu-snapshot-isolation-qcow2.sh b/tests/qemu/qemu-snapshot-isolation-qcow2.sh index 7a35f3ac4d..9b6424003d 100755 --- a/tests/qemu/qemu-snapshot-isolation-qcow2.sh +++ b/tests/qemu/qemu-snapshot-isolation-qcow2.sh @@ -24,8 +24,6 @@ set -e $TEST_FUNCTIONS skip_if_skipped -# UML backend doesn't support qcow2 format. -skip_if_backend uml f=isolation-qcow2.img rm -f $f diff --git a/tests/regressions/rhbz1370424.sh b/tests/regressions/rhbz1370424.sh index c28cf3a16b..f12e160edf 100755 --- a/tests/regressions/rhbz1370424.sh +++ b/tests/regressions/rhbz1370424.sh @@ -24,7 +24,6 @@ set -e $TEST_FUNCTIONS skip_if_skipped -skip_if_backend uml guestfish <<EOF -add-domain rhbz1370424 \ diff --git a/tests/regressions/rhbz690819.sh b/tests/regressions/rhbz690819.sh index 0b790862d4..e6f61d00d3 100755 --- a/tests/regressions/rhbz690819.sh +++ b/tests/regressions/rhbz690819.sh @@ -31,8 +31,6 @@ skip_if_arch ppc64 skip_if_arch ppc64le skip_if_arch s390x skip_if_backend libvirt -# UML doesn't support the 'iface' parameter. -skip_if_backend uml rm -f rhbz690819.img diff --git a/tests/regressions/rhbz975797.sh b/tests/regressions/rhbz975797.sh index 04e8d23de3..c676abfa3c 100755 --- a/tests/regressions/rhbz975797.sh +++ b/tests/regressions/rhbz975797.sh @@ -31,8 +31,6 @@ skip_if_arch ppc64 skip_if_arch ppc64le skip_if_arch s390x skip_if_backend libvirt -# UML doesn't support the 'iface' parameter. -skip_if_backend uml rm -f rhbz975797-*.img diff --git a/tests/relative-paths/test-relative-paths.sh b/tests/relative-paths/test-relative-paths.sh index ad739142f0..bb27065436 100755 --- a/tests/relative-paths/test-relative-paths.sh +++ b/tests/relative-paths/test-relative-paths.sh @@ -21,10 +21,6 @@ set -e $TEST_FUNCTIONS skip_if_skipped -# UML doesn't support qcow2. Conceivably there might be a similar -# problem with UML COW images which would require a separate test. -skip_if_backend uml - rm -f backing* rm -f overlay* rm -f link* diff --git a/tests/rsync/test-rsync.sh b/tests/rsync/test-rsync.sh index 02966c0ba1..ab18241698 100755 --- a/tests/rsync/test-rsync.sh +++ b/tests/rsync/test-rsync.sh @@ -44,10 +44,6 @@ case "$backend" in echo "$0: skipping test because host firewall will probably prevent this test from working" exit 77 ;; - uml) - echo "$0: skipping test because networking is not available in the UML backend" - exit 77 - ;; *) echo "$0: don't know how to get IP address of backend $backend" exit 77 diff --git a/tests/test-functions.sh b/tests/test-functions.sh index 8a9c59d0f9..afccfbbdb1 100755 --- a/tests/test-functions.sh +++ b/tests/test-functions.sh @@ -54,7 +54,7 @@ skip_if_skipped () } # Skip if the current libguestfs backend is $1. -# eg. skip_if_backend uml +# eg. skip_if_backend libvirt skip_if_backend () { local b="$(guestfish get-backend)" -- 2.35.1
These APIs were an experimental feature for adding 9pfs disks to the libguestfs appliance. It was never possible to use this without hacking the qemu command line of the appliance to add such drives by hand. Note that for ABI reasons these APIs are not actually removed, they have been changed so that they always return an error. These APIs were actually hard-removed from all versions of RHEL. See-also: https://bugzilla.redhat.com/921710 --- daemon/9p.c | 153 +-------------------------- generator/actions_core.ml | 21 ---- generator/actions_core_deprecated.ml | 17 +++ tests/9p/test-9p.sh | 63 ----------- tests/Makefile.am | 3 - 5 files changed, 21 insertions(+), 236 deletions(-) diff --git a/daemon/9p.c b/daemon/9p.c index 9a3a5cfdf2..a0bc66539e 100644 --- a/daemon/9p.c +++ b/daemon/9p.c @@ -18,165 +18,20 @@ #include <config.h> -#include <stdio.h> -#include <stdlib.h> -#include <string.h> -#include <unistd.h> -#include <limits.h> -#include <errno.h> -#include <sys/types.h> -#include <sys/stat.h> -#include <dirent.h> -#include <fcntl.h> - -#include "ignore-value.h" - #include "daemon.h" #include "actions.h" -#define BUS_PATH "/sys/bus/virtio/drivers/9pnet_virtio" - -static void -modprobe_9pnet_virtio (void) -{ - /* Required with Linux 5.6 and maybe earlier kernels. For unclear - * reasons the module is not an automatic dependency of the 9p - * module so doesn't get loaded automatically. - */ - ignore_value (command (NULL, NULL, "modprobe", "9pnet_virtio", NULL)); -} - -/* https://bugzilla.redhat.com/show_bug.cgi?id=714981#c1 */ char ** do_list_9p (void) { - CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (r); - DIR *dir; - - modprobe_9pnet_virtio (); - - dir = opendir (BUS_PATH); - if (!dir) { - perror ("opendir: " BUS_PATH); - if (errno != ENOENT) { - reply_with_perror ("opendir: " BUS_PATH); - return NULL; - } - - /* If this directory doesn't exist, it probably means that - * the virtio driver isn't loaded. Don't return an error - * in this case, but return an empty list. - */ - if (end_stringsbuf (&r) == -1) - return NULL; - - return take_stringsbuf (&r); - } - - while (1) { - struct dirent *d; - - errno = 0; - d = readdir (dir); - if (d == NULL) break; - - if (STRPREFIX (d->d_name, "virtio")) { - CLEANUP_FREE char *mount_tag_path = NULL; - if (asprintf (&mount_tag_path, BUS_PATH "/%s/mount_tag", - d->d_name) == -1) { - reply_with_perror ("asprintf"); - closedir (dir); - return NULL; - } - - /* A bit unclear, but it looks like the virtio transport allows - * the mount tag length to be unlimited (or up to 65536 bytes). - * See: linux/include/linux/virtio_9p.h - */ - CLEANUP_FREE char *mount_tag = read_whole_file (mount_tag_path, NULL); - if (mount_tag == 0) - continue; - - if (add_string (&r, mount_tag) == -1) { - closedir (dir); - return NULL; - } - } - } - - /* Check readdir didn't fail */ - if (errno != 0) { - reply_with_perror ("readdir: " BUS_PATH); - closedir (dir); - return NULL; - } - - /* Close the directory handle */ - if (closedir (dir) == -1) { - reply_with_perror ("closedir: " BUS_PATH); - return NULL; - } - - /* Sort the tags. */ - if (r.size > 0) - sort_strings (r.argv, r.size); - - /* NULL terminate the list */ - if (end_stringsbuf (&r) == -1) - return NULL; - - return take_stringsbuf (&r); + reply_with_perror ("9p support was removed in libguestfs 1.48"); + return NULL; } /* Takes optional arguments, consult optargs_bitmask. */ int do_mount_9p (const char *mount_tag, const char *mountpoint, const char *options) { - CLEANUP_FREE char *mp = NULL, *opts = NULL, *err = NULL; - struct stat statbuf; - int r; - - ABS_PATH (mountpoint, 0, return -1); - - mp = sysroot_path (mountpoint); - if (!mp) { - reply_with_perror ("malloc"); - return -1; - } - - /* Check the mountpoint exists and is a directory. */ - if (stat (mp, &statbuf) == -1) { - reply_with_perror ("%s", mountpoint); - return -1; - } - if (!S_ISDIR (statbuf.st_mode)) { - reply_with_perror ("%s: mount point is not a directory", mountpoint); - return -1; - } - - /* Add trans=virtio to the options. */ - if ((optargs_bitmask & GUESTFS_MOUNT_9P_OPTIONS_BITMASK) && - STRNEQ (options, "")) { - if (asprintf (&opts, "trans=virtio,%s", options) == -1) { - reply_with_perror ("asprintf"); - return -1; - } - } - else { - opts = strdup ("trans=virtio"); - if (opts == NULL) { - reply_with_perror ("strdup"); - return -1; - } - } - - modprobe_9pnet_virtio (); - r = command (NULL, &err, - "mount", "-o", opts, "-t", "9p", mount_tag, mp, NULL); - if (r == -1) { - reply_with_error ("%s on %s: %s", mount_tag, mountpoint, err); - return -1; - } - - return 0; + reply_with_perror ("9p support was removed in libguestfs 1.48"); + return -1; } diff --git a/generator/actions_core.ml b/generator/actions_core.ml index 8884f35807..d82ddb70eb 100644 --- a/generator/actions_core.ml +++ b/generator/actions_core.ml @@ -6157,27 +6157,6 @@ This returns true iff the device exists and contains all zero bytes. Note that for large devices this can take a long time to run." }; - { defaults with - name = "list_9p"; added = (1, 11, 12); - style = RStringList (RPlainString, "mounttags"), [], []; - shortdesc = "list 9p filesystems"; - longdesc = "\ -List all 9p filesystems attached to the guest. A list of -mount tags is returned." }; - - { defaults with - name = "mount_9p"; added = (1, 11, 12); - style = RErr, [String (PlainString, "mounttag"); String (PlainString, "mountpoint")], [OString "options"]; - camel_name = "Mount9P"; - shortdesc = "mount 9p filesystem"; - longdesc = "\ -Mount the virtio-9p filesystem with the tag C<mounttag> on the -directory C<mountpoint>. - -If required, C<trans=virtio> will be automatically added to the options. -Any other options required can be passed in the optional C<options> -parameter." }; - { defaults with name = "list_dm_devices"; added = (1, 11, 15); style = RStringList (RDevice, "devices"), [], []; diff --git a/generator/actions_core_deprecated.ml b/generator/actions_core_deprecated.ml index 27b358f17b..60d11bfee5 100644 --- a/generator/actions_core_deprecated.ml +++ b/generator/actions_core_deprecated.ml @@ -899,4 +899,21 @@ C<device> parameter must be the name of the LUKS mapping device (ie. F</dev/mapper/mapname>) and I<not> the name of the underlying block device." }; + { defaults with + name = "list_9p"; added = (1, 11, 12); + style = RStringList (RPlainString, "mounttags"), [], []; + shortdesc = "list 9p filesystems"; + deprecated_by = Deprecated_no_replacement; + longdesc = "\ +This call does nothing and returns an error." }; + + { defaults with + name = "mount_9p"; added = (1, 11, 12); + style = RErr, [String (PlainString, "mounttag"); String (PlainString, "mountpoint")], [OString "options"]; + camel_name = "Mount9P"; + deprecated_by = Deprecated_no_replacement; + shortdesc = "mount 9p filesystem"; + longdesc = "\ +This call does nothing and returns an error." }; + ] diff --git a/tests/9p/test-9p.sh b/tests/9p/test-9p.sh deleted file mode 100755 index 4fd5de7fda..0000000000 --- a/tests/9p/test-9p.sh +++ /dev/null @@ -1,63 +0,0 @@ -#!/bin/bash - -# libguestfs -# 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 9p filesystems for Avi. As there is no way to add a 9p disk to -# libguestfs, we have to fake it using 'config'. - -set -e - -$TEST_FUNCTIONS -skip_if_skipped -skip_unless_backend direct - -# The name of the virtio-9p device is different on some architectures. -case "$(uname -m)" in - arm*) - virtio_9p=virtio-9p-device - ;; - s390*) - virtio_9p=virtio-9p-ccw - ;; - *) - virtio_9p=virtio-9p-pci - ;; -esac - -rm -f test-9p.img test-9p.out - -guestfish <<EOF -# This dummy disk is not actually used, but libguestfs requires one. -sparse test-9p.img 1M - -config -device '$virtio_9p,fsdev=test9p,mount_tag=test9p' -config -fsdev 'local,id=test9p,path=${abs_srcdir}/9p,security_model=passthrough' - -run - -mount-9p test9p / -ls / | grep 'test-9p.sh\$' > test-9p.out - -EOF - -if [ "$(cat test-9p.out)" != "test-9p.sh" ]; then - echo "$0: unexpected output from listing 9p directory:" - cat test-9p.out - exit 1 -fi - -rm test-9p.img test-9p.out diff --git a/tests/Makefile.am b/tests/Makefile.am index 707a840225..c65fc2669a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -43,9 +43,6 @@ check-slow: check-valgrind: $(MAKE) VG="@VG@" check -TESTS += 9p/test-9p.sh -EXTRA_DIST += 9p/test-9p.sh - SLOW_TESTS += bigdirs/test-big-dirs.pl EXTRA_DIST += bigdirs/test-big-dirs.pl -- 2.35.1
Richard W.M. Jones
2022-Mar-08 15:57 UTC
[Libguestfs] [PATCH 4/4] 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(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 7322bfa5d7..bbd49f9ea1 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 193a5ffb91..0000000000 --- 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 30147ef493..ca18afbad3 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 15b1247b00..deb05dd97e 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 d82ddb70eb..66c46d97e1 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 @@ -1127,25 +1121,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")], []; @@ -7817,30 +7792,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"]; @@ -8063,9 +8014,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 60d11bfee5..00dde3d2a5 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 74b95baf78..b20672ff09 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 46af66db45..fd95308d2d 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 09ab3ea9e7..5bb00bc101 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 bf4238ce93..b04c28d627 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 4f038f4f04..5c91822fb5 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 cc714c02ef..44764f3cc0 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 c65fc2669a..f864f82b2c 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 18ff7c47f9..0000000000 --- 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 3e7cf72ed0..0000000000 --- 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. - at 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 142f44521e..0000000000 --- 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 -- 2.35.1
Laszlo Ersek
2022-Mar-09 08:49 UTC
[Libguestfs] [PATCH 0/4] Remove some experimental features from libguestfs 1.48
On 03/08/22 16:57, Richard W.M. Jones wrote:> I think we should take the opportunity of libguestfs 1.48 to remove > some features which are experimental, inadvisable or both. This will > also reduce the delta between Fedora and RHEL. > > The commit messages outline why in my opinion it's right to remove > each feature. > > This doesn't break API/ABI (as in, compiling and linking will still > work). Of course if you were using one of these features then that > sucks because the APIs will return errors instead of doing what you > expected. None of our existing tools use any of these features. > > (I wouldn't necessarily bother with detailed reviews of each patch -- > all the tests pass and if there's something I've overlooked we can > remove that later. It's more about do we agree on this as a general > direction of travel.)I've read the commit messages; everything makes sense to me. Nice line count savings, too! series Acked-by: Laszlo Ersek <lersek at redhat.com> Thanks Laszlo