[Libguestfs] [PATCH 1/2] v2v: fill the list of the EFI system partitions
Pavel Butsykin
pbutsykin at virtuozzo.com
Fri Jun 10 15:29:35 UTC 2016
On 10.06.2016 18:02, Richard W.M. Jones wrote:
> On Fri, Jun 10, 2016 at 05:07:02PM +0300, Pavel Butsykin wrote:
>> Store the list of EFI system partitions on the inspect object in order to be
>> able to tune their contents later in the process.
>>
>> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>
>> ---
>> v2v/convert_linux.ml | 2 +-
>> v2v/inspect_source.ml | 29 +++++++++++++++++++++--------
>> v2v/types.ml | 4 ++--
>> v2v/types.mli | 3 ++-
>> v2v/v2v.ml | 2 +-
>> v2v/v2v_unit_tests.ml | 2 +-
>> 6 files changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
>> index 08b27d6..2de5e42 100644
>> --- a/v2v/convert_linux.ml
>> +++ b/v2v/convert_linux.ml
>> @@ -97,7 +97,7 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source rcaps =
>> "/boot/grub/grub.conf", `Grub1;
>> ] in
>> let locations =
>> - if inspect.i_uefi then
>> + if inspect.i_uefi <> None then
>> ("/boot/efi/EFI/redhat/grub.cfg", `Grub2) :: locations
>> else
>> locations in
>> diff --git a/v2v/inspect_source.ml b/v2v/inspect_source.ml
>> index 65dcb88..7a7674a 100644
>> --- a/v2v/inspect_source.ml
>> +++ b/v2v/inspect_source.ml
>> @@ -68,7 +68,7 @@ let rec inspect_source root_choice g =
>> (* See if this guest could use UEFI to boot. It should use GPT and
>> * it should have an EFI System Partition (ESP).
>> *)
>> - let uefi = has_uefi_bootable_device g in
>> + let uefi = get_uefi_bootable_device g in
>>
>> let inspect = {
>> i_root = root;
>> @@ -153,10 +153,12 @@ and reject_if_not_installed_image g root =
>> if fmt <> "installed" then
>> error (f_"libguestfs thinks this is not an installed operating system (it might be, for example, an installer disk or live CD). If this is wrong, it is probably a bug in libguestfs. root=%s fmt=%s") root fmt
>>
>> -and has_uefi_bootable_device g =
>> +and get_uefi_bootable_device g =
>> let rec uefi_ESP_guid = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B"
>> and is_uefi_ESP dev { G.part_num = partnum } =
>> g#part_get_gpt_type dev (Int32.to_int partnum) = uefi_ESP_guid
>> + and part_dev_name dev { G.part_num = partnum } =
>> + sprintf "%s%d" dev (Int32.to_int partnum)
>> and parttype_is_gpt dev =
>> try g#part_get_parttype dev = "gpt"
>> with G.Error msg as exn ->
>> @@ -164,14 +166,25 @@ and has_uefi_bootable_device g =
>> if g#last_errno () <> G.Errno.errno_EINVAL then raise exn;
>> debug "%s (ignored)" msg;
>> false
>> - and is_uefi_bootable_device dev =
>> - parttype_is_gpt dev && (
>> - let partitions = Array.to_list (g#part_list dev) in
>> - List.exists (is_uefi_ESP dev) partitions
>> - )
>> + and is_uefi_bootable_part dev part =
>> + parttype_is_gpt dev && is_uefi_ESP dev part
>> in
>> let devices = Array.to_list (g#list_devices ()) in
>> - List.exists is_uefi_bootable_device devices
>> + let uefi_list = ref [] in
>> +
>> + List.iter (
>> + fun dev ->
>> + Array.iter (
>> + fun part ->
>> + if (is_uefi_bootable_part dev part) then (
>> + uefi_list := (part_dev_name dev part) :: !uefi_list
>> + )
>> + ) (g#part_list dev)
>> + ) devices;
>> +
>> + match !uefi_list with
>> + | [] -> None
>> + | list -> Some list
>>
>> (* If some inspection fields are "unknown", then that indicates a
>> * failure in inspection, and we shouldn't continue. For an example
>> diff --git a/v2v/types.ml b/v2v/types.ml
>> index 08e1631..7f8a9b3 100644
>> --- a/v2v/types.ml
>> +++ b/v2v/types.ml
>> @@ -315,7 +315,7 @@ type inspect = {
>> i_mountpoints : (string * string) list;
>> i_apps : Guestfs.application2 list;
>> i_apps_map : Guestfs.application2 list StringMap.t;
>> - i_uefi : bool;
>> + i_uefi : string list option;
>
> I think what you really want is for the type to be "string list" (not
> option), and for empty list to mean BIOS. (Assuming that UEFI cannot
> have zero system partitions -- as far as I understand UEFI, that's
> true).
I really wanted to use just "string list", but then I was confused by
the abundance of "list option" :)
>
> Alternately you could write something like:
>
> type i_firmware =
> | BIOS
> | UEFI of string list
>
> ...
> i_firmware : i_firmware;
>
> which is quite nice, I think, because it's explicit about the meaning.
>
>> }
>>
>> let string_of_inspect inspect =
>> @@ -341,7 +341,7 @@ i_uefi = %b
>> inspect.i_package_management
>> inspect.i_product_name
>> inspect.i_product_variant
>> - inspect.i_uefi
>> + (inspect.i_uefi <> None)
>
> We should print the list here, so:
>
> "...i_uefi = [%s]"
> ...
> (String.concat ", " inspect.i_uefi)
>
> or if using the i_firmware type above:
>
> "...i_firmware = %s"
> ...
> (match inspect.i_firmware with
> | BIOS -> "BIOS"
> | UEFI devices -> sprintf "UEFI [%s]" (String.concat ", " devices)
> )
>
>> type mpstat = {
>> mp_dev : string;
>> diff --git a/v2v/types.mli b/v2v/types.mli
>> index dacc991..a0a8399 100644
>> --- a/v2v/types.mli
>> +++ b/v2v/types.mli
>> @@ -221,7 +221,8 @@ type inspect = {
>> (** This is a map from the app name to the application object.
>> Since RPM allows multiple packages with the same name to be
>> installed, the value is a list. *)
>> - i_uefi : bool; (** True if the guest could boot with UEFI. *)
>> + i_uefi : string list option;
>> + (** Some list if the guest could boot with UEFI. *)
>
> This comment needs to document what are the strings in this list.
>
>> }
>> (** Inspection information. *)
>>
>> diff --git a/v2v/v2v.ml b/v2v/v2v.ml
>> index fe81df5..2527633 100644
>> --- a/v2v/v2v.ml
>> +++ b/v2v/v2v.ml
>> @@ -555,7 +555,7 @@ and get_target_firmware inspect guestcaps source output =
>> | BIOS -> TargetBIOS
>> | UEFI -> TargetUEFI
>> | UnknownFirmware ->
>> - if inspect.i_uefi then TargetUEFI else TargetBIOS in
>> + if inspect.i_uefi = None then TargetBIOS else TargetUEFI in
>> let supported_firmware = output#supported_firmware in
>> if not (List.mem target_firmware supported_firmware) then
>> error (f_"this guest cannot run on the target, because the target does not support %s firmware (supported firmware on target: %s)")
>> diff --git a/v2v/v2v_unit_tests.ml b/v2v/v2v_unit_tests.ml
>> index 95d8430..874cd48 100644
>> --- a/v2v/v2v_unit_tests.ml
>> +++ b/v2v/v2v_unit_tests.ml
>> @@ -30,7 +30,7 @@ let inspect_defaults = {
>> i_major_version = 0; i_minor_version = 0;
>> i_root = ""; i_package_format = ""; i_package_management = "";
>> i_product_name = ""; i_product_variant = ""; i_mountpoints = [];
>> - i_apps = []; i_apps_map = StringMap.empty; i_uefi = false
>> + i_apps = []; i_apps_map = StringMap.empty; i_uefi = None
>> }
>
> Rich.
>
Thanks for the good explanation!
More information about the Libguestfs
mailing list