From 3a13726152f20a1c264a20c5f8dafeed8e78873f Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 15 Jul 2022 15:48:52 +0100 Subject: [PATCH] Fix remapping of nvme devices in /boot/grub2/device.map resolves: rhbz#2101665 Improve documentation of vmx+ssh and -ip option resolves: rhbz#1854275 Remove legacy crypto advice and replace with targeted mechanism resolves: rhbz#2062360 Fix race condition when unmounting in -o rhv mode (1953286#c26) --- ...linux-complete-the-remapping-of-NVMe.patch | 83 ++++++++ ...p-limitations-language-from-input-vm.patch | 52 +++++ ...e-enable-LEGACY-crypto-advice-with-t.patch | 85 ++++++++ ...renamed-function-On_exit.rmdir-On_ex.patch | 175 +++++++++++++++++ ...e-temporary-NFS-mountpoint-as-late-a.patch | 174 +++++++++++++++++ ...tput-modes-to-wait-on-the-local-NBD-.patch | 182 ++++++++++++++++++ ...he-NBD-server-to-exit-to-avoid-a-rac.patch | 36 ++++ virt-v2v.spec | 21 +- 8 files changed, 805 insertions(+), 3 deletions(-) create mode 100644 0023-convert-convert_linux-complete-the-remapping-of-NVMe.patch create mode 100644 0024-input-xen-sync-ip-limitations-language-from-input-vm.patch create mode 100644 0025-input-xen-replace-enable-LEGACY-crypto-advice-with-t.patch create mode 100644 0026-common-Adapt-to-renamed-function-On_exit.rmdir-On_ex.patch create mode 100644 0027-o-rhv-Unmount-the-temporary-NFS-mountpoint-as-late-a.patch create mode 100644 0028-output-Permit-output-modes-to-wait-on-the-local-NBD-.patch create mode 100644 0029-o-rhv-Wait-for-the-NBD-server-to-exit-to-avoid-a-rac.patch diff --git a/0023-convert-convert_linux-complete-the-remapping-of-NVMe.patch b/0023-convert-convert_linux-complete-the-remapping-of-NVMe.patch new file mode 100644 index 0000000..e105eb8 --- /dev/null +++ b/0023-convert-convert_linux-complete-the-remapping-of-NVMe.patch @@ -0,0 +1,83 @@ +From ba2963bc57c8c8a3d6f7cc2fd274c9ebd4ddb7d8 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Wed, 6 Jul 2022 12:32:15 +0200 +Subject: [PATCH] convert/convert_linux: complete the remapping of NVMe devices + +In commit 75872bf282d7 ("input: -i vmx: Add support for NVMe devices", +2022-04-08), we missed that pathnames such as + + /dev/nvme0n1[p1] + +would not match our "rex_device_cciss" and "rex_device" regular +expressions. + +As a consequence, we don't remap such pathnames now in the boot config +files with Augeas. + +Add a new regex and associated mapping logic for this kind of pathname. + +Notes: + +(1) "rex_device_cciss" could be extended internally with an alternative +pattern: + + ^/dev/(cciss/c\\d+d\\d+|nvme\\d+n1)(?:p(\\d+))?$ + ^^^^^^^^^^^ + +but Rich suggested we should add a separate, complete regexp for +maintainability. + +(2) Even with a separate regexp, we could reuse the existent CCISS pattern +handler: + + if PCRE.matches rex_device_cciss value || + PCRE.matches rex_device_nvme value then ( + let device = PCRE.sub 1 + and part = try PCRE.sub 2 with Not_found -> "" in + "/dev/" ^ replace device ^ part + ) + +Namely, although "PCRE.matches" creates/updates global state, and +"PCRE.sub" reads that state, the "||" operator in OCaml has short-circuit +behavior, and both regexps have the same structure. + +But, using the same maintainability argument, let's keep the handler logic +for NVMe detached. + +Fixes: 75872bf282d7f2322110caca70963717b43806b1 +Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2101665 +Signed-off-by: Laszlo Ersek +Message-Id: <20220706103215.5607-1-lersek@redhat.com> +Reviewed-by: Richard W.M. Jones +(cherry picked from commit 4368b94ee1724c16aa35c0ee42ce4c51ce037b5a) +--- + convert/convert_linux.ml | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml +index 59d143bd..a66ff1e4 100644 +--- a/convert/convert_linux.ml ++++ b/convert/convert_linux.ml +@@ -1199,6 +1199,7 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ = + (* Map device names for each entry. *) + let rex_resume = PCRE.compile "^resume=(/dev/[-a-z\\d/_]+)(.*)$" + and rex_device_cciss = PCRE.compile "^/dev/(cciss/c\\d+d\\d+)(?:p(\\d+))?$" ++ and rex_device_nvme = PCRE.compile "^/dev/(nvme\\d+n1)(?:p(\\d+))?$" + and rex_device = PCRE.compile "^/dev/([a-z]+)(\\d*)?$" in + + let rec replace_if_device path value = +@@ -1221,6 +1222,11 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ = + and part = try PCRE.sub 2 with Not_found -> "" in + "/dev/" ^ replace device ^ part + ) ++ else if PCRE.matches rex_device_nvme value then ( ++ let device = PCRE.sub 1 ++ and part = try PCRE.sub 2 with Not_found -> "" in ++ "/dev/" ^ replace device ^ part ++ ) + else if PCRE.matches rex_device value then ( + let device = PCRE.sub 1 + and part = try PCRE.sub 2 with Not_found -> "" in +-- +2.31.1 + diff --git a/0024-input-xen-sync-ip-limitations-language-from-input-vm.patch b/0024-input-xen-sync-ip-limitations-language-from-input-vm.patch new file mode 100644 index 0000000..0491770 --- /dev/null +++ b/0024-input-xen-sync-ip-limitations-language-from-input-vm.patch @@ -0,0 +1,52 @@ +From c34fe9a52abdde05cb31c5bd2c99237652e1b0dc Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Mon, 11 Jul 2022 09:01:56 +0200 +Subject: [PATCH] input-xen: sync "-ip" limitations language from input-vmware + manual + +My analysis in +was partially wrong; I had missed that for the xen+ssh transport, the +client-side libvirt library launches a naked "ssh" utility, underneath +"Libvirt.Connect.connect_auth": + + setup [input/input_xen_ssh.ml] + Libvirt.Connect.connect_auth + no effect of "-ip" + Nbdkit_ssh.create_ssh [input/nbdkit_ssh.ml] + starts nbdkit with the ssh + plugin honoring "-ip" + +Which requires a password just the same, and ignores "-ip" just the same. + +Recommend the ssh agent in the docs. + +Fixes: 46298c6514710013c59828b4933f0b3b1a354566 +Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1854275 +Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2062360 +Signed-off-by: Laszlo Ersek +Message-Id: <20220711070157.5399-2-lersek@redhat.com> +Reviewed-by: Richard W.M. Jones +(cherry picked from commit ae067a9ce0eb5631940a8cc5dcc5ee056903276b) +--- + docs/virt-v2v-input-xen.pod | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/docs/virt-v2v-input-xen.pod b/docs/virt-v2v-input-xen.pod +index ad5772de..80ad94f7 100644 +--- a/docs/virt-v2v-input-xen.pod ++++ b/docs/virt-v2v-input-xen.pod +@@ -32,6 +32,11 @@ server to the Xen host. For example: + $ ssh root@xen.example.com + [ logs straight into the shell, no password is requested ] + ++Note that support for non-interactive authentication via the I<-ip> ++option is incomplete. Some operations remain that still require the ++user to enter the password manually. Therefore ssh-agent is recommended ++over the I<-ip> option. See L. ++ + With some modern ssh implementations, legacy crypto policies required + to interoperate with RHEL 5 sshd are disabled. To enable them you may + need to run this command on the conversion server (ie. ssh client), +-- +2.31.1 + diff --git a/0025-input-xen-replace-enable-LEGACY-crypto-advice-with-t.patch b/0025-input-xen-replace-enable-LEGACY-crypto-advice-with-t.patch new file mode 100644 index 0000000..d6ecff8 --- /dev/null +++ b/0025-input-xen-replace-enable-LEGACY-crypto-advice-with-t.patch @@ -0,0 +1,85 @@ +From 3f7f730ac9cbf38267839ffcebd6b6fd721123c5 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Mon, 11 Jul 2022 09:01:57 +0200 +Subject: [PATCH] input-xen: replace "enable LEGACY crypto" advice with + targeted ssh options + +- "KexAlgorithms": the Fedora 35 ssh binary, using the DEFAULT + crypto-policy, cannot log in to RHEL5 sshd without relaxing + "KexAlgorithms". The server offers three algorithms: + "diffie-hellman-group-exchange-sha1", "diffie-hellman-group14-sha1", + "diffie-hellman-group1-sha1"; and according to RFC 9142, + "diffie-hellman-group14-sha1" is the least deprecated from those. (The + RFC marks it as MAY be implemented, and marks the other two as SHOULD + NOT be implemented.) Recommend "diffie-hellman-group14-sha1". + +- "MACs": the Fedora 35 ssh binary, using the FUTURE crypto-policy, cannot + log in to RHEL5 sshd without relaxing "MACs". The server offers + "hmac-md5", "hmac-sha1", "hmac-ripemd160", "hmac-ripemd160@openssh.com", + "hmac-sha1-96", "hmac-md5-96". After eliminating the MD5-based algos + (MD5 is considered completely broken), and the one based on truncated + SHA1, we're left with "hmac-sha1", "hmac-ripemd160", and + "hmac-ripemd160@openssh.com". RIPEMD-160 is generally trusted, but it is + compiled out of the Fedora 35 "ssh" client binary. Therefore only + "hmac-sha1" remains. + +- "HostKeyAlgorithms", "PubkeyAcceptedAlgorithms": these options control + the usage of public key algorithms, for authenticating the server to the + client, and vice versa, respectively. RHEL5 sshd only supports "ssh-rsa" + and "ssh-dss", and from those, "ssh-rsa" is more commonly used (for + example, "ssh-keygen" defaults to creating "ssh-rsa" keys). Recommend + "ssh-rsa". + +- "PubkeyAcceptedKeyTypes": this is the old ("legacy") name for + "PubkeyAcceptedAlgorithms". As of this writing, the latest upstream + release of libssh (also packaged in Fedora 35 -- libssh-0.9.6-1.fc35) + does not recognize the new "PubkeyAcceptedAlgorithms" option name, only + the original "PubkeyAcceptedKeyTypes". openssh-8.7p1-3.fc35 recognizes + both option variants. Include "PubkeyAcceptedKeyTypes" in the + recommendation along with "PubkeyAcceptedAlgorithms", for backward and + forward compatbility. + +Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2062360 +Signed-off-by: Laszlo Ersek +Message-Id: <20220711070157.5399-3-lersek@redhat.com> +Reviewed-by: Richard W.M. Jones +(cherry picked from commit af4a0454cdd21bb5e86f2dbfaa153e83afca3988) +--- + docs/virt-v2v-input-xen.pod | 20 +++++++++++++++----- + 1 file changed, 15 insertions(+), 5 deletions(-) + +diff --git a/docs/virt-v2v-input-xen.pod b/docs/virt-v2v-input-xen.pod +index 80ad94f7..1775fc31 100644 +--- a/docs/virt-v2v-input-xen.pod ++++ b/docs/virt-v2v-input-xen.pod +@@ -37,12 +37,22 @@ option is incomplete. Some operations remain that still require the + user to enter the password manually. Therefore ssh-agent is recommended + over the I<-ip> option. See L. + +-With some modern ssh implementations, legacy crypto policies required +-to interoperate with RHEL 5 sshd are disabled. To enable them you may +-need to run this command on the conversion server (ie. ssh client), +-but read L first: ++With some modern ssh implementations, legacy crypto algorithms required ++to interoperate with RHEL 5 sshd are disabled. To enable them, you may ++need to add the following C stanza to your F<~/.ssh/config>: + +- # update-crypto-policies --set LEGACY ++ Host xen.example.com ++ KexAlgorithms +diffie-hellman-group14-sha1 ++ MACs +hmac-sha1 ++ HostKeyAlgorithms +ssh-rsa ++ PubkeyAcceptedKeyTypes +ssh-rsa ++ PubkeyAcceptedAlgorithms +ssh-rsa ++ ++(C and C have ++identical meaning; the former is the old option name, the latter is the ++new one. Virt-v2v uses both C and C when converting a guest ++from Xen, and on some operating systems, C and C may not ++both accept the same option variant.) + + =head2 Test libvirt connection to remote Xen host + +-- +2.31.1 + diff --git a/0026-common-Adapt-to-renamed-function-On_exit.rmdir-On_ex.patch b/0026-common-Adapt-to-renamed-function-On_exit.rmdir-On_ex.patch new file mode 100644 index 0000000..2787a5b --- /dev/null +++ b/0026-common-Adapt-to-renamed-function-On_exit.rmdir-On_ex.patch @@ -0,0 +1,175 @@ +From ea881513e9c15b0a816d3ba4afe471ff2f591a03 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Thu, 14 Jul 2022 12:44:27 +0100 +Subject: [PATCH] common: Adapt to renamed function On_exit.rmdir -> + On_exit.rm_rf + +This function was renamed to make it clearer what it does (and that +it's potentially dangerous). The functionality is unchanged. + +Reviewed-by: Laszlo Ersek +(cherry picked from commit 2eb6441264deb0411d36dabaf8fb2da9f07c8439) +--- + common | 2 +- + input/OVA.ml | 2 +- + input/parse_domain_from_vmx.ml | 2 +- + lib/nbdkit.ml | 2 +- + lib/qemuNBD.ml | 2 +- + lib/utils.ml | 2 +- + output/python_script.ml | 2 +- + 7 files changed, 7 insertions(+), 7 deletions(-) + +Submodule common af6cb55b..fd964c1b: +diff --git a/common/mlcustomize/guest_packages.ml b/common/mlcustomize/guest_packages.ml +index 4c3c34ed..7c29a2ab 100644 +--- a/common/mlcustomize/guest_packages.ml ++++ b/common/mlcustomize/guest_packages.ml +@@ -73,9 +73,9 @@ let install_command packages package_management = + | "zypper" -> sprintf "zypper -n in -l %s" quoted_args + + | "unknown" -> +- error_unknown_package_manager (s_"--install") ++ error_unknown_package_manager "--install" + | pm -> +- error_unimplemented_package_manager (s_"--install") pm ++ error_unimplemented_package_manager "--install" pm + + let update_command package_management = + match package_management with +@@ -103,9 +103,9 @@ let update_command package_management = + | "zypper" -> "zypper -n update -l" + + | "unknown" -> +- error_unknown_package_manager (s_"--update") ++ error_unknown_package_manager "--update" + | pm -> +- error_unimplemented_package_manager (s_"--update") pm ++ error_unimplemented_package_manager "--update" pm + + let uninstall_command packages package_management = + let quoted_args = String.concat " " (List.map quote packages) in +@@ -127,6 +127,6 @@ let uninstall_command packages package_management = + | "zypper" -> sprintf "zypper -n rm %s" quoted_args + + | "unknown" -> +- error_unknown_package_manager (s_"--uninstall") ++ error_unknown_package_manager "--uninstall" + | pm -> +- error_unimplemented_package_manager (s_"--uninstall") pm ++ error_unimplemented_package_manager "--uninstall" pm +diff --git a/common/mltools/on_exit.ml b/common/mltools/on_exit.ml +index 53ccb68a..cae12e73 100644 +--- a/common/mltools/on_exit.ml ++++ b/common/mltools/on_exit.ml +@@ -52,7 +52,7 @@ let do_actions () = + List.iter (do_action (fun file -> Unix.unlink file)) !files; + List.iter (do_action ( + fun dir -> +- let cmd = sprintf "rm -rf %s" (Filename.quote dir) in ++ let cmd = sprintf "rm -rf -- %s" (Filename.quote dir) in + ignore (Tools_utils.shell_command cmd) + ) + ) !rmdirs; +@@ -102,7 +102,7 @@ let unlink filename = + register (); + List.push_front filename files + +-let rmdir dir = ++let rm_rf dir = + register (); + List.push_front dir rmdirs + +diff --git a/common/mltools/on_exit.mli b/common/mltools/on_exit.mli +index a02e3db3..9bcf104f 100644 +--- a/common/mltools/on_exit.mli ++++ b/common/mltools/on_exit.mli +@@ -47,7 +47,7 @@ val f : (unit -> unit) -> unit + val unlink : string -> unit + (** Unlink a single temporary file on exit. *) + +-val rmdir : string -> unit ++val rm_rf : string -> unit + (** Recursively remove a temporary directory on exit (using [rm -rf]). *) + + val kill : ?signal:int -> int -> unit +diff --git a/input/OVA.ml b/input/OVA.ml +index 9e9c3712..09ceee98 100644 +--- a/input/OVA.ml ++++ b/input/OVA.ml +@@ -78,7 +78,7 @@ let rec parse_ova ova = + else ( + let tmpdir = + let t = Mkdtemp.temp_dir ~base_dir:large_tmpdir "ova." in +- On_exit.rmdir t; ++ On_exit.rm_rf t; + t in + + match detect_file_type ova with +diff --git a/input/parse_domain_from_vmx.ml b/input/parse_domain_from_vmx.ml +index 947ca414..7aca2c24 100644 +--- a/input/parse_domain_from_vmx.ml ++++ b/input/parse_domain_from_vmx.ml +@@ -375,7 +375,7 @@ and find_nics vmx = + let parse_domain_from_vmx vmx_source = + let tmpdir = + let t = Mkdtemp.temp_dir "vmx." in +- On_exit.rmdir t; ++ On_exit.rm_rf t; + t in + + (* If the transport is SSH, fetch the file from remote, else +diff --git a/lib/nbdkit.ml b/lib/nbdkit.ml +index 07896684..1137b6bb 100644 +--- a/lib/nbdkit.ml ++++ b/lib/nbdkit.ml +@@ -105,7 +105,7 @@ let add_filter_if_available cmd filter = + let run_unix socket cmd = + (* Create a temporary directory where we place the PID file. *) + let piddir = Mkdtemp.temp_dir "v2vnbdkit." in +- On_exit.rmdir piddir; ++ On_exit.rm_rf piddir; + + let id = unique () in + let pidfile = piddir // sprintf "nbdkit%d.pid" id in +diff --git a/lib/qemuNBD.ml b/lib/qemuNBD.ml +index bbb65f41..c3dd1666 100644 +--- a/lib/qemuNBD.ml ++++ b/lib/qemuNBD.ml +@@ -69,7 +69,7 @@ let run_unix socket { disk; snapshot; format; imgopts } = + + (* Create a temporary directory where we place the PID file. *) + let piddir = Mkdtemp.temp_dir "v2vqemunbd." in +- On_exit.rmdir piddir; ++ On_exit.rm_rf piddir; + + let id = unique () in + let pidfile = piddir // sprintf "qemunbd%d.pid" id in +diff --git a/lib/utils.ml b/lib/utils.ml +index 7116a4f9..84b9a93f 100644 +--- a/lib/utils.ml ++++ b/lib/utils.ml +@@ -204,7 +204,7 @@ let error_if_no_ssh_agent () = + let create_v2v_directory () = + let d = Mkdtemp.temp_dir "v2v." in + chown_for_libvirt_rhbz_1045069 d; +- On_exit.rmdir d; ++ On_exit.rm_rf d; + d + + (* Wait for a file to appear until a timeout. *) +diff --git a/output/python_script.ml b/output/python_script.ml +index 54ccd1b5..ecf46c2d 100644 +--- a/output/python_script.ml ++++ b/output/python_script.ml +@@ -33,7 +33,7 @@ type script = { + + let create ?(name = "script.py") code = + let tmpdir = Mkdtemp.temp_dir "v2v." in +- On_exit.rmdir tmpdir; ++ On_exit.rm_rf tmpdir; + let path = tmpdir // name in + with_open_out path (fun chan -> output_string chan code); + { tmpdir; path } +-- +2.31.1 + diff --git a/0027-o-rhv-Unmount-the-temporary-NFS-mountpoint-as-late-a.patch b/0027-o-rhv-Unmount-the-temporary-NFS-mountpoint-as-late-a.patch new file mode 100644 index 0000000..3a37588 --- /dev/null +++ b/0027-o-rhv-Unmount-the-temporary-NFS-mountpoint-as-late-a.patch @@ -0,0 +1,174 @@ +From 0d92a42aab3fb0e7569294675666976724156128 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Thu, 14 Jul 2022 13:15:49 +0100 +Subject: [PATCH] -o rhv: Unmount the temporary NFS mountpoint as late as + possible + +To partially avoid a potential race against nbdkit or qemu-nbd +releasing files on the mountpoint before they exit, unmount as late as +we can. + +See also https://bugzilla.redhat.com/show_bug.cgi?id=1953286#c26 + +Reviewed-by: Laszlo Ersek +(cherry picked from commit e96357fc3b26aaf96eaa21afa36c894a27af6261) +--- + common | 2 +- + output/output_rhv.ml | 4 ++-- + 2 files changed, 3 insertions(+), 3 deletions(-) + +Submodule common fd964c1b..1000604f: +diff --git a/common/mltools/on_exit.ml b/common/mltools/on_exit.ml +index cae12e73..f8ef74e1 100644 +--- a/common/mltools/on_exit.ml ++++ b/common/mltools/on_exit.ml +@@ -23,39 +23,39 @@ open Common_gettext.Gettext + open Unix + open Printf + +-(* List of files to unlink. *) +-let files = ref [] ++type action = ++ | Unlink of string (* filename *) ++ | Rm_rf of string (* directory *) ++ | Kill of int * int (* signal, pid *) ++ | Fn of (unit -> unit) (* generic function *) + +-(* List of directories to remove. *) +-let rmdirs = ref [] +- +-(* List of PIDs to kill. *) +-let kills = ref [] +- +-(* List of functions to call. *) +-let fns = ref [] ++(* List of (priority, action). *) ++let actions = ref [] + + (* Perform a single exit action, printing any exception but + * otherwise ignoring failures. + *) +-let do_action f arg = +- try f arg with exn -> debug "%s" (Printexc.to_string exn) ++let do_action action = ++ try ++ match action with ++ | Unlink file -> Unix.unlink file ++ | Rm_rf dir -> ++ let cmd = sprintf "rm -rf -- %s" (Filename.quote dir) in ++ ignore (Tools_utils.shell_command cmd) ++ | Kill (signal, pid) -> ++ kill pid signal ++ | Fn f -> f () ++ with exn -> debug "%s" (Printexc.to_string exn) + + (* Make sure the actions are performed only once. *) + let done_actions = ref false + +-(* Perform the exit actions. *) ++(* Perform the exit actions in priority order (lowest prio first). *) + let do_actions () = + if not !done_actions then ( +- List.iter (do_action (fun f -> f ())) !fns; +- List.iter (do_action (fun (signal, pid) -> kill pid signal)) !kills; +- List.iter (do_action (fun file -> Unix.unlink file)) !files; +- List.iter (do_action ( +- fun dir -> +- let cmd = sprintf "rm -rf -- %s" (Filename.quote dir) in +- ignore (Tools_utils.shell_command cmd) +- ) +- ) !rmdirs; ++ let actions = List.sort (fun (a, _) (b, _) -> compare a b) !actions in ++ let actions = List.map snd actions in ++ List.iter do_action actions + ); + done_actions := true + +@@ -94,18 +94,18 @@ let register () = + ); + registered := true + +-let f fn = ++let f ?(prio = 5000) fn = + register (); +- List.push_front fn fns ++ List.push_front (prio, Fn fn) actions + +-let unlink filename = ++let unlink ?(prio = 5000) filename = + register (); +- List.push_front filename files ++ List.push_front (prio, Unlink filename) actions + +-let rm_rf dir = ++let rm_rf ?(prio = 5000) dir = + register (); +- List.push_front dir rmdirs ++ List.push_front (prio, Rm_rf dir) actions + +-let kill ?(signal = Sys.sigterm) pid = ++let kill ?(prio = 5000) ?(signal = Sys.sigterm) pid = + register (); +- List.push_front (signal, pid) kills ++ List.push_front (prio, Kill (signal, pid)) actions +diff --git a/common/mltools/on_exit.mli b/common/mltools/on_exit.mli +index 9bcf104f..66a85542 100644 +--- a/common/mltools/on_exit.mli ++++ b/common/mltools/on_exit.mli +@@ -28,6 +28,12 @@ + killing another process, so we provide simple + wrappers for those common actions here. + ++ Actions can be ordered by setting the optional [?prio] ++ parameter in the range 0..9999. By default actions ++ have priority 5000. Lower numbered actions run first. ++ Higher numbered actions run last. So to have an action ++ run at the very end before exit you might use [~prio:9999] ++ + Note this module registers signal handlers for + SIGINT, SIGQUIT, SIGTERM and SIGHUP. This means + that any program that links with mltools.cmxa +@@ -39,18 +45,20 @@ + Your cleanup action might no longer run unless the + program calls {!Stdlib.exit}. *) + +-val f : (unit -> unit) -> unit ++val f : ?prio:int -> (unit -> unit) -> unit + (** Register a function [f] which runs when the program exits. + Similar to [Stdlib.at_exit] but also runs if the program is +- killed with a signal that we can catch. *) ++ killed with a signal that we can catch. + +-val unlink : string -> unit ++ [?prio] is the priority, default 5000. See the description above. *) ++ ++val unlink : ?prio:int -> string -> unit + (** Unlink a single temporary file on exit. *) + +-val rm_rf : string -> unit ++val rm_rf : ?prio:int -> string -> unit + (** Recursively remove a temporary directory on exit (using [rm -rf]). *) + +-val kill : ?signal:int -> int -> unit ++val kill : ?prio:int -> ?signal:int -> int -> unit + (** Kill [PID] on exit. The signal sent defaults to [Sys.sigterm]. + + Use this with care since you can end up unintentionally killing +diff --git a/output/output_rhv.ml b/output/output_rhv.ml +index 8571e07b..15a2c14a 100644 +--- a/output/output_rhv.ml ++++ b/output/output_rhv.ml +@@ -204,8 +204,8 @@ module RHV = struct + if run_command cmd <> 0 then + error (f_"mount command failed, see earlier errors.\n\nThis probably means you didn't specify the right %s path [-os %s], or else you need to rerun virt-v2v as root.") domain_class os; + +- (* Make sure it is unmounted at exit. *) +- On_exit.f ( ++ (* Make sure it is unmounted at exit, as late as possible (prio=9999) *) ++ On_exit.f ~prio:9999 ( + fun () -> + let cmd = [ "umount"; mp ] in + ignore (run_command cmd); +-- +2.31.1 + diff --git a/0028-output-Permit-output-modes-to-wait-on-the-local-NBD-.patch b/0028-output-Permit-output-modes-to-wait-on-the-local-NBD-.patch new file mode 100644 index 0000000..5e97012 --- /dev/null +++ b/0028-output-Permit-output-modes-to-wait-on-the-local-NBD-.patch @@ -0,0 +1,182 @@ +From 96efdcf54c887ae88d54332df12a5f5dd962fd0a Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Fri, 15 Jul 2022 11:25:45 +0100 +Subject: [PATCH] output: Permit output modes to wait on the local NBD server + +Output.output_to_local_file is used by several output modes that write +to local files or devices. It launches an instance of qemu-nbd or +nbdkit connected to the local file. + +Previously we unconditionally added an On_exit handler to kill the NBD +server. This is usually safe because nbdcopy --flush has guaranteed +that the data was written through to permanent storage, and so killing +the NBD server is just there to prevent orphaned processes. + +However for output to RHV (-o rhv) we actually need the NBD server to +be cleaned up before we exit. See the analysis here: + +https://bugzilla.redhat.com/show_bug.cgi?id=1953286#c26 + +Allow an alternate strategy of waiting for the NBD server to exit +during virt-v2v shutdown. + +We only need this in virt-v2v so implement it here instead of pushing +it all the way into the On_exit module. + +Reviewed-by: Laszlo Ersek +(cherry picked from commit e2a1a7b4dfb6a9e44260da10a7e7029c09753b5c) +--- + output/output.ml | 91 ++++++++++++++++++++++++++++------------------- + output/output.mli | 17 +++++++-- + 2 files changed, 69 insertions(+), 39 deletions(-) + +diff --git a/output/output.ml b/output/output.ml +index 496c32b6..8f83a324 100644 +--- a/output/output.ml ++++ b/output/output.ml +@@ -69,7 +69,10 @@ let error_if_disk_count_gt dir n = + if Sys.file_exists socket then + error (f_"this output module doesn't support copying more than %d disks") n + ++type on_exit_kill = Kill | KillAndWait ++ + let output_to_local_file ?(changeuid = fun f -> f ()) ?(compressed = false) ++ ?(on_exit_kill = Kill) + output_alloc output_format filename size socket = + (* Check nbdkit is installed and has the required plugin. *) + if not (Nbdkit.is_installed ()) then +@@ -94,46 +97,60 @@ let output_to_local_file ?(changeuid = fun f -> f ()) ?(compressed = false) + fun () -> g#disk_create ?preallocation filename output_format size + ); + +- match output_format with +- | "raw" -> +- let cmd = Nbdkit.create "file" in +- Nbdkit.add_arg cmd "file" filename; +- if Nbdkit.version nbdkit_config >= (1, 22, 0) then ( +- let cmd = Nbdkit.add_arg cmd "cache" "none" in +- cmd +- ); +- let _, pid = Nbdkit.run_unix socket cmd in ++ let pid = ++ match output_format with ++ | "raw" -> ++ let cmd = Nbdkit.create "file" in ++ Nbdkit.add_arg cmd "file" filename; ++ if Nbdkit.version nbdkit_config >= (1, 22, 0) then ( ++ let cmd = Nbdkit.add_arg cmd "cache" "none" in ++ cmd ++ ); ++ let _, pid = Nbdkit.run_unix socket cmd in ++ pid + +- (* --exit-with-parent should ensure nbdkit is cleaned +- * up when we exit, but it's not supported everywhere. +- *) +- On_exit.kill pid ++ | "qcow2" -> ++ let cmd = ++ if compressed then ( ++ let qemu_quote str = String.replace str "," ",," in ++ let image_opts = [ "driver=compress"; ++ "file.driver=qcow2"; ++ "file.file.driver=file"; ++ "file.file.filename=" ^ qemu_quote filename ] in ++ let image_opts = String.concat "," image_opts in ++ let cmd = QemuNBD.create image_opts in ++ QemuNBD.set_image_opts cmd true; ++ cmd ++ ) ++ else (* not compressed *) ( ++ let cmd = QemuNBD.create filename in ++ QemuNBD.set_format cmd (Some "qcow2"); ++ cmd ++ ) in ++ QemuNBD.set_snapshot cmd false; ++ let _, pid = QemuNBD.run_unix socket cmd in ++ pid + +- | "qcow2" -> +- let cmd = +- if compressed then ( +- let qemu_quote str = String.replace str "," ",," in +- let image_opts = [ "driver=compress"; +- "file.driver=qcow2"; +- "file.file.driver=file"; +- "file.file.filename=" ^ qemu_quote filename ] in +- let image_opts = String.concat "," image_opts in +- let cmd = QemuNBD.create image_opts in +- QemuNBD.set_image_opts cmd true; +- cmd +- ) +- else (* not compressed *) ( +- let cmd = QemuNBD.create filename in +- QemuNBD.set_format cmd (Some "qcow2"); +- cmd +- ) in +- QemuNBD.set_snapshot cmd false; +- let _, pid = QemuNBD.run_unix socket cmd in +- On_exit.kill pid ++ | _ -> ++ error (f_"output mode only supports raw or qcow2 format (format: %s)") ++ output_format in ++ ++ match on_exit_kill with ++ | Kill -> ++ (* Kill the NBD server on exit. (For nbdkit we use --exit-with-parent ++ * but it's not supported everywhere). ++ *) ++ On_exit.kill pid + +- | _ -> +- error (f_"output mode only supports raw or qcow2 format (format: %s)") +- output_format ++ | KillAndWait -> ++ On_exit.f ( ++ fun () -> ++ kill pid Sys.sigterm; ++ (* Errors from the NBD server don't matter. On successful ++ * completion we've already committed the data to disk. ++ *) ++ ignore (waitpid [] pid) ++ ) + + let disk_path os name i = + let outdisk = sprintf "%s/%s-sd%s" os name (drive_name i) in +diff --git a/output/output.mli b/output/output.mli +index c1f0f53d..c4486311 100644 +--- a/output/output.mli ++++ b/output/output.mli +@@ -83,14 +83,27 @@ val error_if_disk_count_gt : string -> int -> unit + "in[n]" in the v2v directory [dir]. If the socket exists, [error] is + called. *) + ++type on_exit_kill = Kill | KillAndWait ++ + val output_to_local_file : ?changeuid:((unit -> unit) -> unit) -> +- ?compressed:bool -> ++ ?compressed:bool -> ?on_exit_kill:on_exit_kill -> + Types.output_allocation -> + string -> string -> int64 -> string -> + unit + (** When an output mode wants to create a local file with a + particular format (only "raw" or "qcow2" allowed) then +- this common function can be used. *) ++ this common function can be used. ++ ++ Optional parameter [?on_exit_kill] controls how the NBD server ++ is cleaned up. The default is {!Kill} which registers an ++ {!On_exit.kill} handler that kills (but does not wait for) ++ the server when virt-v2v exits. Most callers should use this. ++ ++ Setting [~on_exit_kill:KillAndWait] should be used if the NBD ++ server must fully exit before we continue with the rest of ++ virt-v2v shut down. This is only necessary if some other action ++ (such as unmounting a host filesystem or removing a host device) ++ depends on the NBD server releasing resources. *) + + val disk_path : string -> string -> int -> string + (** For [-o disk|qemu], return the output disk name of the i'th disk, +-- +2.31.1 + diff --git a/0029-o-rhv-Wait-for-the-NBD-server-to-exit-to-avoid-a-rac.patch b/0029-o-rhv-Wait-for-the-NBD-server-to-exit-to-avoid-a-rac.patch new file mode 100644 index 0000000..079abf1 --- /dev/null +++ b/0029-o-rhv-Wait-for-the-NBD-server-to-exit-to-avoid-a-rac.patch @@ -0,0 +1,36 @@ +From f820585c37beb648ab856818179091349a604523 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Fri, 15 Jul 2022 11:37:46 +0100 +Subject: [PATCH] -o rhv: Wait for the NBD server to exit to avoid a race with + unmounting + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1953286#c26 +Reviewed-by: Laszlo Ersek +(cherry picked from commit 2fbd578b4e6884a23063ad67ee36f02c4eb6c668) +--- + output/output_rhv.ml | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/output/output_rhv.ml b/output/output_rhv.ml +index 15a2c14a..45f831e3 100644 +--- a/output/output_rhv.ml ++++ b/output/output_rhv.ml +@@ -175,7 +175,14 @@ module RHV = struct + chmod filename 0o666 + ) + in +- output_to_local_file ~changeuid ++ ++ (* We have to wait for the NBD server to exit rather than just ++ * killing it, otherwise it races with unmounting. See: ++ * https://bugzilla.redhat.com/show_bug.cgi?id=1953286#c26 ++ *) ++ let on_exit_kill = Output.KillAndWait in ++ ++ output_to_local_file ~changeuid ~on_exit_kill + output_alloc output_format filename size socket + ) (List.combine disks filenames); + +-- +2.31.1 + diff --git a/virt-v2v.spec b/virt-v2v.spec index 6c50294..1c18abe 100644 --- a/virt-v2v.spec +++ b/virt-v2v.spec @@ -16,7 +16,7 @@ Name: virt-v2v Epoch: 1 Version: 2.0.7 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Convert a virtual machine to run on KVM License: GPLv2+ @@ -55,6 +55,13 @@ Patch0019: 0019-o-disk-o-libvirt-o-qemu-Implement-of-qcow2-oo-compre.patch Patch0020: 0020-tests-Add-a-simple-test-of-o-local-of-qcow2-oo-compr.patch Patch0021: 0021-RHEL-9-oo-compressed-Remove-nbdcopy-version-check-an.patch Patch0022: 0022-RHEL-9-tests-Remove-btrfs-test.patch +Patch0023: 0023-convert-convert_linux-complete-the-remapping-of-NVMe.patch +Patch0024: 0024-input-xen-sync-ip-limitations-language-from-input-vm.patch +Patch0025: 0025-input-xen-replace-enable-LEGACY-crypto-advice-with-t.patch +Patch0026: 0026-common-Adapt-to-renamed-function-On_exit.rmdir-On_ex.patch +Patch0027: 0027-o-rhv-Unmount-the-temporary-NFS-mountpoint-as-late-a.patch +Patch0028: 0028-output-Permit-output-modes-to-wait-on-the-local-NBD-.patch +Patch0029: 0029-o-rhv-Wait-for-the-NBD-server-to-exit-to-avoid-a-rac.patch %if !0%{?rhel} # libguestfs hasn't been built on i686 for a while since there is no @@ -333,7 +340,7 @@ rm $RPM_BUILD_ROOT%{_mandir}/man1/virt-v2v-in-place.1* %changelog -* Wed Jul 06 2022 Richard W.M. Jones - 1:2.0.7-1 +* Fri Jul 15 2022 Richard W.M. Jones - 1:2.0.7-2 - Rebase to stable branch version 2.0.7 resolves: rhbz#2059287, rhbz#1658126, rhbz#1788823, rhbz#1854275 - Fix openssh-clients dependency @@ -377,11 +384,19 @@ rm $RPM_BUILD_ROOT%{_mandir}/man1/virt-v2v-in-place.1* resolves: rhbz#2102719 - Add -oo compressed support resolves: rhbz#2047660 -- Install qemu-ga package during conversion (2028764) +- Install qemu-ga package during conversion + resolves: rhbz#2028764 - Limit the maximum of disks per guest resolves: rhbz#2051564 - Add support for LUKS encrypted guests using Clevis & Tang resolves: rhbz#1809453 +- Fix remapping of nvme devices in /boot/grub2/device.map + resolves: rhbz#2101665 +- Improve documentation of vmx+ssh and -ip option + resolves: rhbz#1854275 +- Remove legacy crypto advice and replace with targeted mechanism + resolves: rhbz#2062360 +- Fix race condition when unmounting in -o rhv mode (1953286#c26) * Tue Feb 15 2022 Richard W.M. Jones - 1:1.45.99-1 - Rebase to upstream 1.45.99.