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: > 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.