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