Libguestfs - Dec 2015 - [PATCH 0/3] supermin: add --include-packagelist

Hi,

to ease debugging issues with appliances (e.g. when used in libguestfs),
using --include-packagelist will add a file containing the list of all
the packages used.

Thanks,


Pino Toscano (3):
  ext2: add ext2fs_chmod and ext2fs_chown
  chroot: factor out file copy code
  Add --include-packagelist

 src/build.ml     | 42 +++++++++++++++++++++++++------
 src/chroot.ml    | 29 ++++++++++++++++++---
 src/ext2.ml      | 18 +++++++++++++-
 src/ext2fs-c.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ext2fs.ml    |  2 ++
 src/ext2fs.mli   |  2 ++
 src/prepare.ml   |  3 ++-
 src/supermin.ml  |  9 +++++--
 src/supermin.pod | 11 ++++++++
 9 files changed, 176 insertions(+), 16 deletions(-)

-- 
2.1.0

Implement two simple helpers to change permissions and ownership of an
existing path.
---
 src/ext2fs-c.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ext2fs.ml  |  2 ++
 src/ext2fs.mli |  2 ++
 3 files changed, 80 insertions(+)

diff --git a/src/ext2fs-c.c b/src/ext2fs-c.c
index f3ca7dc..f01ca9d 100644
--- a/src/ext2fs-c.c
+++ b/src/ext2fs-c.c
@@ -289,6 +289,82 @@ supermin_ext2fs_copy_dir_recursively_from_host (value fsv,
   CAMLreturn (Val_unit);
 }
 
