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,