Pino Toscano
2015-Aug-11 13:45 UTC
[Libguestfs] [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
Pino Toscano
2015-Aug-11 13:45 UTC
[Libguestfs] [PATCH 2/2] builder: normalize architectures
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
Richard W.M. Jones
2015-Aug-11 14:05 UTC
Re: [Libguestfs] [PATCH 1/2] mllib: add normalize_arch helper
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 > | _ -> falseCommit 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
Pino Toscano
2015-Aug-11 14:20 UTC
Re: [Libguestfs] [PATCH 1/2] mllib: add normalize_arch helper
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: > 8864c47b94cf44ac2d0d8d4b5019073ad1da121bYes, 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