Libguestfs - May 2015 - [PATCH v2 0/2] customize: Allow --selinux-relabel flag to work on cross-architecture builds.

Fixes
https://bugzilla.redhat.com/show_bug.cgi?id=1212807

Since v1:
 - Combine the virt-builder detection code into virt-customize.
 - Enables us to delete Architecture and Uname modules completely.

Rich.

In cases where we are asked to run commands in the guest (eg.  options
such as --run-command or --install), give a clear error in the cases
where the guest arch is not compatible with the host arch.

Similar code existed in virt-builder, but I have removed that.  Users
will still get an error message, it will just happen a bit later on.

There is a slight change in semantics here, in that architectures are
no longer normalized when matching, but that's probably fine since
`virt-builder -l' prints the exact arch string that people should use.
---
 builder/Makefile.am        |  8 ++-----
 builder/architecture.ml    | 41 ---------------------------------
 builder/builder.ml         |  2 +-
 builder/cmdline.ml         | 21 ++---------------
 builder/uname-c.c          | 57 ----------------------------------------------
 builder/uname.ml           | 27 ----------------------
 builder/uname.mli          | 28 -----------------------
 customize/customize_run.ml | 10 ++++++++
 mllib/common_utils.ml      |  9 ++++++++
 mllib/common_utils.mli     |  4 ++++
 po/POTFILES                |  1 -
 po/POTFILES-ml             |  2 --
 12 files changed, 28 insertions(+), 182 deletions(-)
 delete mode 100644 builder/architecture.ml
 delete mode 100644 builder/uname-c.c
 delete mode 100644 builder/uname.ml
 delete mode 100644 builder/uname.mli

diff --git a/builder/Makefile.am b/builder/Makefile.am
index 182f5a4..28b2adf 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -47,15 +47,12 @@ SOURCES_MLI = \
 	pxzcat.mli \
 	setlocale.mli \
 	sigchecker.mli \
-	sources.mli \
-	uname.mli
+	sources.mli
 
 SOURCES_ML = \
 	utils.ml \
 	pxzcat.ml \
 	setlocale.ml \
-	uname.ml \
-	architecture.ml \
 	ini_reader.ml \
 	paths.ml \
 	languages.ml \
@@ -82,8 +79,7 @@ SOURCES_C = \
 	index-parse.c \
 	index-parser-c.c \
 	pxzcat-c.c \
-	setlocale-c.c \
-	uname-c.c
+	setlocale-c.c
 
 man_MANS  noinst_DATA diff --git a/builder/architecture.ml
b/builder/architecture.ml
deleted file mode 100644
index 59c1cf6..0000000
--- a/builder/architecture.ml
+++ /dev/null
@@ -1,41 +0,0 @@
-(* virt-builder
- * Copyright (C) 2014 Red Hat Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *)
-
-open Common_gettext.Gettext
-open Common_utils
-
-open Unix
-
-let filter_arch = function
-  | "amd64" | "x86_64" | "x64" ->
"x86_64"
-  | "powerpc" | "ppc" -> "ppc"
-  | arch -> arch
-
-let arch_is_compatible nativearch otherarch -  let nativearch = filter_arch
nativearch in
-  let otherarch = filter_arch otherarch in
-  match nativearch, otherarch with
-  | a, b when a = b -> true
-  | "x86_64", "i386" -> true
-  | "ppc64", "ppc" -> true
-  | "sparc64", "sparc" -> true
-  | a, b -> false
-
-let current_arch -  try filter_arch ((Uname.uname ()).Uname.machine)
-  with Unix_error _ -> "unknown"
diff --git a/builder/builder.ml b/builder/builder.ml
index 260281c..663aeef 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -227,7 +227,7 @@ let main ()    let item      try List.find (
       fun (name, { Index_parser.arch = a }) ->
-        name = arg && arch = Architecture.filter_arch a
+        name = arg && arch = a
     ) index
     with Not_found ->
       error (f_"cannot find os-version '%s' with architecture
'%s'.\nUse --list to list available guest types.")
diff --git a/builder/cmdline.ml b/builder/cmdline.ml
index ed68e91..7aa0c45 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -296,25 +296,8 @@ read the man page virt-builder(1).
   (* Check the architecture. *)
   let arch      match arch with
-    | "" -> Architecture.current_arch
-    | arch ->
-      let target_arch = Architecture.filter_arch arch in
-      if Architecture.arch_is_compatible Architecture.current_arch target_arch
<> true then (
-        let requires_execute_on_guest = List.exists (
-          function
-          | `Command _ | `InstallPackages _ | `Script _ | `Update -> true
-          | `Delete _ | `Edit _ | `FirstbootCommand _ | `FirstbootPackages _
-          | `FirstbootScript _ | `Hostname _ | `Link _ | `Mkdir _
-          | `Password _ | `RootPassword _ | `Scrub _ | `SSHInject _
-          | `Timezone _ | `Truncate _ | `TruncateRecursive _
-          | `Upload _ | `Write _ | `Chmod _
-          | `CommandsFromFile _ | `CopyIn _ | `Copy _ | `Move _
-          | `Touch _ -> false
-        ) ops.ops in
-        if requires_execute_on_guest then
-          error (f_"sorry, cannot run commands on a guest with a different
architecture");
-      );
-      target_arch in
+    | "" -> Config.host_cpu
+    | arch -> arch in
 
   (* If user didn't elect any root password, that means we set a random
    * root password.
diff --git a/builder/uname-c.c b/builder/uname-c.c
deleted file mode 100644
index fc63233..0000000
--- a/builder/uname-c.c
+++ /dev/null
@@ -1,57 +0,0 @@
-/* virt-builder
- * Copyright (C) 2014 Red Hat Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
- */
-
-#include <config.h>
-
-#include <errno.h>
-#include <sys/utsname.h>
-
-#include <caml/alloc.h>
-#include <caml/fail.h>
-#include <caml/memory.h>
-#include <caml/mlvalues.h>
-
-#ifdef HAVE_CAML_UNIXSUPPORT_H
-#include <caml/unixsupport.h>
-#else
-#define Nothing ((value) 0)
-extern void unix_error (int errcode, char * cmdname, value arg) Noreturn;
-#endif
-
-extern value virt_builder_uname (value unit);
-
-value
-virt_builder_uname (value unit)
-{
-  CAMLparam0 ();
-  CAMLlocal1 (rv);
-  struct utsname u;
-
-  if (uname (&u) < 0)
-    unix_error (errno, (char *) "uname", Val_int (0));
-
-  rv = caml_alloc (5, 0);
-
-  Store_field (rv, 0, caml_copy_string (u.sysname));
-  Store_field (rv, 1, caml_copy_string (u.nodename));
-  Store_field (rv, 2, caml_copy_string (u.release));
-  Store_field (rv, 3, caml_copy_string (u.version));
-  Store_field (rv, 4, caml_copy_string (u.machine));
-
-  CAMLreturn (rv);
-}
diff --git a/builder/uname.ml b/builder/uname.ml
deleted file mode 100644
index c370c2c..0000000
--- a/builder/uname.ml
+++ /dev/null
@@ -1,27 +0,0 @@
-(* virt-builder
- * Copyright (C) 2014 Red Hat Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *)
-
-type uname_struct = {
-  sysname : string;
-  nodename : string;
-  release : string;
-  version : string;
-  machine : string;
-}
-
-external uname : unit -> uname_struct = "virt_builder_uname"
diff --git a/builder/uname.mli b/builder/uname.mli
deleted file mode 100644
index aea441b..0000000
--- a/builder/uname.mli
+++ /dev/null
@@ -1,28 +0,0 @@
-(* virt-builder
- * Copyright (C) 2014 Red Hat Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *)
-
-type uname_struct = {
-  sysname : string;
-  nodename : string;
-  release : string;
-  version : string;
-  machine : string;
-}
-
-val uname : unit -> uname_struct
-(** [uname] Tiny wrapper to the C [uname]. *)
diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index 4200170..f3924c1 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -30,6 +30,12 @@ let run ~quiet (g : Guestfs.guestfs) root (ops : ops)    (*
Timestamped messages in ordinary, non-debug non-quiet mode. *)
   let msg fs = make_message_function ~quiet fs in
 
+  (* Is the host_cpu compatible with the guest arch?  ie. Can we
+   * run commands in this guest?
+   *)
+  let guest_arch = g#inspect_get_arch root in
+  let guest_arch_compatible = guest_arch_compatible guest_arch in
+
   (* Based on the guest type, choose a log file location. *)
   let logfile      match g#inspect_get_type root with
@@ -53,6 +59,10 @@ let run ~quiet (g : Guestfs.guestfs) root (ops : ops)  
   (* Useful wrapper for scripts. *)
   let do_run ~display cmd +    if not guest_arch_compatible then
+      error (f_"host cpu (%s) and guest arch (%s) are not compatible, so
you cannot use command line options that involve running commands in the guest. 
Use --firstboot scripts instead.")
+            Config.host_cpu guest_arch;
+
     (* Add a prologue to the scripts:
      * - Pass environment variables through from the host.
      * - Send stdout and stderr to a log file so we capture all output
diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 085089a..d61335b 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -715,3 +715,12 @@ let rec mkdir_p path permissions       * directory. *)
     mkdir_p (Filename.dirname path) permissions;
     Unix.mkdir path permissions
+
+(* Are guest arch and host_cpu compatible, in terms of being able
+ * to run commands in the libguestfs appliance?
+ *)
+let guest_arch_compatible guest_arch +  match Config.host_cpu, guest_arch with
+  | x, y when x = y -> true
+  | "x86_64",
("i386"|"i486"|"i586"|"i686") -> true
+  | _ -> false
diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli
index dcc9d42..ff00218 100644
--- a/mllib/common_utils.mli
+++ b/mllib/common_utils.mli
@@ -165,3 +165,7 @@ val qemu_input_filename : string -> string
 
 val mkdir_p : string -> int -> unit
 (** Creates a directory, and its parents if missing. *)
+
+val guest_arch_compatible : string -> bool
+(** Are guest arch and host_cpu compatible, in terms of being able
+    to run commands in the libguestfs appliance? *)
diff --git a/po/POTFILES b/po/POTFILES
index 91005d4..00143db 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -6,7 +6,6 @@ builder/index-struct.c
 builder/index-validate.c
 builder/pxzcat-c.c
 builder/setlocale-c.c
-builder/uname-c.c
 cat/cat.c
 cat/filesystems.c
 cat/log.c
diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index 7f4e096..3fc60e5 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -1,4 +1,3 @@
-builder/architecture.ml
 builder/builder.ml
 builder/cache.ml
 builder/cmdline.ml
@@ -13,7 +12,6 @@ builder/pxzcat.ml
 builder/setlocale.ml
 builder/sigchecker.ml
 builder/sources.ml
-builder/uname.ml
 builder/utils.ml
 customize/crypt.ml
 customize/customize_cmdline.ml
-- 
2.3.1

---
 customize/customize_run.ml | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index f3924c1..e44f198 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -331,15 +331,19 @@ exec >>%s 2>&1
 
   if ops.flags.selinux_relabel then (
     msg (f_"SELinux relabelling");
-    let cmd = sprintf "
-      if load_policy && fixfiles restore; then
-        rm -f /.autorelabel
-      else
-        touch /.autorelabel
-        echo '%s: SELinux relabelling failed, will relabel at boot
instead.'
-      fi
-    " prog in
-    do_run ~display:"load_policy && fixfiles restore" cmd
+    if guest_arch_compatible then (
+      let cmd = sprintf "
+        if load_policy && fixfiles restore; then
+          rm -f /.autorelabel
+        else
+          touch /.autorelabel
+          echo '%s: SELinux relabelling failed, will relabel at boot
instead.'
+        fi
+      " prog in
+      do_run ~display:"load_policy && fixfiles restore" cmd
+    ) else (
+      g#touch "/.autorelabel"
+    )
   );
 
   (* Clean up the log file:
-- 
2.3.1

On Friday 15 May 2015 15:24:33 Richard W.M. Jones wrote:
> Fixes
> https://bugzilla.redhat.com/show_bug.cgi?id=1212807
> 
> Since v1:
>  - Combine the virt-builder detection code into virt-customize.
>  - Enables us to delete Architecture and Uname modules completely.
LGTM.

Thanks for the refactoring,

-- 
Pino Toscano