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