rpms
/
virt-v2v
7
0
Fork 0
Code Issues Pull Requests Releases Activity

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)
This commit is contained in:
Richard W.M. Jones 2022-07-15 15:48:52 +01:00
parent 0ab0657041
commit 3a13726152
8 changed files with 805 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
0023-convert-convert_linux-complete-the-remapping-of-NVMe.patch Normal file
View File

@ -0,0 +1,83 @@
From ba2963bc57c8c8a3d6f7cc2fd274c9ebd4ddb7d8 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
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 <lersek@redhat.com>
Message-Id: <20220706103215.5607-1-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
(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

52
0024-input-xen-sync-ip-limitations-language-from-input-vm.patch Normal file
View File

@ -0,0 +1,52 @@
From c34fe9a52abdde05cb31c5bd2c99237652e1b0dc Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Mon, 11 Jul 2022 09:01:56 +0200
Subject: [PATCH] input-xen: sync "-ip" limitations language from input-vmware
manual
My analysis in <https://bugzilla.redhat.com/show_bug.cgi?id=1854275#c33>
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 <lersek@redhat.com>
Message-Id: <20220711070157.5399-2-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
(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<https://bugzilla.redhat.com/1854275>.
+
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

85
0025-input-xen-replace-enable-LEGACY-crypto-advice-with-t.patch Normal file
View File

@ -0,0 +1,85 @@
From 3f7f730ac9cbf38267839ffcebd6b6fd721123c5 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
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 <lersek@redhat.com>
Message-Id: <20220711070157.5399-3-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
(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<https://bugzilla.redhat.com/1854275>.
-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<update-crypto-policies(8)> 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<Host> 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<PubkeyAcceptedKeyTypes> and C<PubkeyAcceptedAlgorithms> have
+identical meaning; the former is the old option name, the latter is the
+new one. Virt-v2v uses both C<libssh> and C<ssh> when converting a guest
+from Xen, and on some operating systems, C<libssh> and C<ssh> may not
+both accept the same option variant.)
=head2 Test libvirt connection to remote Xen host
--
2.31.1

175
0026-common-Adapt-to-renamed-function-On_exit.rmdir-On_ex.patch Normal file
View File

@ -0,0 +1,175 @@
From ea881513e9c15b0a816d3ba4afe471ff2f591a03 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
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 <lersek@redhat.com>
(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

174
0027-o-rhv-Unmount-the-temporary-NFS-mountpoint-as-late-a.patch Normal file
View File

@ -0,0 +1,174 @@
From 0d92a42aab3fb0e7569294675666976724156128 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
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 <lersek@redhat.com>
(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

182
0028-output-Permit-output-modes-to-wait-on-the-local-NBD-.patch Normal file
View File

@ -0,0 +1,182 @@
From 96efdcf54c887ae88d54332df12a5f5dd962fd0a Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
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 <lersek@redhat.com>
(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

36
0029-o-rhv-Wait-for-the-NBD-server-to-exit-to-avoid-a-rac.patch Normal file
View File

@ -0,0 +1,36 @@
From f820585c37beb648ab856818179091349a604523 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
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 <lersek@redhat.com>
(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

21
virt-v2v.spec
View File

@ -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 <rjones@redhat.com> - 1:2.0.7-1
* Fri Jul 15 2022 Richard W.M. Jones <rjones@redhat.com> - 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 <rjones@redhat.com> - 1:1.45.99-1
- Rebase to upstream 1.45.99.