Blob Blame History Raw
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);