[Libguestfs] [PATCH virt-v2v v3 1/2] output: Permit output modes to wait on the local NBD server

Richard W.M. Jones rjones at redhat.com
Fri Jul 15 12:12:00 UTC 2022


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.
---
 output/output.mli | 17 +++++++--
 output/output.ml  | 91 ++++++++++++++++++++++++++++-------------------
 2 files changed, 69 insertions(+), 39 deletions(-)

diff --git a/output/output.mli b/output/output.mli
index c1f0f53d8e..c448631189 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,
diff --git a/output/output.ml b/output/output.ml
index 23c3932d1b..6065e5923d 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
@@ -105,46 +108,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
-- 
2.37.0.rc2



More information about the Libguestfs mailing list