+/* Change the permissions of 'path' to 'mode'.
+ */
+value
+supermin_ext2fs_chmod (value fsv, value pathv, value modev)
+{
+  CAMLparam3 (fsv, pathv, modev);
+  const char *path = String_val (pathv);
+  mode_t mode = Int_val (modev);
+  errcode_t err;
+  struct ext2_data data;
+
+  data = Ext2fs_val (fsv);
+  if (data.fs == NULL)
+    ext2_handle_closed ();
+
+  ext2_ino_t ino;
+  ++path;
+  if (*path == 0) {             /* "/" */
+    ino = EXT2_ROOT_INO;
+  } else {                      /* "/foo" */
+    err = ext2fs_namei (data.fs, EXT2_ROOT_INO, EXT2_ROOT_INO, path, &ino);
+    if (err != 0)
+      ext2_error_to_exception ("ext2fs_namei", err, path);
+  }
+
+  struct ext2_inode inode;
+  err = ext2fs_read_inode (data.fs, ino, &inode);
+  if (err != 0)
+    ext2_error_to_exception ("ext2fs_read_inode", err, path);
+  inode.i_mode = (inode.i_mode & ~07777) | mode;
+  err = ext2fs_write_inode (data.fs, ino, &inode);
+  if (err != 0)
+    ext2_error_to_exception ("ext2fs_write_inode", err, path);
+
+  CAMLreturn (Val_unit);
+}
+
+/* Change the ownership of 'path' to 'uid' and 'gid'.
+ */
+value
+supermin_ext2fs_chown (value fsv, value pathv, value uidv, value gidv)
+{
+  CAMLparam4 (fsv, pathv, uidv, gidv);
+  const char *path = String_val (pathv);
+  int uid = Int_val (uidv);
+  int gid = Int_val (gidv);
+  errcode_t err;
+  struct ext2_data data;
+
+  data = Ext2fs_val (fsv);
+  if (data.fs == NULL)
+    ext2_handle_closed ();
+
+  ext2_ino_t ino;
+  ++path;
+  if (*path == 0) {             /* "/" */
+    ino = EXT2_ROOT_INO;
+  } else {                      /* "/foo" */
+    err = ext2fs_namei (data.fs, EXT2_ROOT_INO, EXT2_ROOT_INO, path, &ino);
+    if (err != 0)
+      ext2_error_to_exception ("ext2fs_namei", err, path);
+  }
+
+  struct ext2_inode inode;
+  err = ext2fs_read_inode (data.fs, ino, &inode);
+  if (err != 0)
+    ext2_error_to_exception ("ext2fs_read_inode", err, path);
+  inode.i_uid = uid;
+  inode.i_gid = gid;
+  err = ext2fs_write_inode (data.fs, ino, &inode);
+  if (err != 0)
+    ext2_error_to_exception ("ext2fs_write_inode", err, path);
+
+  CAMLreturn (Val_unit);
+}
+
 static void
 ext2_mkdir (ext2_filsys fs,
             ext2_ino_t dir_ino, const char *dirname, const char *basename,
diff --git a/src/ext2fs.ml b/src/ext2fs.ml
index 9f3580e..7468aed 100644
--- a/src/ext2fs.ml
+++ b/src/ext2fs.ml
@@ -24,3 +24,5 @@ external ext2fs_close : t -> unit =
"supermin_ext2fs_close"
 external ext2fs_read_bitmaps : t -> unit =
"supermin_ext2fs_read_bitmaps"
 external ext2fs_copy_file_from_host : t -> string -> string -> unit =
"supermin_ext2fs_copy_file_from_host"
 external ext2fs_copy_dir_recursively_from_host : t -> string -> string
-> unit = "supermin_ext2fs_copy_dir_recursively_from_host"
+external ext2fs_chmod : t -> string -> Unix.file_perm -> unit =
"supermin_ext2fs_chmod"
+external ext2fs_chown : t -> string -> int -> int -> unit =
"supermin_ext2fs_chown"
diff --git a/src/ext2fs.mli b/src/ext2fs.mli
index 2a59b2d..7a60622 100644
--- a/src/ext2fs.mli
+++ b/src/ext2fs.mli
@@ -31,3 +31,5 @@ val ext2fs_close : t -> unit
 val ext2fs_read_bitmaps : t -> unit
 val ext2fs_copy_file_from_host : t -> string -> string -> unit
 val ext2fs_copy_dir_recursively_from_host : t -> string -> string ->
unit
+val ext2fs_chmod : t -> string -> Unix.file_perm -> unit
+val ext2fs_chown : t -> string -> int -> int -> unit
-- 
2.1.0

Move the code executing cp in an helper function; just code motion.
---
 src/chroot.ml | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/chroot.ml b/src/chroot.ml
index d0ee4c3..b9c1dc2 100644
--- a/src/chroot.ml
+++ b/src/chroot.ml
@@ -23,6 +23,12 @@ open Utils
 open Package_handler
 
 let build_chroot debug files outputdir +  let do_copy src dest +    if debug
>= 2 then printf "supermin: chroot: copy %s\n%!" dest;
+    let cmd = sprintf "cp -p %s %s" (quote src) (quote dest) in
+    ignore (Sys.command cmd)
+  in
+
   List.iter (
     fun file ->
       try
@@ -59,9 +65,7 @@ let build_chroot debug files outputdir            symlink link
opath
 
         | S_REG | S_CHR | S_BLK | S_FIFO | S_SOCK ->
-          if debug >= 2 then printf "supermin: chroot: copy
%s\n%!" opath;
-          let cmd = sprintf "cp -p %s %s" (quote path) (quote opath)
in
-          ignore (Sys.command cmd)
+          do_copy path opath
       with Unix_error _ -> ()
   ) files;
 
-- 
2.1.0

Introduce a new option, mainly used for debugging, to add in the
root of the appliance a file containing the list of all the packages
used to build it.
---
 src/build.ml     | 42 ++++++++++++++++++++++++++++++++++--------
 src/chroot.ml    | 19 ++++++++++++++++++-
 src/ext2.ml      | 18 +++++++++++++++++-
 src/prepare.ml   |  3 ++-
 src/supermin.ml  |  9 +++++++--
 src/supermin.pod | 11 +++++++++++
 6 files changed, 89 insertions(+), 13 deletions(-)

diff --git a/src/build.ml b/src/build.ml
index fb2e6d3..4675454 100644
--- a/src/build.ml
+++ b/src/build.ml
@@ -59,7 +59,8 @@ and string_of_file_content = function
 
 let rec build debug
     (copy_kernel, dtb_wildcard, format, host_cpu,
-     packager_config, tmpdir, use_installed, size)
+     packager_config, tmpdir, use_installed, size,
+     include_packagelist)
     inputs outputdir    if debug >= 1 then
     printf "supermin: build: %s\n%!" (String.concat " "
inputs);
@@ -101,14 +102,22 @@ let rec build debug
     let packages = package_set_of_list packages in
     get_all_requires packages in
 
