[PATCH v2 0/2] v2v: Use OVMF secure boot file (RHBZ#1367615).
First version was posted here: https://www.redhat.com/archives/libguestfs/2016-August/thread.html#00100 This is semantically the same as the first version. However I've split the patch up into two parts. In the first part, I factor out the UEFI paths so now they are created by the generator and written in the library and v2v/ directory directly, instead of the complex business of having a C database which is read in virt-v2v using C bindings. The second part is the same as before (but simplified because it now needs fewer changes). Rich.
Previously we had a list of UEFI paths in src/uefi.c, which was accessed in virt-v2v using a private (guestfs_int_*) API and some C binding code. This was clumsy and required the paths to be replicated in the virt-v2v unit tests. Instead just generate the list of paths from the generator, creating src/uefi.c and v2v/uefi.ml with the same content. Remove the C bindings and the virt-v2v unit tests associated with UEFI paths. --- .gitignore | 3 + generator/Makefile.am | 3 + generator/main.ml | 5 ++ generator/uefi.ml | 163 ++++++++++++++++++++++++++++++++++++++++ generator/uefi.mli | 21 ++++++ generator/utils.ml | 16 ++++ generator/utils.mli | 6 ++ src/guestfs-internal-frontend.h | 1 + src/uefi.c | 79 ------------------- v2v/Makefile.am | 4 + v2v/output_libvirt.ml | 2 +- v2v/output_qemu.ml | 2 +- v2v/utils-c.c | 47 ------------ v2v/utils.ml | 24 +----- v2v/utils.mli | 16 +--- v2v/v2v_unit_tests.ml | 31 -------- 16 files changed, 229 insertions(+), 194 deletions(-) create mode 100644 generator/uefi.ml create mode 100644 generator/uefi.mli delete mode 100644 src/uefi.c diff --git a/.gitignore b/.gitignore index c1ae484..6f6a0b8 100644 --- a/.gitignore +++ b/.gitignore @@ -483,6 +483,7 @@ Makefile.in /src/structs-free.c /src/structs-print.c /src/structs-print.h +/src/uefi.c /src/unit-tests /stamp-h1 /sysprep/.depend @@ -594,6 +595,8 @@ Makefile.in /v2v/test-harness/META /v2v/test-harness/stamp-virt-v2v-test-harness.pod /v2v/test-harness/virt-v2v-test-harness.1 +/v2v/uefi.ml +/v2v/uefi.mli /v2v/v2v_unit_tests /v2v/virt-v2v /v2v/virt-v2v.1 diff --git a/generator/Makefile.am b/generator/Makefile.am index fdb6c0e..a5ff516 100644 --- a/generator/Makefile.am +++ b/generator/Makefile.am @@ -75,6 +75,8 @@ sources = \ tests_c_api.ml \ tests_c_api.mli \ types.ml \ + uefi.ml \ + uefi.mli \ utils.ml \ utils.mli \ xdr.ml \ @@ -113,6 +115,7 @@ objects = \ bindtests.cmo \ errnostring.cmo \ customize.cmo \ + uefi.cmo \ main.cmo EXTRA_DIST = $(sources) files-generated.txt diff --git a/generator/main.ml b/generator/main.ml index 91ca4d7..87616a8 100644 --- a/generator/main.ml +++ b/generator/main.ml @@ -45,6 +45,7 @@ open Gobject open Golang open Bindtests open Errnostring +open Uefi open Customize let perror msg = function @@ -211,6 +212,10 @@ Run it from the top source directory using the command generate_gobject_session_header; output_to "gobject/src/session.c" generate_gobject_session_source; + output_to "src/uefi.c" generate_uefi_c; + output_to "v2v/uefi.ml" generate_uefi_ml; + output_to "v2v/uefi.mli" generate_uefi_mli; + output_to "customize/customize_cmdline.mli" generate_customize_cmdline_mli; output_to "customize/customize_cmdline.ml" generate_customize_cmdline_ml; output_to "customize/customize-synopsis.pod" generate_customize_synopsis_pod; diff --git a/generator/uefi.ml b/generator/uefi.ml new file mode 100644 index 0000000..1f78a6d --- /dev/null +++ b/generator/uefi.ml @@ -0,0 +1,163 @@ +(* libguestfs + * Copyright (C) 2016 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 + *) + +(* Please read generator/README first. *) + +open Utils +open Pr +open Docstrings + +(* danpb is proposing that libvirt supports E<lt>loader type="efi"/E<gt> + * (L<https://bugzilla.redhat.com/1217444#c6>). If that happens we can + * simplify or even remove this code. + *) + +(* Order is significant *within architectures only*. *) +let firmware = [ + "i386", + "/usr/share/edk2.git/ovmf-ia32/OVMF_CODE-pure-efi.fd", + None, + "/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd", + []; + + "x86_64", + "/usr/share/OVMF/OVMF_CODE.fd", + None, + "/usr/share/OVMF/OVMF_VARS.fd", + []; + + "x86_64", + "/usr/share/edk2/ovmf/OVMF_CODE.fd", + None, + "/usr/share/edk2/ovmf/OVMF_VARS.fd", + []; + + (* kraxel's old repository, these will be removed by end of 2016. *) + "x86_64", + "/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd", + None, + "/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd", + []; + + "x86_64", + "/usr/share/qemu/ovmf-x86_64-code.bin", + None, + "/usr/share/qemu/ovmf-x86_64-vars.bin", + []; + + "aarch64", + "/usr/share/AAVMF/AAVMF_CODE.fd", + Some "/usr/share/AAVMF/AAVMF_CODE.verbose.fd", + "/usr/share/AAVMF/AAVMF_VARS.fd", + []; + + "aarch64", + "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw", + None, + "/usr/share/edk2/aarch64/vars-template-pflash.raw", + []; + + (* kraxel's old repository, these will be removed by end of 2016. *) + "aarch64", + "/usr/share/edk2.git/aarch64/QEMU_EFI-pflash.raw", + None, + "/usr/share/edk2.git/aarch64/vars-template-pflash.raw", + []; +] + +let arches = sort_uniq (List.map (fun (arch, _, _, _, _) -> arch) firmware) + +let generate_uefi_c () = + generate_header CStyle LGPLv2plus; + + pr "#include <config.h>\n"; + pr "\n"; + pr "#include <stdio.h>\n"; + pr "\n"; + pr "#include \"guestfs-internal-frontend.h\"\n"; + + List.iter ( + fun arch -> + let firmware = + List.filter (fun (arch', _, _, _, _) -> arch = arch') firmware in + pr "\n"; + pr "struct uefi_firmware\n"; + pr "guestfs_int_uefi_%s_firmware[] = {\n" arch; + List.iter ( + fun (_, code, code_debug, vars, flags) -> + assert (flags = []); + pr " { \"%s\",\n" (c_quote code); + (match code_debug with + | None -> pr " NULL,\n" + | Some code_debug -> pr " \"%s\",\n" (c_quote code_debug) + ); + pr " \"%s\",\n" (c_quote vars); + pr " 0\n"; + pr " },\n"; + ) firmware; + pr "};\n"; + ) arches + +let generate_uefi_ml () = + generate_header OCamlStyle GPLv2plus; + + pr "\ +type uefi_firmware = { + code : string; + code_debug : string option; + vars : string; + flags : unit list; +} +"; + List.iter ( + fun arch -> + let firmware = + List.filter (fun (arch', _, _, _, _) -> arch = arch') firmware in + pr "\n"; + pr "let uefi_%s_firmware = [\n" arch; + List.iter ( + fun (_, code, code_debug, vars, flags) -> + assert (flags = []); + pr " { code = %S;\n" code; + (match code_debug with + | None -> pr " code_debug = None;\n" + | Some code_debug -> pr " code_debug = Some %S;\n" code_debug + ); + pr " vars = %S;\n" vars; + pr " flags = [];\n"; + pr " };\n"; + ) firmware; + pr "]\n"; + ) arches + +let generate_uefi_mli () = + generate_header OCamlStyle GPLv2plus; + + pr "\ +(** UEFI paths. *) + +type uefi_firmware = { + code : string; (** code file *) + code_debug : string option; (** code debug file *) + vars : string; (** vars template file *) + flags : unit list; (** flags *) +} + +"; + + List.iter (pr "val uefi_%s_firmware : uefi_firmware list\n") arches diff --git a/generator/uefi.mli b/generator/uefi.mli new file mode 100644 index 0000000..64aa1f8 --- /dev/null +++ b/generator/uefi.mli @@ -0,0 +1,21 @@ +(* libguestfs + * Copyright (C) 2016 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 + *) + +val generate_uefi_c : unit -> unit +val generate_uefi_ml : unit -> unit +val generate_uefi_mli : unit -> unit diff --git a/generator/utils.ml b/generator/utils.ml index eee8d59..bb96eb6 100644 --- a/generator/utils.ml +++ b/generator/utils.ml @@ -233,6 +233,22 @@ let mapi f xs = in loop 0 xs +let uniq ?(cmp = Pervasives.compare) xs = + let rec loop acc = function + | [] -> acc + | [x] -> x :: acc + | x :: (y :: _ as xs) when cmp x y = 0 -> + loop acc xs + | x :: (y :: _ as xs) -> + loop (x :: acc) xs + in + List.rev (loop [] xs) + +let sort_uniq ?(cmp = Pervasives.compare) xs = + let xs = List.sort cmp xs in + let xs = uniq ~cmp xs in + xs + let count_chars c str = let count = ref 0 in for i = 0 to String.length str - 1 do diff --git a/generator/utils.mli b/generator/utils.mli index 41dd47d..a69328f 100644 --- a/generator/utils.mli +++ b/generator/utils.mli @@ -84,6 +84,12 @@ val iteri : (int -> 'a -> unit) -> 'a list -> unit val mapi : (int -> 'a -> 'b) -> 'a list -> 'b list +val uniq : ?cmp:('a -> 'a -> int) -> 'a list -> 'a list +(** Uniquify a list (the list must be sorted first). *) + +val sort_uniq : ?cmp:('a -> 'a -> int) -> 'a list -> 'a list +(** Sort and uniquify a list. *) + val count_chars : char -> string -> int (** Count number of times the character occurs in string. *) diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index 8689009..f40d2c6 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -100,6 +100,7 @@ struct uefi_firmware { const char *code; /* code file (NULL = end of list) */ const char *code_debug; /* code file with debugging msgs (may be NULL)*/ const char *vars; /* vars template file */ + int flags; /* flags */ }; extern struct uefi_firmware guestfs_int_ovmf_i386_firmware[]; extern struct uefi_firmware guestfs_int_ovmf_x86_64_firmware[]; diff --git a/src/uefi.c b/src/uefi.c deleted file mode 100644 index 44340fe..0000000 --- a/src/uefi.c +++ /dev/null @@ -1,79 +0,0 @@ -/* libguestfs - * Copyright (C) 2009-2016 Red Hat Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2 of the License, or (at your option) any later version. - * - * This library 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 - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - */ - -/** - * Locations of UEFI files. - */ - -#include <config.h> - -#include <stdio.h> - -/* NB: MUST NOT include "guestfs-internal.h". */ -#include "guestfs-internal-frontend.h" - -/* See src/appliance.c:guestfs_int_get_uefi. */ -struct uefi_firmware -guestfs_int_ovmf_i386_firmware[] = { - /* kraxel's old repository, these will be removed by end of 2016. */ - { "/usr/share/edk2.git/ovmf-ia32/OVMF_CODE-pure-efi.fd", - NULL, - "/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd" }, - - { NULL } -}; - -struct uefi_firmware -guestfs_int_ovmf_x86_64_firmware[] = { - { "/usr/share/OVMF/OVMF_CODE.fd", - NULL, - "/usr/share/OVMF/OVMF_VARS.fd" }, - - { "/usr/share/edk2/ovmf/OVMF_CODE.fd", - NULL, - "/usr/share/edk2/ovmf/OVMF_VARS.fd" }, - - /* kraxel's old repository, these will be removed by end of 2016. */ - { "/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd", - NULL, - "/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd" }, - - { "/usr/share/qemu/ovmf-x86_64-code.bin", - NULL, - "/usr/share/qemu/ovmf-x86_64-vars.bin" }, - - { NULL } -}; - -struct uefi_firmware -guestfs_int_aavmf_firmware[] = { - { "/usr/share/AAVMF/AAVMF_CODE.fd", - "/usr/share/AAVMF/AAVMF_CODE.verbose.fd", - "/usr/share/AAVMF/AAVMF_VARS.fd" }, - - { "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw", - NULL, - "/usr/share/edk2/aarch64/vars-template-pflash.raw" }, - - /* kraxel's old repository, these will be removed by end of 2016. */ - { "/usr/share/edk2.git/aarch64/QEMU_EFI-pflash.raw", - NULL, - "/usr/share/edk2.git/aarch64/vars-template-pflash.raw" }, - - { NULL } -}; diff --git a/v2v/Makefile.am b/v2v/Makefile.am index 008c5ba..d53e6e6 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -53,6 +53,7 @@ SOURCES_MLI = \ OVF.mli \ target_bus_assignment.mli \ types.mli \ + uefi.mli \ utils.mli \ vCenter.mli \ windows.mli \ @@ -62,6 +63,7 @@ SOURCES_MLI = \ SOURCES_ML = \ types.ml \ xml.ml \ + uefi.ml \ utils.ml \ vCenter.ml \ domainxml.ml \ @@ -173,6 +175,7 @@ virt_v2v_copy_to_local_CFLAGS = \ COPY_TO_LOCAL_BOBJECTS = \ xml.cmo \ + uefi.cmo \ utils.cmo \ vCenter.cmo \ domainxml.cmo \ @@ -384,6 +387,7 @@ endif v2v_unit_tests_BOBJECTS = \ types.cmo \ xml.cmo \ + uefi.cmo \ utils.cmo \ DOM.cmo \ OVF.cmo \ diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml index 1f66d6a..df0963e 100644 --- a/v2v/output_libvirt.ml +++ b/v2v/output_libvirt.ml @@ -118,7 +118,7 @@ let create_libvirt_xml ?pool source target_buses guestcaps * (https://bugzilla.redhat.com/show_bug.cgi?id=1217444#c6) but * until that day we have to use a bunch of heuristics. XXX *) - let { code = code; vars = vars_template } = + let { Uefi.code = code; vars = vars_template } = find_uefi_firmware guestcaps.gcaps_arch in [ e "loader" ["readonly", "yes"; "type", "pflash"] [ PCData code ]; e "nvram" ["template", vars_template] [] ] in diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml index b6093be..93694b5 100644 --- a/v2v/output_qemu.ml +++ b/v2v/output_qemu.ml @@ -83,7 +83,7 @@ object (match uefi_firmware with | None -> () - | Some { code = code } -> + | Some { Uefi.code = code } -> fpf "%s-drive if=pflash,format=raw,file=%s,readonly" nl (quote code); fpf "%s-drive if=pflash,format=raw,file=\"$uefi_vars\"" nl ); diff --git a/v2v/utils-c.c b/v2v/utils-c.c index 2812a3d..af74c12 100644 --- a/v2v/utils-c.c +++ b/v2v/utils-c.c @@ -66,53 +66,6 @@ v2v_utils_drive_index (value strv) CAMLreturn (Val_int (r)); } -static value -get_firmware (struct uefi_firmware *firmware) -{ - CAMLparam0 (); - CAMLlocal5 (rv, v, v1, v2, cons); - size_t i, len; - - rv = Val_int (0); - - /* Build the list backwards so we don't have to reverse it at the end. */ - len = 0; - for (i = 0; firmware[i].code != NULL; ++i) - ++len; - - for (i = len; i > 0; --i) { - v1 = caml_copy_string (firmware[i-1].code); - v2 = caml_copy_string (firmware[i-1].vars); - v = caml_alloc (2, 0); - Store_field (v, 0, v1); - Store_field (v, 1, v2); - cons = caml_alloc (2, 0); - Store_field (cons, 1, rv); - rv = cons; - Store_field (cons, 0, v); - } - - CAMLreturn (rv); -} - -value -v2v_utils_ovmf_i386_firmware (value unitv) -{ - return get_firmware (guestfs_int_ovmf_i386_firmware); -} - -value -v2v_utils_ovmf_x86_64_firmware (value unitv) -{ - return get_firmware (guestfs_int_ovmf_x86_64_firmware); -} - -value -v2v_utils_aavmf_firmware (value unitv) -{ - return get_firmware (guestfs_int_aavmf_firmware); -} - value v2v_utils_shell_unquote (value strv) { diff --git a/v2v/utils.ml b/v2v/utils.ml index 2dff6aa..9673062 100644 --- a/v2v/utils.ml +++ b/v2v/utils.ml @@ -81,30 +81,21 @@ let qemu_supports_sound_card = function | Types.USBAudio -> true -type uefi_firmware = { - code : string; (* code file *) - vars : string; (* vars template file *) -} - -external ovmf_i386_firmware : unit -> uefi_firmware list = "v2v_utils_ovmf_i386_firmware" -external ovmf_x86_64_firmware : unit -> uefi_firmware list = "v2v_utils_ovmf_x86_64_firmware" -external aavmf_firmware : unit -> uefi_firmware list = "v2v_utils_aavmf_firmware" - (* Find the UEFI firmware. *) let find_uefi_firmware guest_arch = let files = (* The lists of firmware are actually defined in src/uefi.c. *) match guest_arch with - | "i386" | "i486" | "i586" | "i686" -> ovmf_i386_firmware () - | "x86_64" -> ovmf_x86_64_firmware () - | "aarch64" -> aavmf_firmware () + | "i386" | "i486" | "i586" | "i686" -> Uefi.uefi_i386_firmware + | "x86_64" -> Uefi.uefi_x86_64_firmware + | "aarch64" -> Uefi.uefi_aarch64_firmware | arch -> error (f_"don't know how to convert UEFI guests for architecture %s") guest_arch in let rec loop = function | [] -> error (f_"cannot find firmware for UEFI guests.\n\nYou probably need to install OVMF, or Gerd's firmware repo (https://www.kraxel.org/repos/), or AAVMF (if using aarch64)") - | ({ code = code; vars = vars_template } as ret) :: rest -> + | ({ Uefi.code = code; vars = vars_template } as ret) :: rest -> if Sys.file_exists code && Sys.file_exists vars_template then ret else loop rest in @@ -143,10 +134,3 @@ let du filename = | [] -> invalid_arg filename external shell_unquote : string -> string = "v2v_utils_shell_unquote" - -(* The following functions are only exported for unit tests. *) -module UNIT_TESTS = struct - let ovmf_i386_firmware = ovmf_i386_firmware - let ovmf_x86_64_firmware = ovmf_x86_64_firmware - let aavmf_firmware = aavmf_firmware -end diff --git a/v2v/utils.mli b/v2v/utils.mli index a00e60b..bdabab4 100644 --- a/v2v/utils.mli +++ b/v2v/utils.mli @@ -43,12 +43,7 @@ val kvm_arch : string -> string val qemu_supports_sound_card : Types.source_sound_model -> bool (** Does qemu support the given sound card? *) -type uefi_firmware = { - code : string; (** code file *) - vars : string; (** vars template file *) -} - -val find_uefi_firmware : string -> uefi_firmware +val find_uefi_firmware : string -> Uefi.uefi_firmware (** Find the UEFI firmware for the guest architecture. This cannot return an error, it calls [error] and fails instead. *) @@ -73,12 +68,3 @@ val shell_unquote : string -> string This is just intended to deal with quoting in configuration files (like ones under /etc/sysconfig), and it doesn't deal with some situations such as $variable interpolation. *) - -(**/**) - -(* The following functions are only exported for unit tests. *) -module UNIT_TESTS : sig - val ovmf_i386_firmware : unit -> uefi_firmware list - val ovmf_x86_64_firmware : unit -> uefi_firmware list - val aavmf_firmware : unit -> uefi_firmware list -end diff --git a/v2v/v2v_unit_tests.ml b/v2v/v2v_unit_tests.ml index 6064448..f0fa859 100644 --- a/v2v/v2v_unit_tests.ml +++ b/v2v/v2v_unit_tests.ml @@ -778,36 +778,6 @@ let test_shell_unquote ctx = assert_equal ~printer "i`" (Utils.shell_unquote "\"i\\`\""); assert_equal ~printer "j\"" (Utils.shell_unquote "\"j\\\"\"") -(* XXX These tests contain the same strings as found in src/uefi.c. *) -let test_find_uefi_firmware ctx = - let rec printer = function - | [] -> "" - | { Utils.code = code; vars = vars } :: xs -> - sprintf "code=%s vars=%s\n%s" code vars (printer xs) - in - assert_equal ~printer - [ { Utils.code = "/usr/share/edk2.git/ovmf-ia32/OVMF_CODE-pure-efi.fd"; - vars = "/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd" } ] - (Utils.UNIT_TESTS.ovmf_i386_firmware ()); - assert_equal ~printer - [ { Utils.code = "/usr/share/OVMF/OVMF_CODE.fd"; - vars = "/usr/share/OVMF/OVMF_VARS.fd" }; - { Utils.code = "/usr/share/edk2/ovmf/OVMF_CODE.fd"; - vars = "/usr/share/edk2/ovmf/OVMF_VARS.fd" }; - { Utils.code = "/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd"; - vars = "/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd" }; - { Utils.code = "/usr/share/qemu/ovmf-x86_64-code.bin"; - vars = "/usr/share/qemu/ovmf-x86_64-vars.bin" } ] - (Utils.UNIT_TESTS.ovmf_x86_64_firmware ()); - assert_equal ~printer - [ { Utils.code = "/usr/share/AAVMF/AAVMF_CODE.fd"; - vars = "/usr/share/AAVMF/AAVMF_VARS.fd" }; - { Utils.code = "/usr/share/edk2/aarch64/QEMU_EFI-pflash.raw"; - vars = "/usr/share/edk2/aarch64/vars-template-pflash.raw" }; - { Utils.code = "/usr/share/edk2.git/aarch64/QEMU_EFI-pflash.raw"; - vars = "/usr/share/edk2.git/aarch64/vars-template-pflash.raw" } ] - (Utils.UNIT_TESTS.aavmf_firmware ()) - (* Suites declaration. *) let suite = "virt-v2v" >::: @@ -818,7 +788,6 @@ let suite = "Windows_virtio.virtio_iso_path_matches_guest_os" >:: test_virtio_iso_path_matches_guest_os; "Utils.shell_unquote" >:: test_shell_unquote; - "Utils.find_uefi_firmware" >:: test_find_uefi_firmware; ] let () = -- 2.7.4
From RHEL 7.3, Red Hat have decided to only compile the secure boot version of OVMF on x86-64, with flags -D SECURE_BOOT_ENABLE -D SMM_REQUIRE.
The filename has also changed to reflect this - it is now /usr/share/OVMF/OVMF_CODE.secboot.fd. The old file /usr/share/OVMF/OVMF_CODE.fd is no longer shipped. However switching to using this variant of OVMF for UEFI guests is not just a matter of changing the filename. The new OVMF variant won't run unless we also change: - The qemu machine model, from the default ("pc" == "pc-i440fx-rhel7.3.0" or later) to Q35 ("q35" == "pc-q35-rhel7.3.0" or later). - Add <smm> under <features>. - Set <loader ... secure="yes">. NB: On RHEL the changes requires qemu-kvm-rhev. It is no longer possible to convert UEFI guests using the basic qemu-kvm. Thanks: Laszlo Ersek, Ming Xie. --- generator/uefi.ml | 24 ++++++++++++++++++------ src/appliance.c | 6 ++++-- src/guestfs-internal-frontend.h | 3 ++- src/guestfs-internal.h | 2 +- src/launch-direct.c | 15 ++++++++++++++- src/launch-libvirt.c | 16 +++++++++++++++- v2v/output_libvirt.ml | 40 +++++++++++++++++++++++++++++----------- v2v/output_qemu.ml | 15 +++++++++++---- v2v/virt-v2v.pod | 2 ++ 9 files changed, 96 insertions(+), 27 deletions(-) diff --git a/generator/uefi.ml b/generator/uefi.ml index 1f78a6d..88e54b8 100644 --- a/generator/uefi.ml +++ b/generator/uefi.ml @@ -41,6 +41,16 @@ let firmware = [ "/usr/share/OVMF/OVMF_VARS.fd", []; + (* From RHEL 7.3, only secure boot variants of UEFI are shipped. + * This requires additional qemu options, see RHBZ#1367615 for + * details. + *) + "x86_64", + "/usr/share/OVMF/OVMF_CODE.secboot.fd", + None, + "/usr/share/OVMF/OVMF_VARS.fd", + [ "UEFI_FLAG_SECURE_BOOT_REQUIRED" ]; + "x86_64", "/usr/share/edk2/ovmf/OVMF_CODE.fd", None, @@ -100,14 +110,13 @@ let generate_uefi_c () = pr "guestfs_int_uefi_%s_firmware[] = {\n" arch; List.iter ( fun (_, code, code_debug, vars, flags) -> - assert (flags = []); pr " { \"%s\",\n" (c_quote code); (match code_debug with | None -> pr " NULL,\n" | Some code_debug -> pr " \"%s\",\n" (c_quote code_debug) ); pr " \"%s\",\n" (c_quote vars); - pr " 0\n"; + pr " %s\n" (if flags <> [] then String.concat "|" flags else "0"); pr " },\n"; ) firmware; pr "};\n"; @@ -121,8 +130,10 @@ type uefi_firmware = { code : string; code_debug : string option; vars : string; - flags : unit list; + flags : uefi_flags; } +and uefi_flags = uefi_flag list +and uefi_flag = UEFI_FLAG_SECURE_BOOT_REQUIRED "; List.iter ( fun arch -> @@ -132,14 +143,13 @@ type uefi_firmware = { pr "let uefi_%s_firmware = [\n" arch; List.iter ( fun (_, code, code_debug, vars, flags) -> - assert (flags = []); pr " { code = %S;\n" code; (match code_debug with | None -> pr " code_debug = None;\n" | Some code_debug -> pr " code_debug = Some %S;\n" code_debug ); pr " vars = %S;\n" vars; - pr " flags = [];\n"; + pr " flags = [%s];\n" (String.concat "; " flags); pr " };\n"; ) firmware; pr "]\n"; @@ -155,8 +165,10 @@ type uefi_firmware = { code : string; (** code file *) code_debug : string option; (** code debug file *) vars : string; (** vars template file *) - flags : unit list; (** flags *) + flags : uefi_flags; (** flags *) } +and uefi_flags = uefi_flag list +and uefi_flag = UEFI_FLAG_SECURE_BOOT_REQUIRED "; diff --git a/src/appliance.c b/src/appliance.c index 942ecae..025d55d 100644 --- a/src/appliance.c +++ b/src/appliance.c @@ -423,10 +423,10 @@ dir_contains_files (guestfs_h *g, const char *dir, ...) * should cause appliance building to fail (no UEFI firmware is not an * error). * - * XXX See also F<v2v/utils.ml>:find_uefi_firmware + * See also F<v2v/utils.ml>:find_uefi_firmware */ int -guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) +guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags) { #ifdef __aarch64__ size_t i; @@ -464,6 +464,7 @@ guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) /* Caller frees. */ *code = safe_strdup (g, codefile); *vars = varst; + *flags = guestfs_int_aavmf_firmware[i].flags; return 0; } } @@ -471,5 +472,6 @@ guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars) /* Not found. */ *code = *vars = NULL; + *flags = 0; return 0; } diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index f40d2c6..a41ee9c 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -100,7 +100,8 @@ struct uefi_firmware { const char *code; /* code file (NULL = end of list) */ const char *code_debug; /* code file with debugging msgs (may be NULL)*/ const char *vars; /* vars template file */ - int flags; /* flags */ + int flags; /* various flags, see below */ +#define UEFI_FLAG_SECURE_BOOT_REQUIRED 1 /* secure boot (see RHBZ#1367615) */ }; extern struct uefi_firmware guestfs_int_ovmf_i386_firmware[]; extern struct uefi_firmware guestfs_int_ovmf_x86_64_firmware[]; diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 2b49011..d437b9a 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -783,7 +783,7 @@ extern const char *guestfs_int_drive_protocol_to_string (enum drive_protocol pro /* appliance.c */ extern int guestfs_int_build_appliance (guestfs_h *g, char **kernel, char **initrd, char **appliance); -extern int guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars); +extern int guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags); /* launch.c */ extern int64_t guestfs_int_timeval_diff (const struct timeval *x, const struct timeval *y); diff --git a/src/launch-direct.c b/src/launch-direct.c index 6450d7e..64e52e8 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -226,6 +226,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) int sv[2]; struct sockaddr_un addr; CLEANUP_FREE char *uefi_code = NULL, *uefi_vars = NULL; + int uefi_flags; CLEANUP_FREE char *kernel = NULL, *initrd = NULL, *appliance = NULL; int has_appliance_drive; CLEANUP_FREE char *appliance_dev = NULL; @@ -422,8 +423,20 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) } /* UEFI (firmware) if required. */ - if (guestfs_int_get_uefi (g, &uefi_code, &uefi_vars) == -1) + if (guestfs_int_get_uefi (g, &uefi_code, &uefi_vars, &uefi_flags) == -1) goto cleanup0; + if (uefi_flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) { + /* Implementing this requires changes to the qemu command line. + * See RHBZ#1367615 for details. As the guestfs_int_get_uefi + * function is only implemented for aarch64, and UEFI secure boot + * is some way off on aarch64 (2017/2018), we only need to worry + * about this later. + */ + error (g, "internal error: direct backend " + "does not implement UEFI secure boot, " + "see comments in the code"); + goto cleanup0; + } if (uefi_code) { ADD_CMDLINE ("-drive"); ADD_CMDLINE_PRINTF ("if=pflash,format=raw,file=%s,readonly", uefi_code); diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 6249494..d8479dc 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -313,6 +313,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) uint32_t size; CLEANUP_FREE void *buf = NULL; unsigned long version_number; + int uefi_flags; params.current_proc_is_root = geteuid () == 0; @@ -409,8 +410,21 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) goto cleanup; /* UEFI code and variables, on architectures where that is required. */ - if (guestfs_int_get_uefi (g, &data->uefi_code, &data->uefi_vars) == -1) + if (guestfs_int_get_uefi (g, &data->uefi_code, &data->uefi_vars, + &uefi_flags) == -1) goto cleanup; + if (uefi_flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) { + /* Implementing this requires changes to the libvirt XML. See + * RHBZ#1367615 for details. As the guestfs_int_get_uefi function + * is only implemented for aarch64, and UEFI secure boot is some + * way off on aarch64 (2017/2018), we only need to worry about + * this later. + */ + error (g, "internal error: libvirt backend " + "does not implement UEFI secure boot, " + "see comments in the code"); + goto cleanup; + } /* Misc backend settings. */ guestfs_push_error_handler (g, NULL, NULL); diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml index df0963e..ed430a2 100644 --- a/v2v/output_libvirt.ml +++ b/v2v/output_libvirt.ml @@ -71,6 +71,16 @@ let target_features_of_capabilities_doc doc arch = let create_libvirt_xml ?pool source target_buses guestcaps target_features target_firmware = + let uefi_firmware = + match target_firmware with + | TargetBIOS -> None + | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in + let secure_boot_required = + match uefi_firmware with + | Some { Uefi.flags = flags } + when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true + | _ -> false in + let memory_k = source.s_memory /^ 1024L in (* We have the machine features of the guest when it was on the @@ -106,24 +116,32 @@ let create_libvirt_xml ?pool source target_buses guestcaps StringSet.inter(*section*) force_features target_features in let features = StringSet.union features force_features in + (* Add <smm> feature if UEFI requires it. Note that libvirt + * capabilities doesn't list this feature even if it is supported + * by qemu, so we have to blindly add it, which might cause libvirt + * to fail. (XXX) + *) + let features = + if secure_boot_required then StringSet.add "smm" features else features in + let features = List.sort compare (StringSet.elements features) in (* The <os> section subelements. *) let os_section = + let machine = if secure_boot_required then [ "machine", "q35" ] else [] in + let loader = - match target_firmware with - | TargetBIOS -> [] - | TargetUEFI -> - (* danpb is proposing that libvirt supports <loader type="efi"/>, - * (https://bugzilla.redhat.com/show_bug.cgi?id=1217444#c6) but - * until that day we have to use a bunch of heuristics. XXX - *) - let { Uefi.code = code; vars = vars_template } = - find_uefi_firmware guestcaps.gcaps_arch in - [ e "loader" ["readonly", "yes"; "type", "pflash"] [ PCData code ]; + match uefi_firmware with + | None -> [] + | Some { Uefi.code = code; vars = vars_template } -> + let secure = + if secure_boot_required then [ "secure", "yes" ] else [] in + [ e "loader" (["readonly", "yes"; "type", "pflash"] @ secure) + [ PCData code ]; e "nvram" ["template", vars_template] [] ] in - (e "type" ["arch", guestcaps.gcaps_arch] [PCData "hvm"]) :: loader in + (e "type" (["arch", guestcaps.gcaps_arch] @ machine) [PCData "hvm"]) + :: loader in (* The devices. *) let devices = ref [] in diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml index 93694b5..0788e6c 100644 --- a/v2v/output_qemu.ml +++ b/v2v/output_qemu.ml @@ -57,8 +57,12 @@ object let uefi_firmware = match target_firmware with | TargetBIOS -> None - | TargetUEFI -> - Some (find_uefi_firmware guestcaps.gcaps_arch) in + | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in + let secure_boot_required = + match uefi_firmware with + | Some { Uefi.flags = flags } + when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true + | _ -> false in let chan = open_out file in @@ -69,7 +73,7 @@ object (match uefi_firmware with | None -> () - | Some { vars = vars_template } -> + | Some { Uefi.vars = vars_template } -> fpf "# Make a copy of the UEFI variables template\n"; fpf "uefi_vars=\"$(mktemp)\"\n"; fpf "cp %s \"$uefi_vars\"\n" (quote vars_template); @@ -79,11 +83,14 @@ object fpf "qemu-system-%s" guestcaps.gcaps_arch; fpf "%s-no-user-config -nodefaults" nl; fpf "%s-name %s" nl (quote source.s_name); - fpf "%s-machine accel=kvm:tcg" nl; + fpf "%s-machine %saccel=kvm:tcg" nl + (if secure_boot_required then "q35,smm=on," else ""); (match uefi_firmware with | None -> () | Some { Uefi.code = code } -> + if secure_boot_required then + fpf "%s-global driver=cfi.pflash01,property=secure,value=on" nl; fpf "%s-drive if=pflash,format=raw,file=%s,readonly" nl (quote code); fpf "%s-drive if=pflash,format=raw,file=\"$uefi_vars\"" nl ); diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod index ed0d286..220dd45 100644 --- a/v2v/virt-v2v.pod +++ b/v2v/virt-v2v.pod @@ -881,6 +881,8 @@ automatically, but note that the same version of OVMF must be installed on the conversion host as is installed on the target hypervisor, else you will have to adjust paths in the metadata. +On RHEL E<ge> 7.3, only qemu-kvm-rhev (not qemu-kvm) is supported. + =item UEFI on OpenStack Not supported. -- 2.7.4
On Thursday, 18 August 2016 13:18:35 CEST Richard W.M. Jones wrote:
First version was posted here: https://www.redhat.com/archives/libguestfs/2016-August/thread.html#00100
This is semantically the same as the first version. However I've split the patch up into two parts. In the first part, I factor out the UEFI paths so now they are created by the generator and written in the library and v2v/ directory directly, instead of the complex business of having a C database which is read in virt-v2v using C bindings.
The second part is the same as before (but simplified because it now needs fewer changes).
Not tried, but seems okay from a cursory look. -- Pino Toscano
participants (2)
-
Pino Toscano -
Richard W.M. Jones