From 89edc42bc6e62851dffc32276b0c24f2a4bf8cf6 Mon Sep 17 00:00:00 2001

From: Pino Toscano <ptoscano@redhat.com>

Date: Wed, 3 Aug 2016 16:40:20 +0200

Subject: [PATCH] dib: rework run of extra-data.d hooks (RHBZ#1362354)

Instead of running them before lanching the appliance with the disks and

then uploading the result after root.d hooks run, mount the root in the

local temporary directory using FUSE and then run the hooks on the

guest.  Other than being closer to what diskimage-builder does, it also

avoids issues with the extra-data.d scripts assuming to find things

already, and we don't error out while trying to unpack files on the

guest.

Since virt-dib requires FUSE now, build it only if FUSE is supported.

(cherry picked from commit d12be6625a74b4a088c75da5ae3a968678d814fd)

---

 Makefile.am      |   4 +-

 dib/Makefile.am  |   4 +-

 dib/dib.ml       | 123 +++++++++++++++++++++++++++++++------------------------

 dib/virt-dib.pod |  23 -----------

 4 files changed, 76 insertions(+), 78 deletions(-)

diff --git a/Makefile.am b/Makefile.am

index ce20058..0945993 100644

--- a/Makefile.am

+++ b/Makefile.am

@@ -145,7 +145,6 @@ SUBDIRS += \

 	mllib \

 	customize \

 	builder builder/website \

-	dib \

 	get-kernel \

 	resize \

 	sparsify \

@@ -154,6 +153,9 @@ SUBDIRS += \

 if HAVE_OCAML_PKG_LIBVIRT

 SUBDIRS += v2v/test-harness

 endif

+if HAVE_FUSE

+SUBDIRS += dib

+endif

 endif

 # Perl tools.

diff --git a/dib/Makefile.am b/dib/Makefile.am

index ff6a933..ac0af26 100644

--- a/dib/Makefile.am

+++ b/dib/Makefile.am

@@ -34,7 +34,8 @@ SOURCES_ML = \

 SOURCES_C = \

 	../mllib/dev_t-c.c \

-	../mllib/mkdtemp-c.c

+	../mllib/mkdtemp-c.c \

+	../mllib/exit-c.c

 bin_PROGRAMS =

@@ -62,6 +63,7 @@ BOBJECTS = \

 	$(top_builddir)/mllib/stringMap.cmo \

 	$(top_builddir)/mllib/common_utils.cmo \

 	$(top_builddir)/mllib/mkdtemp.cmo \

+	$(top_builddir)/mllib/exit.cmo \

 	$(SOURCES_ML:.ml=.cmo)

 XOBJECTS = $(BOBJECTS:.cmo=.cmx)

diff --git a/dib/dib.ml b/dib/dib.ml

index 87af4eb..17775d8 100644

--- a/dib/dib.ml

+++ b/dib/dib.ml

@@ -69,13 +69,15 @@ let envvars_string l =

 let prepare_external ~envvars ~dib_args ~dib_vars ~out_name ~root_label

   ~rootfs_uuid ~image_cache ~arch ~network ~debug

-  destdir libdir hooksdir tmpdir fakebindir all_elements element_paths =

+  destdir libdir hooksdir fakebindir all_elements element_paths =

   let network_string = if network then "" else "1" in

   let run_extra = sprintf "\

 #!/bin/bash

 set -e

 %s

+mount_dir=$1

+shift

 target_dir=$1

 shift

 script=$1

@@ -87,7 +89,7 @@ shift

 export PATH=%s:$PATH

 # d-i-b variables

-export TMP_MOUNT_PATH=%s

+export TMP_MOUNT_PATH=\"$mount_dir\"

 export DIB_OFFLINE=%s

 export IMAGE_NAME=\"%s\"

 export DIB_ROOT_LABEL=\"%s\"

@@ -120,7 +122,6 @@ $target_dir/$script

     (if debug >= 1 then "set -x\n" else "")

     (envvars_string envvars)

     fakebindir

-    (quote tmpdir)

     network_string

     out_name

     root_label

@@ -134,10 +135,7 @@ $target_dir/$script

     (String.concat ":" element_paths)

     (quote dib_vars)

     debug in

-  write_script (destdir // "run-part-extra.sh") run_extra;

-

-  (* Needed as TMPDIR for the extra-data hooks *)

-  do_mkdir (tmpdir // "tmp")

+  write_script (destdir // "run-part-extra.sh") run_extra

 let prepare_aux ~envvars ~dib_args ~dib_vars ~log_file ~out_name ~rootfs_uuid

   ~arch ~network ~root_label ~install_type ~debug ~extra_packages

@@ -392,26 +390,61 @@ let run_parts ~debug ~sysroot ~blockdev ~log_file ?(new_wd = "")

   flush_all ();

   Buffer.contents outbuf

-let run_parts_host ~debug hooks_dir hook_name scripts run_script =

+let run_parts_host ~debug (g : Guestfs.guestfs) hooks_dir hook_name base_mount_dir scripts run_script =

   let hook_dir = hooks_dir // hook_name in

   let scripts = List.sort digit_prefix_compare scripts in

-  let timings = Hashtbl.create 13 in

-  List.iter (

-    fun x ->

-      message (f_"Running: %s/%s") hook_name x;

-      let cmd = [ run_script; hook_dir; x ] in

-      let run () =

-        run_command cmd in

-      let delta_t = timed_run run in

-      if debug >= 1 then (

-        printf "\n";

-        printf "%s completed after %.3f s\n" x delta_t

-      );

-      Hashtbl.add timings x delta_t;

-  ) scripts;

-  if debug >= 1 then (

-    print_string (timing_output ~target_name:hook_name scripts timings)

+  let mount_dir = base_mount_dir // hook_name in

+  do_mkdir mount_dir;

+

+  let rec fork_and_run () =

+    let pid = Unix.fork () in

+    if pid = 0 then ( (* child *)

+      let retcode = run_scripts () in

+      flush_all ();

+      let cmd = [ "guestunmount"; mount_dir ] in

+      ignore (run_command cmd);

+      Exit._exit retcode

+    );

+    pid

+  and run_scripts () =

+    let timings = Hashtbl.create 13 in

+    let rec loop = function

+      | x :: xs ->

+        message (f_"Running: %s/%s") hook_name x;

+        let cmd = [ run_script; mount_dir; hook_dir; x ] in

+        let retcode = ref 0 in

+        let run () =

+          retcode := run_command cmd in

+        let delta_t = timed_run run in

+        if debug >= 1 then (

+          printf "\n";

+          printf "%s completed after %.3f s\n" x delta_t

+        );

+        Hashtbl.add timings x delta_t;

+        let retcode = !retcode in

+        if retcode <> 0 then retcode

+        else loop xs

+      | [] -> 0

+    in

+    let retcode = loop scripts in

+    if debug >= 1 then (

+      print_string (timing_output ~target_name:hook_name scripts timings)

+    );

+    retcode

+  in

+

+  g#mount_local mount_dir;

+  let pid = fork_and_run () in

+  g#mount_local_run ();

+

+  (match snd (Unix.waitpid [] pid) with

+  | Unix.WEXITED 0 -> ()

+  | Unix.WEXITED i -> exit i

+  | Unix.WSIGNALED i

+  | Unix.WSTOPPED i ->

+    error (f_"sub-process killed by signal (%d)") i

   );

+

   flush_all ()

 let run_install_packages ~debug ~blockdev ~log_file

@@ -455,8 +488,6 @@ let main () =

   do_mkdir auxtmpdir;

   let hookstmpdir = auxtmpdir // "hooks" in

   do_mkdir (hookstmpdir // "environment.d");    (* Just like d-i-b does. *)

-  let extradatatmpdir = tmpdir // "extra-data" in

-  do_mkdir extradatatmpdir;

   do_mkdir (auxtmpdir // "out" // image_basename_d);

   let elements =

     if cmdline.use_base then ["base"] @ cmdline.elements

@@ -575,20 +606,11 @@ let main () =

   prepare_external ~envvars ~dib_args ~dib_vars ~out_name:image_basename

                    ~root_label ~rootfs_uuid ~image_cache ~arch

                    ~network:cmdline.network ~debug

-                   tmpdir cmdline.basepath hookstmpdir extradatatmpdir

+                   tmpdir cmdline.basepath hookstmpdir

                    (auxtmpdir // "fake-bin")

                    all_elements cmdline.element_paths;

-  let run_hook_host hook =

-    try

-      let scripts = Hashtbl.find final_hooks hook in

-      if debug >= 1 then (

-        printf "Running hooks for %s...\n%!" hook;

-      );

-      run_parts_host ~debug hookstmpdir hook scripts

-        (tmpdir // "run-part-extra.sh")

-    with Not_found -> ()

-  and run_hook ~blockdev ~sysroot ?(new_wd = "") (g : Guestfs.guestfs) hook =

+  let run_hook ~blockdev ~sysroot ?(new_wd = "") (g : Guestfs.guestfs) hook =

     try

       let scripts = Hashtbl.find final_hooks hook in

       if debug >= 1 then (

@@ -597,8 +619,6 @@ let main () =

       run_parts ~debug ~sysroot ~blockdev ~log_file ~new_wd g hook scripts

     with Not_found -> "" in

-  run_hook_host "extra-data.d";

-

   let copy_in (g : Guestfs.guestfs) srcdir destdir =

     let desttar = Filename.temp_file ~temp_dir:tmpdir "virt-dib." ".tar.gz" in

     let cmd = [ "tar"; "czf"; desttar; "-C"; srcdir; "--owner=root";

@@ -608,18 +628,6 @@ let main () =

     g#tar_in ~compress:"gzip" desttar destdir;

     Sys.remove desttar in

-  let copy_preserve_in (g : Guestfs.guestfs) srcdir destdir =

-    let desttar = Filename.temp_file ~temp_dir:tmpdir "virt-dib." ".tar.gz" in

-    let remotetar = "/tmp/aux/" ^ (Filename.basename desttar) in

-    let cmd = [ "tar"; "czf"; desttar; "-C"; srcdir; "--owner=root";

-                "--group=root"; "." ] in

-    if run_command cmd <> 0 then exit 1;

-    g#upload desttar remotetar;

-    let verbose_flag = if debug > 0 then "v" else "" in

-    ignore (g#debug "sh" [| "tar"; "-C"; "/sysroot" ^ destdir; "--no-overwrite-dir"; "-x" ^ verbose_flag ^ "zf"; "/sysroot" ^ remotetar |]);

-    Sys.remove desttar;

-    g#rm remotetar in

-

   if debug >= 1 then

     ignore (run_command [ "tree"; "-ps"; tmpdir ]);

@@ -747,7 +755,16 @@ let main () =

   and run_hook_subroot hook =

     do_run_hooks_noout ~sysroot:Subroot hook

   and do_run_hooks_noout ~sysroot ?(new_wd = "") hook =

-    ignore (run_hook ~sysroot ~blockdev ~new_wd g hook) in

+    ignore (run_hook ~sysroot ~blockdev ~new_wd g hook)

+  and run_hook_host hook =

+    try

+      let scripts = Hashtbl.find final_hooks hook in

+      if debug >= 1 then (

+        printf "Running hooks for %s...\n%!" hook;

+      );

+      run_parts_host ~debug g hookstmpdir hook tmpdir scripts

+        (tmpdir // "run-part-extra.sh")

+    with Not_found -> () in

   g#sync ();

   checked_umount_all ();

@@ -799,7 +816,7 @@ let main () =

   mount_aux ();

   g#ln_s "aux/hooks" "/tmp/in_target.d";

-  copy_preserve_in g extradatatmpdir "/";

+  run_hook_host "extra-data.d";

   run_hook_in "pre-install.d";

diff --git a/dib/virt-dib.pod b/dib/virt-dib.pod

index 8ccb9f5..41e7ec7 100644

--- a/dib/virt-dib.pod

+++ b/dib/virt-dib.pod

@@ -577,29 +577,6 @@ arguments

 =item

-C<extra-data.d> scripts run in the host environment, before all the

-other ones (even C<root.d>); this means that, depending on the

-configuration for the elements, some of them may fail due to missing

-content (usually directories) in C<TMP_HOOKS_PATH>.

-

-Workarounds for this may be either:

-

-=over 4

-

-=item

-

-fix the C<extra-data.d> scripts to create the missing directories

-

-=item

-

-create (and use) a simple element with a C<extra-data.d> script

-named e.g. F<00-create-missing-dirs> to create the missing

-directories

-

-=back

-

-=item

-

 extra tools needed on some out-of-chroot phases need to be available

 in the appliance, see L</EXTRA DEPENDENCIES>.

-- 

2.7.4