+  (* Get the list of packages only if we need to, i.e. when creating
+   * /packagelist in the appliance, when printing all the packages
+   * for debug, or in both cases.
+   *)
+  let pretty_packages +    if include_packagelist || debug >= 2 then (
+      let pkg_names = PackageSet.elements packages in
+      let pkg_names = List.map ph.ph_package_to_string pkg_names in
+      List.sort compare pkg_names
+    ) else [] in
+
   if debug >= 1 then (
     printf "supermin: build: %d packages, including dependencies\n%!"
       (PackageSet.cardinal packages);
     if debug >= 2 then (
-      let pkg_names = PackageSet.elements packages in
-      let pkg_names = List.map ph.ph_package_to_string pkg_names in
-      let pkg_names = List.sort compare pkg_names in
-      List.iter (printf "  - %s\n") pkg_names;
+      List.iter (printf "  - %s\n") pretty_packages;
       flush Pervasives.stdout
     )
   );
@@ -197,11 +206,21 @@ let rec build debug
     )
   );
 
+  (* Create a temporary file for packagelist, if requested. *)
+  let packagelist_file +    if include_packagelist then (
+      let filename, chan = Filename.open_temp_file "packagelist."
"" in
+      List.iter (fprintf chan "%s\n") pretty_packages;
+      flush chan;
+      close_out chan;
+      Some filename
+    ) else None in
+
   (* Depending on the format, we build the appliance in different ways. *)
-  match format with
+  (match format with
   | Chroot ->
     (* chroot doesn't need an external kernel or initrd *)
-    Chroot.build_chroot debug files outputdir
+    Chroot.build_chroot debug files outputdir packagelist_file
 
   | Ext2 ->
     let kernel = outputdir // "kernel"
@@ -210,8 +229,15 @@ let rec build debug
     and initrd = outputdir // "initrd" in
     let kernel_version, modpath        Kernel.build_kernel debug host_cpu
dtb_wildcard copy_kernel kernel dtb in
-    Ext2.build_ext2 debug basedir files modpath kernel_version appliance size;
+    Ext2.build_ext2 debug basedir files modpath kernel_version appliance size
+      packagelist_file;
     Ext2_initrd.build_initrd debug tmpdir modpath initrd
+  );
+
+  (match packagelist_file with
+  | None -> ()
+  | Some filename -> Sys.remove filename
+  )
 
 and read_appliance debug basedir appliance = function
   | [] -> appliance
diff --git a/src/chroot.ml b/src/chroot.ml
index b9c1dc2..346c24b 100644
--- a/src/chroot.ml
+++ b/src/chroot.ml
@@ -22,7 +22,7 @@ open Printf
 open Utils
 open Package_handler
 
-let build_chroot debug files outputdir +let build_chroot debug files outputdir
packagelist_file    let do_copy src dest      if debug >= 2 then printf
"supermin: chroot: copy %s\n%!" dest;
     let cmd = sprintf "cp -p %s %s" (quote src) (quote dest) in
@@ -69,6 +69,23 @@ let build_chroot debug files outputdir        with Unix_error
_ -> ()
   ) files;
 
+  (* Add packagelist file, if requested. *)
+  (match packagelist_file with
+  | None -> ()
+  | Some filename ->
+    if debug >= 1 then
+      printf "supermin: chroot: creating /packagelist\n%!";
+
+    let opath = outputdir // "packagelist" in
+
+    do_copy filename opath;
+    (* Change the permissions of the file to be sure it is readable
+     * by everyone.  Unfortunately we cannot change the ownership,
+     * as non-root users cannot give away files to other users.
+     *)
+    chmod opath 0o644
+  );
+
   (* Second pass: fix up directory permissions in reverse. *)
   let dirs = filter_map (
     fun file ->
diff --git a/src/ext2.ml b/src/ext2.ml
index 79539a3..6348c29 100644
--- a/src/ext2.ml
+++ b/src/ext2.ml
@@ -34,7 +34,8 @@ open Package_handler
  *)
 let default_appliance_size = 4L *^ 1024L *^ 1024L *^ 1024L
 
-let build_ext2 debug basedir files modpath kernel_version appliance size +let
build_ext2 debug basedir files modpath kernel_version appliance size
+    packagelist_file    if debug >= 1 then
     printf "supermin: ext2: creating empty ext2 filesystem
'%s'\n%!" appliance;
 
@@ -74,6 +75,21 @@ let build_ext2 debug basedir files modpath kernel_version
appliance size        ext2fs_copy_file_from_host fs src file.ft_path
   ) files;
 
