Libguestfs - Aug 2015 - [PATCH 1/2] mllib: add normalize_arch helper

Small helper to normalize an architecture string, so it is easier to
compare them and check for compatibilities.

Make use of it in guest_arch_compatible, simplifying it.
---
 mllib/common_utils.ml  | 12 ++++++++++--
 mllib/common_utils.mli |  5 +++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index f9e8996..ca6d470 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -730,13 +730,21 @@ let rec mkdir_p path permissions      mkdir_p
(Filename.dirname path) permissions;
     Unix.mkdir path permissions
 
+let normalize_arch = function
+  | "i386" | "i486" | "i586" | "i686"
-> "i386"
+  | "amd64" | "x86_64" -> "x86_64"
+  | "powerpc" | "ppc" -> "ppc"
+  | arch -> arch
+
 (* 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
+  let own = normalize_arch Config.host_cpu in
+  let guest_arch = normalize_arch guest_arch in
+  match own, guest_arch with
   | x, y when x = y -> true
-  | "x86_64",
("i386"|"i486"|"i586"|"i686") -> true
+  | "x86_64", "i386" -> true
   | _ -> false
 
 (** Return the last part of a string, after the specified separator. *)
diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli
index 16834f7..ac232af 100644
--- a/mllib/common_utils.mli
+++ b/mllib/common_utils.mli
@@ -175,6 +175,11 @@ val qemu_input_filename : string -> string
 val mkdir_p : string -> int -> unit
 (** Creates a directory, and its parents if missing. *)
 
+val normalize_arch : string -> string
+(** Normalize the architecture name, i.e. maps it into a defined
+    identifier for it -- e.g. i386, i486, i586, and i686 are
+    normalized as i386. *)
+
 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? *)
-- 
2.1.0

Normalize the target architecture, and also each architecture when
checking for a compatible image.

This sort of reverts the effects of
commit 8864c47b94cf44ac2d0d8d4b5019073ad1da121b, but at least it is
possible to build e.g. Debian-based amd64 images on any x86_64 system
without being considered as foreign architecture.
---
 builder/builder.ml | 2 +-
 builder/cmdline.ml | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index adfa412..d59380b 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -238,7 +238,7 @@ let main ()    let item      try List.find (
       fun (name, { Index.arch = a }) ->
-        name = arg && arch = a
+        name = arg && arch = normalize_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 eca8c89..49435ae 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -295,6 +295,7 @@ read the man page virt-builder(1).
     match arch with
     | "" -> Config.host_cpu
     | arch -> arch in
+  let arch = normalize_arch arch in
 
   (* If user didn't elect any root password, that means we set a random
    * root password.
-- 
2.1.0

On Tue, Aug 11, 2015 at 03:45:11PM +0200, Pino Toscano
wrote:
> Small helper to normalize an architecture string, so it is easier to
> compare them and check for compatibilities.
> 
> Make use of it in guest_arch_compatible, simplifying it.
Have a look at:
8864c47b94cf44ac2d0d8d4b5019073ad1da121b
>  mllib/common_utils.ml  | 12 ++++++++++--
>  mllib/common_utils.mli |  5 +++++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
> index f9e8996..ca6d470 100644
> --- a/mllib/common_utils.ml
> +++ b/mllib/common_utils.ml
> @@ -730,13 +730,21 @@ let rec mkdir_p path permissions >      mkdir_p
(Filename.dirname path) permissions;
>      Unix.mkdir path permissions
>  
> +let normalize_arch = function
> +  | "i386" | "i486" | "i586" |
"i686" -> "i386"
> +  | "amd64" | "x86_64" -> "x86_64"
Commit 8864c47b had "x64" also (added by you in b1cf6246).
> +  | "powerpc" | "ppc" -> "ppc"
> +  | arch -> arch
> +
>  (* 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
> +  let own = normalize_arch Config.host_cpu in
> +  let guest_arch = normalize_arch guest_arch in
> +  match own, guest_arch with
>    | x, y when x = y -> true
> -  | "x86_64",
("i386"|"i486"|"i586"|"i686") -> true
> +  | "x86_64", "i386" -> true
>    | _ -> false
Commit 8864c47b may have got this wrong.  You cannot, for example, run
'dnf install' in an i686 guest, as it tries to install x86_64
packages.  Try this command for example:

  virt-builder --arch i686 fedora-22 --install net-tools

There doesn't seem to be a way to force dnf to use another basearch.

I wonder also if instead of a normalize function, we want a compare
function.  Or maybe we want both?  A compare function is safer, maybe,
since it preserves the actual arch string.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Tuesday 11 August 2015 15:05:04 Richard W.M. Jones
wrote:
> On Tue, Aug 11, 2015 at 03:45:11PM +0200, Pino Toscano wrote:
> > Small helper to normalize an architecture string, so it is easier to
> > compare them and check for compatibilities.
> > 
> > Make use of it in guest_arch_compatible, simplifying it.
> 
> Have a look at:
> 8864c47b94cf44ac2d0d8d4b5019073ad1da121b
Yes, this is what patch #2 partially reverts (and it's mentioned
there).
> >  mllib/common_utils.ml  | 12 ++++++++++--
> >  mllib/common_utils.mli |  5 +++++
> >  2 files changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
> > index f9e8996..ca6d470 100644
> > --- a/mllib/common_utils.ml
> > +++ b/mllib/common_utils.ml
> > @@ -730,13 +730,21 @@ let rec mkdir_p path permissions > >     
mkdir_p (Filename.dirname path) permissions;
> >      Unix.mkdir path permissions
> >  
> > +let normalize_arch = function
> > +  | "i386" | "i486" | "i586" |
"i686" -> "i386"
> > +  | "amd64" | "x86_64" -> "x86_64"
> 
> Commit 8864c47b had "x64" also (added by you in b1cf6246).
IIRC x64 was for 64bit Windows guests, but for now that is not needed.
> > +  | "powerpc" | "ppc" -> "ppc"
> > +  | arch -> arch
> > +
> >  (* 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
> > +  let own = normalize_arch Config.host_cpu in
> > +  let guest_arch = normalize_arch guest_arch in
> > +  match own, guest_arch with
> >    | x, y when x = y -> true
> > -  | "x86_64",
("i386"|"i486"|"i586"|"i686") -> true
> > +  | "x86_64", "i386" -> true
> >    | _ -> false
> 
> Commit 8864c47b may have got this wrong.  You cannot, for example, run
> 'dnf install' in an i686 guest, as it tries to install x86_64
> packages.  Try this command for example:
> 
>   virt-builder --arch i686 fedora-22 --install net-tools
> 
> There doesn't seem to be a way to force dnf to use another basearch.
I would say that this is not a problem of guest_arch_compatible itself
(running programs is possible as it is supposed to be), but more like
some dnf behaviour.  Maybe we should pass the architecture when
installing using yum/dnf and the appliance and guest architecture are
not the same?
> I wonder also if instead of a normalize function, we want a compare
> function.  Or maybe we want both?  A compare function is safer, maybe,
> since it preserves the actual arch string.
I guess it depends on how it is used. In the way patch #2 uses it in
virt-builder, the target architecture does not need to be preserved
exactly -- it is used only for checking among the templates, whose
architecture is normalized on the fly when searching, not changing what
is the actual value coming from the index.

Thanks,
-- 
Pino Toscano