Libguestfs - May 2015 - [PATCH 1/2] customize: Give a clear error message if host_cpu not compatible with guest arch.

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

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.
---
 customize/customize_run.ml | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index 08cff0b..0f1d72a 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -30,6 +30,19 @@ let run ~verbose ~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 +    match Config.host_cpu, guest_arch with
+    | x, y when x = y -> true
+    | "x86_64",
("i386"|"i486"|"i586"|"i686") -> true
+    (* In theory aarch64 host could run armv7l commands, but it won't
+     * work without libvirt changes.  XXX
+     *)
+    | _ -> false in
+
   (* Based on the guest type, choose a log file location. *)
   let logfile      match g#inspect_get_type root with
@@ -53,6 +66,10 @@ let run ~verbose ~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
-- 
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 0f1d72a..cd4616c 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -338,15 +338,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 Fri, May 15, 2015 at 10:08:09AM +0100, Richard W.M. Jones
wrote:
> +    (* In theory aarch64 host could run armv7l commands, but it won't
> +     * work without libvirt changes.  XXX
> +     *)
Actually this is not possible even in theory, at least, not on Fedora
or RHEL where we use 64K page size on aarch64 vs 4K page size on
armv7.  (It might work on SUSE & Debian, which uses 4K pages on both,
and where, especially for SUSE, they actually use and test running 32
bit binaries).

I will remove this comment.

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

On Friday 15 May 2015 10:08:09 Richard W.M. Jones wrote:
> 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.
> ---
>  customize/customize_run.ml | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/customize/customize_run.ml b/customize/customize_run.ml
> index 08cff0b..0f1d72a 100644
> --- a/customize/customize_run.ml
> +++ b/customize/customize_run.ml
> @@ -30,6 +30,19 @@ let run ~verbose ~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 > +    match Config.host_cpu, guest_arch
with
> +    | x, y when x = y -> true
> +    | "x86_64",
("i386"|"i486"|"i586"|"i686") -> true
> +    (* In theory aarch64 host could run armv7l commands, but it won't
> +     * work without libvirt changes.  XXX
> +     *)
> +    | _ -> false in
Isn't this close to what Architecture.arch_is_compatible in
builder/architecture.ml does? It seems so to me, it would just require
filter_arch to map
("i386"|"i486"|"i586"|"i686") as
"i386".

If so, I guess it might be better to adapt that and move Architecture
(with or without Uname) to mllib.

-- 
Pino Toscano

On Friday 15 May 2015 10:08:10 Richard W.M. Jones wrote:
> ---
>  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 0f1d72a..cd4616c 100644
> --- a/customize/customize_run.ml
> +++ b/customize/customize_run.ml
> @@ -338,15 +338,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"
> +    )
LGTM.

-- 
Pino Toscano