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);