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

Richard W.M. Jones
2016-08-03T15:21:25Z

On Wed, Aug 03, 2016 at 04:49:43PM +0200, Pino Toscano
wrote:
> 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.
> ---
>  Makefile.am      |   4 +-
>  dib/dib.ml       | 123
+++++++++++++++++++++++++++++++------------------------
>  dib/virt-dib.pod |  23 -----------
>  3 files changed, 73 insertions(+), 77 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 4b5babb..acc6bd4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -147,7 +147,6 @@ SUBDIRS += \
>  	mllib \
>  	customize \
>  	builder builder/website \
> -	dib \
>  	get-kernel \
>  	resize \
>  	sparsify \
> @@ -156,6 +155,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/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
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

ACK

Rich.

--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch