From 037a603c2d5cf9d2d5f8157116dbf14945277dc2 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Mon, 2 Dec 2024 15:22:43 +0000 Subject: [PATCH] v2v: Implement --parallel=N for parallel disk copies When set, run up to N copies of nbdcopy in parallel. This only applies for guests with multiple disks. The default, as for all older versions of virt-v2v, is to copy disks one at a time. (cherry picked from commit fd1148f79581b148525eb12154aef7603ccf0baa) --- docs/virt-v2v.pod | 13 +++++++ lib/utils.ml | 6 ++++ lib/utils.mli | 4 +++ tests/Makefile.am | 2 ++ tests/test-v2v-i-disk-parallel.sh | 54 +++++++++++++++++++++++++++++ v2v/v2v.ml | 56 +++++++++++++++++++++++++------ 6 files changed, 125 insertions(+), 10 deletions(-) create mode 100755 tests/test-v2v-i-disk-parallel.sh diff --git a/docs/virt-v2v.pod b/docs/virt-v2v.pod index 74581463..216e617d 100644 --- a/docs/virt-v2v.pod +++ b/docs/virt-v2v.pod @@ -749,6 +749,19 @@ C. You will get an error if virt-v2v is unable to mount/write to the Export Storage Domain. +=item B<--parallel> N + +Enable parallel copying if the guest has multiple disks. I is the +maximum number of parallel L instances to run. + +The default is to run at most one instance of nbdcopy +(ie. I<--parallel=1>). All versions of virt-v2v E 2.7.2 also did +disk copies one at a time. + +Within each guest disk, nbdcopy tries to copy in parallel if the +underlying endpoints support that. This is not affected by this +command line option. See the L manual page for details. + =item B<--print-source> Print information about the source guest and stop. This option is diff --git a/lib/utils.ml b/lib/utils.ml index c4cfd89b..f2da9e80 100644 --- a/lib/utils.ml +++ b/lib/utils.ml @@ -29,6 +29,12 @@ let large_tmpdir = try Sys.getenv "VIRT_V2V_TMPDIR" with Not_found -> (open_guestfs ())#get_cachedir () +let string_of_process_status = function + | Unix.WEXITED 0 -> s_"success" + | WEXITED i -> sprintf (f_"exited with non-zero error code %d") i + | WSIGNALED i -> sprintf (f_"signalled by signal %d") i + | WSTOPPED i -> sprintf (f_"stopped by signal %d") i + (* Is SELinux enabled and enforcing on the host? *) let have_selinux = 0 = Sys.command "getenforce 2>/dev/null | grep -isq Enforcing" diff --git a/lib/utils.mli b/lib/utils.mli index afe61a4e..e7ee13d1 100644 --- a/lib/utils.mli +++ b/lib/utils.mli @@ -23,6 +23,10 @@ val large_tmpdir : string such as overlays in this directory. Small temporary files can use the default behaviour eg. of {!Filename.temp_file} *) +val string_of_process_status : Unix.process_status -> string +(** Convert a process status (such as returned by {!Unix.wait}) into + a printable string. *) + val have_selinux : bool (** True if SELinux is enabled and enforcing on the host. *) diff --git a/tests/Makefile.am b/tests/Makefile.am index 8a710b99..fa5bb4f1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -76,6 +76,7 @@ TESTS = \ test-v2v-cdrom.sh \ test-v2v-floppy.sh \ test-v2v-i-disk.sh \ + test-v2v-i-disk-parallel.sh \ test-v2v-i-ova.sh \ test-v2v-inspector.sh \ test-v2v-mac.sh \ @@ -189,6 +190,7 @@ EXTRA_DIST += \ test-v2v-floppy.expected \ test-v2v-floppy.sh \ test-v2v-i-disk.sh \ + test-v2v-i-disk-parallel.sh \ test-v2v-i-ova-as-root.ovf \ test-v2v-i-ova-as-root.sh \ test-v2v-i-ova-bad-sha1.sh \ diff --git a/tests/test-v2v-i-disk-parallel.sh b/tests/test-v2v-i-disk-parallel.sh new file mode 100755 index 00000000..a6470fdd --- /dev/null +++ b/tests/test-v2v-i-disk-parallel.sh @@ -0,0 +1,54 @@ +#!/bin/bash - +# libguestfs virt-v2v test script +# Copyright (C) 2014-2024 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 --parallel option. + +set -e + +source ./functions.sh +set -e +set -x + +skip_if_skipped +windows=../test-data/phony-guests/windows.img +requires test -f $windows + +export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools" + +d=test-v2v-i-disk-parallel.d +rm -rf $d +cleanup_fn rm -rf $d +mkdir $d + +truncate -s $((100*1024*1024)) $d/disk-2.img $d/disk-3.img $d/disk-4.img + +$VG virt-v2v --debug-gc \ + --parallel=2 \ + -i disk \ + $windows \ + $d/disk-2.img \ + $d/disk-3.img \ + $d/disk-4.img \ + -o local -os $d + +# Test the libvirt XML metadata and output disks were created. +test -f $d/windows.xml +test -f $d/windows-sda +test -f $d/windows-sdb +test -f $d/windows-sdc +test -f $d/windows-sdd diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 2fdaf40b..68502884 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -79,6 +79,7 @@ let rec main () = ) in + let parallel = ref 1 in let network_map = Networks.create () in let static_ips = ref [] in let rec add_network str = @@ -263,6 +264,8 @@ let rec main () = s_"Set output storage location"; [ L"password-file" ], Getopt.String ("filename", set_string_option_once "-ip" input_password), s_"Same as ‘-ip filename’"; + [ L"parallel" ], Getopt.Set_int ("N", parallel), + s_"Run up to N instances of nbdcopy in parallel"; [ L"print-source" ], Getopt.Set print_source, s_"Print source and stop"; [ L"root" ], Getopt.String ("ask|... ", set_root_choice), @@ -365,6 +368,7 @@ read the man page virt-v2v(1). | `Preallocated -> Types.Preallocated in let output_mode = !output_mode in let output_name = !output_name in + let parallel = !parallel in let print_source = !print_source in let root_choice = !root_choice in let static_ips = !static_ips in @@ -386,6 +390,7 @@ read the man page virt-v2v(1). pr "mac-option\n"; pr "bandwidth-option\n"; pr "mac-ip-option\n"; + pr "parallel-option\n"; pr "customize-ops\n"; pr "input:disk\n"; pr "input:libvirt\n"; @@ -583,12 +588,15 @@ read the man page virt-v2v(1). else List.rev acc in - let disks = loop [] 0 in - let nr_disks = List.length disks in + let disks = ref (loop [] 0) in + let nr_disks = List.length !disks in (* Copy the disks. *) - List.iter ( - fun (i, input_socket, output_socket) -> + let nbdcopy_pids = ref [] in + let rec copy_loop () = + if List.length !nbdcopy_pids < parallel && !disks <> [] then ( + (* Schedule another nbdcopy process. *) + let i, input_socket, output_socket = List.pop_front disks in message (f_"Copying disk %d/%d") (i+1) nr_disks; let request_size = Output_module.request_size @@ -608,8 +616,33 @@ read the man page virt-v2v(1). flush Stdlib.stderr ); - nbdcopy ?request_size output_alloc input_uri output_uri - ) disks; + let pid = nbdcopy ?request_size output_alloc input_uri output_uri in + List.push_front pid nbdcopy_pids; + + copy_loop (); + ) + else if !nbdcopy_pids <> [] then ( + (* Wait for one nbdcopy instance to exit. *) + let pid, status = wait () in + (* If this internal error turns up in real world scenarios then + * we may need to change the [wait] above so it only waits on + * the nbdcopy PIDs. + *) + if not (List.mem pid !nbdcopy_pids) then + error (f_"internal error: wait returned unexpected \ + process ID %d status \"%s\"") + pid (string_of_process_status status); + nbdcopy_pids := List.filter ((<>) pid) !nbdcopy_pids; + (match status with + | WEXITED 0 -> copy_loop () + | WEXITED _ | WSIGNALED _ | WSTOPPED _ -> + error "nbdcopy %s" (string_of_process_status status) + ); + ) + in + copy_loop (); + assert (!disks == []); + assert (!nbdcopy_pids == []); (* End of copying phase. *) unlink (v2vdir // "copy"); @@ -647,6 +680,7 @@ and check_host_free_space () = \"Minimum free space check in the host\".") large_tmpdir (human_size free_space) +(* Start nbdcopy as a background process, returning the PID. *) and nbdcopy ?request_size output_alloc input_uri output_uri = (* XXX It's possible that some output modes know whether * --target-is-zero which would be a useful optimization. @@ -674,10 +708,12 @@ and nbdcopy ?request_size output_alloc input_uri output_uri = if not (quiet ()) then List.push_back cmd "--progress"; if output_alloc = Types.Preallocated then List.push_back cmd "--allocated"; - let cmd = !cmd in - - if run_command cmd <> 0 then - error (f_"nbdcopy command failed, see earlier error messages") + let args = Array.of_list !cmd in + match fork () with + | 0 -> + (* Child process (nbdcopy). *) + execvp "nbdcopy" args + | pid -> pid (* Run nbdinfo on a URI and collect the information. However don't * fail if nbdinfo is not installed since this is just used for debugging.