+  (* Add packagelist file, if requested. *)
+  (match packagelist_file with
+  | None -> ()
+  | Some filename ->
+    if debug >= 1 then
+      printf "supermin: ext2: creating /packagelist\n%!";
+
+    ext2fs_copy_file_from_host fs filename "/packagelist";
+    (* Change the permissions and ownership of the file, to be sure
+     * it is root-owned, and readable by everyone.
+     *)
+    ext2fs_chmod fs "/packagelist" 0o644;
+    ext2fs_chown fs "/packagelist" 0 0
+  );
+
   if debug >= 1 then
     printf "supermin: ext2: copying kernel modules\n%!";
 
diff --git a/src/prepare.ml b/src/prepare.ml
index 7e522e9..8193f36 100644
--- a/src/prepare.ml
+++ b/src/prepare.ml
@@ -22,7 +22,8 @@ open Package_handler
 open Utils
 
 let prepare debug (copy_kernel, dtb_wildcard, format, host_cpu,
-             packager_config, tmpdir, use_installed, size)
+             packager_config, tmpdir, use_installed, size,
+             include_packagelist)
     inputs outputdir    if debug >= 1 then
     printf "supermin: prepare: %s\n%!" (String.concat " "
inputs);
diff --git a/src/supermin.ml b/src/supermin.ml
index 3070b6b..bbb1dba 100644
--- a/src/supermin.ml
+++ b/src/supermin.ml
@@ -95,6 +95,7 @@ let main ()      let packager_config = ref "" in
     let use_installed = ref false in
     let size = ref None in
+    let include_packagelist = ref false in
 
     let set_debug () = incr debug in
 
@@ -140,6 +141,8 @@ let main ()        "--format",  Arg.String
set_format,     ditto;
       "--host-cpu", Arg.Set_string host_cpu,  "ARCH Set host CPU
architecture";
       "--if-newer", Arg.Set if_newer,             " Only build
if needed";
+      "--include-packagelist", Arg.Set include_packagelist,
+                                              " Add a file with the list
of packages";
       "--list-drivers", Arg.Unit display_drivers, " Display list
of drivers and exit";
       "--lock",    Arg.Set_string lockfile,   "LOCKFILE Use a
lock file";
       "--names",   Arg.Unit error_supermin_5, " Give an error
for people needing supermin 4";
@@ -170,6 +173,7 @@ let main ()        match !packager_config with ""
-> None | s -> Some s in
     let use_installed = !use_installed in
     let size = !size in
+    let include_packagelist = !include_packagelist in
 
     let format        match mode, !format with
@@ -194,7 +198,8 @@ let main ()  
     debug, mode, if_newer, inputs, lockfile, outputdir,
     (copy_kernel, dtb_wildcard, format, host_cpu,
-     packager_config, tmpdir, use_installed, size) in
+     packager_config, tmpdir, use_installed, size,
+     include_packagelist) in
 
   if debug >= 1 then printf "supermin: version: %s\n"
Config.package_version;
 
@@ -202,7 +207,7 @@ let main ()     * This fails with an error if one could not
be located.
    *)
   let () -    let (_, _, _, _, packager_config, tmpdir, _, _) = args in
+    let (_, _, _, _, packager_config, tmpdir, _, _, _) = args in
     let settings = {
       debug = debug;
       tmpdir = tmpdir;
diff --git a/src/supermin.pod b/src/supermin.pod
index 53d1b11..298247c 100644
--- a/src/supermin.pod
+++ b/src/supermin.pod
@@ -217,6 +217,17 @@ directory.
 
 See also I<--lock> below.
 
+=item B<"--include-packagelist>
+
+(I<--build> mode only)
+
+Add a F</packagelist> file inside the generated chroot or ext2
+filesystem, containing a sorted list of all the packages used to
+build the appliance.
+
+Mostly useful for debugging, as it makes it easier to find out e.g.
+which version of a package was copied in the appliance.
+
 =item B<--list-drivers>
 
 List the package manager drivers compiled into supermin, and whether
-- 
2.1.0

On Wed, Dec 02, 2015 at 07:11:33PM +0100, Pino Toscano
wrote:
> Hi,
> 
> to ease debugging issues with appliances (e.g. when used in libguestfs),
> using --include-packagelist will add a file containing the list of all
> the packages used.
ACK series.

Thanks Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v