Pino Toscano
2014-Mar-10 13:28 UTC
[Libguestfs] [PATCH] builder: complete architecture handling
Add the possibility to choose which architecture use to build the wanted image (--arch). Since this implies that running commands on the guest is usually not possible when the architecture is different than the host one, another new option (--allow-foreign-arch-ops) allows to run commands nevertheless. The caching scheme is adapted to account for the architecture (with --print-cache showing the architecture as well). Furthermore, the short output of --list shows the architecture as well, like the other --list outputs do. --- builder/Makefile.am | 9 +++++- builder/architecture.ml | 32 +++++++++++++++++++++ builder/builder.ml | 28 ++++++++++-------- builder/cmdline.ml | 29 +++++++++++++++++-- builder/downloader.ml | 8 +++--- builder/downloader.mli | 10 +++---- builder/index_parser.ml | 22 ++++++++++---- builder/list_entries.ml | 2 ++ builder/test-virt-builder-list.sh | 14 ++++----- builder/test-virt-builder.sh | 1 + builder/uname-c.c | 60 +++++++++++++++++++++++++++++++++++++++ builder/uname.ml | 27 ++++++++++++++++++ builder/uname.mli | 28 ++++++++++++++++++ builder/virt-builder.pod | 35 +++++++++++++++++++---- po/POTFILES | 1 + po/POTFILES-ml | 2 ++ 16 files changed, 264 insertions(+), 44 deletions(-) create mode 100644 builder/architecture.ml create mode 100644 builder/uname-c.c create mode 100644 builder/uname.ml create mode 100644 builder/uname.mli diff --git a/builder/Makefile.am b/builder/Makefile.am index 387913c..5a978e3 100644 --- a/builder/Makefile.am +++ b/builder/Makefile.am @@ -38,6 +38,7 @@ CLEANFILES = *~ *.cmi *.cmo *.cmx *.cmxa *.o virt-builder # Alphabetical order. SOURCES = \ + architecture.ml \ builder.ml \ cmdline.ml \ downloader.mli \ @@ -61,7 +62,10 @@ SOURCES = \ sigchecker.mli \ sigchecker.ml \ sources.mli \ - sources.ml + sources.ml \ + uname.ml \ + uname.mli \ + uname-c.c man_MANS noinst_DATA @@ -99,6 +103,9 @@ deps = \ pxzcat.cmx \ setlocale-c.o \ setlocale.cmx \ + uname-c.o \ + uname.cmx \ + architecture.cmx \ ini_reader.cmx \ paths.cmx \ get_kernel.cmx \ diff --git a/builder/architecture.ml b/builder/architecture.ml new file mode 100644 index 0000000..2aa7e7e --- /dev/null +++ b/builder/architecture.ml @@ -0,0 +1,32 @@ +(* 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" + | "armhfp" | "armhf" -> "armhf" + | arch -> arch + +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 1800f2d..ed55de1 100644 --- a/builder/builder.ml +++ b/builder/builder.ml @@ -38,9 +38,9 @@ let () = Random.self_init () let main () (* Command line argument parsing - see cmdline.ml. *) let mode, arg, - attach, cache, check_signature, curl, debug, delete, delete_on_failure, - edit, firstboot, run, format, gpg, hostname, install, list_format, links, - memsize, mkdirs, + arch, attach, cache, check_signature, curl, debug, delete, + delete_on_failure, edit, firstboot, run, format, gpg, hostname, install, + list_format, links, memsize, mkdirs, network, output, password_crypto, quiet, root_password, scrub, scrub_logfile, selinux_relabel, size, smp, sources, sync, timezone, update, upload, writes @@ -172,11 +172,11 @@ let main () | Some cachedir -> printf (f_"cache directory: %s\n") cachedir; List.iter ( - fun (name, { Index_parser.revision = revision; hidden = hidden }) -> + fun (name, { Index_parser.revision = revision; arch = arch; hidden = hidden }) -> if not hidden then ( - let filename = Downloader.cache_of_name cachedir name revision in + let filename = Downloader.cache_of_name cachedir name arch revision in let cached = Sys.file_exists filename in - printf "%-24s %s\n" name + printf "%-24s %-10s %s\n" name arch (if cached then s_"cached" else (*s_*)"no") ) ) index @@ -193,7 +193,7 @@ let main () List.iter ( fun (name, { Index_parser.revision = revision; file_uri = file_uri }) -> - let template = name, revision in + let template = name, arch, revision in msg (f_"Downloading: %s") file_uri; let progress_bar = not quiet in ignore (Downloader.download ~prog downloader ~template ~progress_bar @@ -205,12 +205,16 @@ let main () | (`Install|`Notes) as mode -> mode in (* Which os-version (ie. index entry)? *) - let entry - try List.assoc arg index + let item + try List.find ( + fun (name, { Index_parser.arch = a }) -> + name = arg && arch = Architecture.filter_arch a + ) index with Not_found -> - eprintf (f_"%s: cannot find os-version '%s'.\nUse --list to list available guest types.\n") - prog arg; + eprintf (f_"%s: cannot find os-version '%s' with architecture '%s'.\nUse --list to list available guest types.\n") + prog arg arch; exit 1 in + let entry = snd item in let sigchecker = entry.Index_parser.sigchecker in (match mode with @@ -234,7 +238,7 @@ let main () let template let template, delete_on_exit let { Index_parser.revision = revision; file_uri = file_uri } = entry in - let template = arg, revision in + let template = arg, arch, revision in msg (f_"Downloading: %s") file_uri; let progress_bar = not quiet in Downloader.download ~prog downloader ~template ~progress_bar file_uri in diff --git a/builder/cmdline.ml b/builder/cmdline.ml index 6e8bfd8..0dd0ad2 100644 --- a/builder/cmdline.ml +++ b/builder/cmdline.ml @@ -44,6 +44,9 @@ let parse_cmdline () let print_cache_mode () = mode := `Print_cache in let delete_cache_mode () = mode := `Delete_cache in + let arch = ref "" in + let allow_foreign_arch_ops = ref false in + let attach = ref [] in let attach_format = ref None in let set_attach_format = function @@ -221,6 +224,9 @@ let parse_cmdline () let ditto = " -\"-" in let argspec = Arg.align [ + "--allow-foreign-arch-ops", Arg.Set allow_foreign_arch_ops, + " " ^ s_"Allow executing commands even on non-native output architectures"; + "--arch", Arg.Set_string arch, "arch" ^ " " ^ s_"Set the output architecture"; "--attach", Arg.String attach_disk, "iso" ^ " " ^ s_"Attach data disk/ISO during install"; "--attach-format", Arg.String set_attach_format, "format" ^ " " ^ s_"Set attach disk format"; @@ -319,6 +325,8 @@ read the man page virt-builder(1). (* Dereference options. *) let args = List.rev !args in let mode = !mode in + let arch = !arch in + let allow_foreign_arch_ops = !allow_foreign_arch_ops in let attach = List.rev !attach in let cache = !cache in let check_signature = !check_signature in @@ -431,10 +439,25 @@ read the man page virt-builder(1). (* Combine the sources and fingerprints into a single list of pairs. *) List.combine sources fingerprints in + (* Check the architecture. *) + let arch + match arch with + | "" -> Architecture.current_arch + | arch -> + let target_arch = Architecture.filter_arch arch in + if Architecture.current_arch <> target_arch && allow_foreign_arch_ops <> true then ( + if install <> [] || run <> [] || update then ( + eprintf (f_"%s: sorry, cannot run commands on a guest with a different architecture\n") + prog; + exit 1 + ); + ); + target_arch in + mode, arg, - attach, cache, check_signature, curl, debug, delete, delete_on_failure, - edit, firstboot, run, format, gpg, hostname, install, list_format, links, - memsize, mkdirs, + arch, attach, cache, check_signature, curl, debug, delete, + delete_on_failure, edit, firstboot, run, format, gpg, hostname, install, + list_format, links, memsize, mkdirs, network, output, password_crypto, quiet, root_password, scrub, scrub_logfile, selinux_relabel, size, smp, sources, sync, timezone, update, upload, writes diff --git a/builder/downloader.ml b/builder/downloader.ml index e386c06..e23cb37 100644 --- a/builder/downloader.ml +++ b/builder/downloader.ml @@ -25,8 +25,8 @@ open Printf let quote = Filename.quote let (//) = Filename.concat -let cache_of_name cachedir name revision - cachedir // sprintf "%s.%d" name revision +let cache_of_name cachedir name arch revision + cachedir // sprintf "%s.%s.%d" name arch revision type uri = string type filename = string @@ -51,14 +51,14 @@ let rec download ~prog t ?template ?progress_bar uri download_to ~prog t ?progress_bar uri tmpfile; (tmpfile, true) - | Some (name, revision) -> + | Some (name, arch, revision) -> match t.cache with | None -> (* Not using the cache at all? *) download t ~prog ?progress_bar uri | Some cachedir -> - let filename = cache_of_name cachedir name revision in + let filename = cache_of_name cachedir name arch revision in (* Is the requested template name + revision in the cache already? * If not, download it. diff --git a/builder/downloader.mli b/builder/downloader.mli index 62d500d..7b26ba3 100644 --- a/builder/downloader.mli +++ b/builder/downloader.mli @@ -18,7 +18,7 @@ (** This module is a wrapper around curl, plus local caching. *) -val cache_of_name : string -> string -> int -> string +val cache_of_name : string -> string -> string -> int -> string (** [cache_of_name cachedir name revision] returns the filename of the cached file. (Note: It doesn't check if the filename exists, this is just a simple string transformation). *) @@ -32,15 +32,15 @@ type t val create : debug:bool -> curl:string -> cache:string option -> t (** Create the abstract type. *) -val download : prog:string -> t -> ?template:(string*int) -> ?progress_bar:bool -> uri -> (filename * bool) +val download : prog:string -> t -> ?template:(string*string*int) -> ?progress_bar:bool -> uri -> (filename * bool) (** Download the URI, returning the downloaded filename and a temporary file flag. The temporary file flag is [true] iff the downloaded file is temporary and should be deleted by the caller (otherwise it's in the cache and you shouldn't delete it). - For templates, you must supply [~template:(name, revision)]. This - causes the cache to be used (if possible). Name and revision are - used for cache control (see the man page for details). + For templates, you must supply [~template:(name, arch, revision)]. + This causes the cache to be used (if possible). Name, arch(itecture) + and revision are used for cache control (see the man page for details). If [~progress_bar:true] then display a progress bar if the file doesn't come from the cache. In debug mode, progress messages diff --git a/builder/index_parser.ml b/builder/index_parser.ml index 9fd9cda..472a6c7 100644 --- a/builder/index_parser.ml +++ b/builder/index_parser.ml @@ -123,16 +123,26 @@ let get_index ~prog ~debug ~downloader ~sigchecker source if delete_tmpfile then (try Unix.unlink tmpfile with _ -> ()); - (* Check for repeated os-version names. *) + (* Check for repeated os-version+arch combination. *) + let name_arch_map = List.map ( + fun (n, fields) -> + let rec find_arch = function + | ("arch", None, value) :: y -> value + | _ :: y -> find_arch y + | [] -> "" + in + n, (find_arch fields) + ) sections in let nseen = Hashtbl.create 13 in List.iter ( - fun (n, _) -> - if Hashtbl.mem nseen n then ( - eprintf (f_"virt-builder: index is corrupt: os-version '%s' appears two or more times\n") n; + fun (n, arch) -> + let id = n, arch in + if Hashtbl.mem nseen id then ( + eprintf (f_"virt-builder: index is corrupt: os-version '%s' with architecture '%s' appears two or more times\n") n arch; corrupt_file () ); - Hashtbl.add nseen n true - ) sections; + Hashtbl.add nseen id true + ) name_arch_map; (* Check for repeated fields. *) List.iter ( diff --git a/builder/list_entries.ml b/builder/list_entries.ml index 476bf14..75149e7 100644 --- a/builder/list_entries.ml +++ b/builder/list_entries.ml @@ -48,9 +48,11 @@ let rec list_entries ~list_format ~sources index and list_entries_short index List.iter ( fun (name, { Index_parser.printable_name = printable_name; + arch = arch; hidden = hidden }) -> if not hidden then ( printf "%-24s" name; + printf " %-10s" arch; (match printable_name with | None -> () | Some s -> printf " %s" s diff --git a/builder/test-virt-builder-list.sh b/builder/test-virt-builder-list.sh index 2f9b319..1f62838 100755 --- a/builder/test-virt-builder-list.sh +++ b/builder/test-virt-builder-list.sh @@ -27,13 +27,13 @@ export XDG_CONFIG_DIRS="$abs_builddir/test-config" short_list=$($VG ./virt-builder --no-check-signature --no-cache --list) -if [ "$short_list" != "phony-debian Phony Debian -phony-fedora Phony Fedora -phony-fedora-qcow2 Phony Fedora qcow2 -phony-fedora-qcow2-uncompressed Phony Fedora qcow2 uncompressed -phony-fedora-no-format Phony Fedora -phony-ubuntu Phony Ubuntu -phony-windows Phony Windows" ]; then +if [ "$short_list" != "phony-debian x86_64 Phony Debian +phony-fedora x86_64 Phony Fedora +phony-fedora-qcow2 x86_64 Phony Fedora qcow2 +phony-fedora-qcow2-uncompressed x86_64 Phony Fedora qcow2 uncompressed +phony-fedora-no-format x86_64 Phony Fedora +phony-ubuntu x86_64 Phony Ubuntu +phony-windows x86_64 Phony Windows" ]; then echo "$0: unexpected --list output:" echo "$short_list" exit 1 diff --git a/builder/test-virt-builder.sh b/builder/test-virt-builder.sh index 85a7888..8841984 100755 --- a/builder/test-virt-builder.sh +++ b/builder/test-virt-builder.sh @@ -52,6 +52,7 @@ rm -f $output $VG ./virt-builder phony-fedora \ -v --no-cache --no-check-signature $no_network \ -o $output --size 2G --format $format \ + --arch x86_64 --allow-foreign-arch-ops \ --hostname test.example.com \ --timezone Europe/London \ --root-password password:123456 \ diff --git a/builder/uname-c.c b/builder/uname-c.c new file mode 100644 index 0000000..b4f2f61 --- /dev/null +++ b/builder/uname-c.c @@ -0,0 +1,60 @@ +/* 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. + */ + +/* This file handles the interface between the C/lex/yacc index file + * parser, and the OCaml world. See index_parser.ml for the OCaml + * type definition. + */ + +#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 + +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 new file mode 100644 index 0000000..c370c2c --- /dev/null +++ b/builder/uname.ml @@ -0,0 +1,27 @@ +(* 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 new file mode 100644 index 0000000..aea441b --- /dev/null +++ b/builder/uname.mli @@ -0,0 +1,28 @@ +(* 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/builder/virt-builder.pod b/builder/virt-builder.pod index 32c0961..708bfd0 100644 --- a/builder/virt-builder.pod +++ b/builder/virt-builder.pod @@ -15,6 +15,8 @@ virt-builder - Build virtual machine images quickly virt-builder os-version [-o|--output DISKIMAGE] [--size SIZE] [--format raw|qcow2] + [--allow-foreign-arch-ops] + [--arch ARCHITECTURE] [--attach ISOFILE] [--root-password SELECTOR] [--hostname HOSTNAME] @@ -81,7 +83,9 @@ are any installation notes: virt-builder fedora-20 -will build a Fedora 20 image. This will have all default +will build a Fedora 20 image for the same architecture as virt-builder +(so running it from an i386 installation will try to build an i386 +image, if available). This will have all default configuration (minimal size, no user accounts, random root password, only the bare minimum installed software, etc.). @@ -109,6 +113,10 @@ As above, but the output size will be 20 GB. The guest OS is resized as it is copied to the output (automatically, using L<virt-resize(1)>). + virt-builder fedora-20 --arch i386 + +As above, but using an i386 template, if available. + =head2 Setting the root password virt-builder fedora-20 --root-password file:/tmp/rootpw @@ -189,6 +197,21 @@ You can combine these options, and have multiple options of all types. Display help. +=item B<--allow-foreign-arch-ops> + +Forces the execution of commands on the target image even if its +architecture is different than the current host ones. + +See also L</ARCHITECTURE>. + +=item B<--arch> ARCHITECTURE + +Use the specified architecture for the output image. This means +there must be sources providing the requested template for the +requested architecture. + +See also L</ARCHITECTURE>. + =item B<--attach> ISOFILE During the customization phase, the given disk is attached to the @@ -1635,13 +1658,13 @@ highly recommended that you always create signed index and templates. Virt-builder can build a guest for any architecture no matter what the host architecture is. For example an x86-64 guest on an ARM host. -However certain options may not work correctly, specifically options +However certain options may not work, specifically options that require running commands in the guest during the build process: I<--install>, I<--update>, I<--run>, I<--run-command>. You may need -to replace these with their firstboot-equivalents. - -An x86-64 host building 32 bit i686 guests should work without any -special steps. +to replace these with their firstboot-equivalents, or specify +I<--allow-foreign-arch-ops> if the target architecture is compatible +with the host architecture (for example, you are building a 32 bit +i686 guest on a x86-64 host). =head2 SECURITY diff --git a/po/POTFILES b/po/POTFILES index 5b2ec57..5aeeb2e 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -6,6 +6,7 @@ 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/ls.c diff --git a/po/POTFILES-ml b/po/POTFILES-ml index 8725e5e..c96b451 100644 --- a/po/POTFILES-ml +++ b/po/POTFILES-ml @@ -1,3 +1,4 @@ +builder/architecture.ml builder/builder.ml builder/cmdline.ml builder/downloader.ml @@ -10,6 +11,7 @@ builder/pxzcat.ml builder/setlocale.ml builder/sigchecker.ml builder/sources.ml +builder/uname.ml mllib/common_gettext.ml mllib/common_utils.ml mllib/common_utils_tests.ml -- 1.8.3.1
Richard W.M. Jones
2014-Mar-11 10:09 UTC
Re: [Libguestfs] [PATCH] builder: complete architecture handling
On Mon, Mar 10, 2014 at 02:28:20PM +0100, Pino Toscano wrote:> Add the possibility to choose which architecture use to build the wanted > image (--arch). Since this implies that running commands on the guest is > usually not possible when the architecture is different than the host > one, another new option (--allow-foreign-arch-ops) allows to run > commands nevertheless.[..]> +let filter_arch = function > + | "amd64" | "x86_64" | "x64" -> "x86_64" > + | "powerpc" | "ppc" -> "ppc" > + | "armhfp" | "armhf" -> "armhf"I don't think "armhf" is a thing. AFAIK all ARM hard float architectures would be ARM v7 (or ARM v8, but that's separate). For reference, RPM's architectures are: $ rpm --eval %{arm} armv3l armv4b armv4l armv4tl armv5tel armv5tejl armv6l armv7l armv7hl armv7hnl (Of which no one cares about anything < armv7, and by the end of next year no one will care about anything < armv8).> diff --git a/builder/cmdline.ml b/builder/cmdline.ml > index 6e8bfd8..0dd0ad2 100644 > --- a/builder/cmdline.ml > +++ b/builder/cmdline.ml > @@ -44,6 +44,9 @@ let parse_cmdline () > let print_cache_mode () = mode := `Print_cache in > let delete_cache_mode () = mode := `Delete_cache in > > + let arch = ref "" in > + let allow_foreign_arch_ops = ref false inCan't we detect allow_foreign_arch_ops automatically? For normal uses of libguestfs, the only case that matters is a 32 bit requested guest architecture on a compatible 64 bit host (eg. ppc on ppc64 host). So a simple mapping table of host architecture -> compatible guest architectures, and drop the option.> diff --git a/builder/uname-c.c b/builder/uname-c.cI would have just done: List.hd (external_command "uname -m") but now you've written the FFI code, that's fine :-) The rest of the patch looks OK to me. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones 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
Pino Toscano
2014-Mar-11 14:42 UTC
Re: [Libguestfs] [PATCH] builder: complete architecture handling
On Tuesday 11 March 2014 10:09:45 Richard W.M. Jones wrote:> On Mon, Mar 10, 2014 at 02:28:20PM +0100, Pino Toscano wrote: > > Add the possibility to choose which architecture use to build the > > wanted image (--arch). Since this implies that running commands on > > the guest is usually not possible when the architecture is > > different than the host one, another new option > > (--allow-foreign-arch-ops) allows to run commands nevertheless. > > [..] > > > +let filter_arch = function > > + | "amd64" | "x86_64" | "x64" -> "x86_64" > > + | "powerpc" | "ppc" -> "ppc" > > + | "armhfp" | "armhf" -> "armhf" > > I don't think "armhf" is a thing. AFAIK all ARM hard float > architectures would be ARM v7 (or ARM v8, but that's separate). > > For reference, RPM's architectures are: > > $ rpm --eval %{arm} > armv3l armv4b armv4l armv4tl armv5tel armv5tejl armv6l armv7l armv7hl > armv7hnl > > (Of which no one cares about anything < armv7, and by the end of next > year no one will care about anything < armv8).In Debian do exists only the following two architectures: - "armel", arm EABI with only soft-float support, for older arm's - "armhf", EABI with hard-float support see also: https://wiki.debian.org/ArmHardFloatPort#Name_of_the_port I remember having seen "armhfp" for Fedora, so I though about flattening all together. Maybe it could be better to just ignore the complex arm situation for now, and let the users input the exact architecture they need.> > diff --git a/builder/cmdline.ml b/builder/cmdline.ml > > index 6e8bfd8..0dd0ad2 100644 > > --- a/builder/cmdline.ml > > +++ b/builder/cmdline.ml > > @@ -44,6 +44,9 @@ let parse_cmdline () > > > > let print_cache_mode () = mode := `Print_cache in > > let delete_cache_mode () = mode := `Delete_cache in > > > > + let arch = ref "" in > > + let allow_foreign_arch_ops = ref false in > > Can't we detect allow_foreign_arch_ops automatically? > > For normal uses of libguestfs, the only case that matters is a 32 bit > requested guest architecture on a compatible 64 bit host (eg. ppc on > ppc64 host). So a simple mapping table of host architecture -> > compatible guest architectures, and drop the option.Sounds sensible, will do.> > diff --git a/builder/uname-c.c b/builder/uname-c.c > > I would have just done: > > List.hd (external_command "uname -m") > > but now you've written the FFI code, that's fine :-)I'm starting to think that in the next stable series (say 1.27/1.28) we could require ExtUnix, and get rid of the FFI codes we currently have (setlocale, fsync, mkdtemp, statvfs, and now uname). -- Pino Toscano
Reasonably Related Threads
- [PATCH v2 0/2] customize: Allow --selinux-relabel flag to work on cross-architecture builds.
- [PATCH v12 0/3] virt-builder-repository tool
- [PATCH v13 0/3] virt-builder-repository
- [PATCH] builder: complete architecture handling
- [PATCH v11 0/8] virt-builder-repository