[Libguestfs] [PATCH v2v 1/3] v2v: Rename gcaps_default_cpu to gcaps_arch_min_version
Laszlo Ersek
lersek at redhat.com
Mon Feb 20 08:31:23 UTC 2023
On 2/17/23 10:56, Richard W.M. Jones wrote:
> On Fri, Feb 17, 2023 at 08:53:42AM +0100, Laszlo Ersek wrote:
>> On 2/15/23 15:12, Richard W.M. Jones wrote:
>>> Some guests require not just a specific architecture, but cannot run
>>> on qemu's default CPU model, eg. requiring x86_64-v2. Since we
>>> anticipate future guests requiring higher versions, let's encode the
>>> minimum architecture version instead of a simple boolean.
>>>
>>> This patch essentially just remaps:
>>>
>>> gcaps_default_cpu = true => gcaps_arch_min_version = 0
>>> gcaps_default_cpu = false => gcaps_arch_min_version = 2
>>>
>>> and updates the comments.
>>>
>>> Updates: commit a50b975024ac5e46e107882e27fea498bf425685
>>> ---
>>> lib/types.mli | 19 +++++++++++--------
>>> convert/convert_linux.ml | 9 +++++----
>>> convert/convert_windows.ml | 2 +-
>>> lib/types.ml | 6 +++---
>>> output/create_libvirt_xml.ml | 4 ++--
>>> output/output_qemu.ml | 6 +++---
>>> 6 files changed, 25 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/lib/types.mli b/lib/types.mli
>>> index 24a93760cf..4a2bb5b371 100644
>>> --- a/lib/types.mli
>>> +++ b/lib/types.mli
>>> @@ -263,24 +263,27 @@ type guestcaps = {
>>> gcaps_machine : guestcaps_machine; (** Machine model. *)
>>> gcaps_arch : string; (** Architecture that KVM must emulate. *)
>>>
>>> - gcaps_virtio_1_0 : bool;
>>> - (** The guest supports the virtio devices that it does at the virtio-1.0
>>> - protocol level. *)
>>> + gcaps_arch_min_version : int;
>>> + (** Some guest OSes require not just a specific architecture, but a
>>> + minimum version. Notably RHEL >= 9 requires at least x86_64-v2.
>>>
>>> - gcaps_default_cpu : bool;
>>> - (** True iff the guest OS is capable of running on QEMU's default VCPU model
>>> - (eg. "-cpu qemu64" with the Q35 and I440FX machine types).
>>> + If the guest is capable of running on QEMU's default VCPU model
>>> + for the architecture then this is set to [0].
>>>
>>> This capability only matters for the QEMU and libvirt output modules,
>>> - where, in case the capability is false *and* the source hypervisor does
>>> + where, in case the capability is false {b and} the source hypervisor does
>>
>> One omission and one typo here:
>>
>> (1a) "false" should be replaced with "nonzero",
>> (1b) there is a superfluous space character before "{b and}".
>>
>>> not specify a VCPU model, we must manually present the guest OS with a
>>> VCPU that looks as close as possible to a physical CPU. (In that case, we
>>> - specify host-passthrough.)
>>> + specify host-model.)
>>
>> Hmm yes this is probably a leftover from commit 12473bfcb749.
>>
>> (2) However, I think we can be a bit clearer here: we specify host-model
>> for libvirt, but host-passthrough for QEMU.
>>
>> (3) ... after reading through the series though, I've come to the same
>> conclusion that's stated in commit message #3: gcaps_arch_min_version
>> becomes effectively superfluous. I'd rather suggest removing it
>> altogether then! I think that will reduce the series to a single patch,
>> which in this particular case is good, I feel.
>
> Do you think it should be kept for informational purposes? I worry
> about losing this information for other outputs where it might matter
> in future. Also we can easily delete it if we find we never need it.
Hm, OK, if it seems reasonable that other outputs might need it in the
near (?) future, then it might mean less churn to keep it now. Also it
doesn't incur big maintenance costs.
Laszlo
>
> Rich.
>
>> I'll have some more comments on the other patches (I figure the updates
>> from those will be squashed into the single version-2 patch, so they
>> remain relevant).
>>
>> Laszlo
>>
>>>
>>> The management applications targeted by the RHV and OpenStack output
>>> modules have their own explicit VCPU defaults, overriding the QEMU default
>>> model even in case the source hypervisor does not specify a VCPU model;
>>> those modules ignore this capability therefore. Refer to RHBZ#2076013. *)
>>> +
>>> + gcaps_virtio_1_0 : bool;
>>> + (** The guest supports the virtio devices that it does at the virtio-1.0
>>> + protocol level. *)
>>> }
>>> (** Guest capabilities after conversion. eg. Was virtio found or installed? *)
>>>
>>> diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
>>> index 460cbff0ed..d5c0f24dbb 100644
>>> --- a/convert/convert_linux.ml
>>> +++ b/convert/convert_linux.ml
>>> @@ -203,9 +203,10 @@ let convert (g : G.guestfs) source inspect i_firmware keep_serial_console _ =
>>> * microarchitecture level, which the default QEMU VCPU model does not
>>> * satisfy. Refer to RHBZ#2076013 RHBZ#2166619.
>>> *)
>>> - let default_cpu_suffices = family <> `RHEL_family ||
>>> - inspect.i_arch <> "x86_64" ||
>>> - inspect.i_major_version < 9 in
>>> + let arch_min_version =
>>> + if family <> `RHEL_family || inspect.i_arch <> "x86_64" ||
>>> + inspect.i_major_version < 9
>>> + then 0 else 2 in
>>>
>>> (* Return guest capabilities from the convert () function. *)
>>> let guestcaps = {
>>> @@ -217,8 +218,8 @@ let convert (g : G.guestfs) source inspect i_firmware keep_serial_console _ =
>>> gcaps_virtio_socket = kernel.ki_supports_virtio_socket;
>>> gcaps_machine = machine;
>>> gcaps_arch = Utils.kvm_arch inspect.i_arch;
>>> + gcaps_arch_min_version = arch_min_version;
>>> gcaps_virtio_1_0 = virtio_1_0;
>>> - gcaps_default_cpu = default_cpu_suffices;
>>> } in
>>>
>>> guestcaps
>>> diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
>>> index 8d3737995f..9d8d271d05 100644
>>> --- a/convert/convert_windows.ml
>>> +++ b/convert/convert_windows.ml
>>> @@ -265,8 +265,8 @@ let convert (g : G.guestfs) _ inspect i_firmware _ static_ips =
>>> gcaps_machine = of_virtio_win_machine_type
>>> virtio_win_installed.Inject_virtio_win.machine;
>>> gcaps_arch = Utils.kvm_arch inspect.i_arch;
>>> + gcaps_arch_min_version = 0;
>>> gcaps_virtio_1_0 = virtio_win_installed.Inject_virtio_win.virtio_1_0;
>>> - gcaps_default_cpu = true;
>>> } in
>>>
>>> guestcaps
>>> diff --git a/lib/types.ml b/lib/types.ml
>>> index 98f8bc6fa5..6c019ae130 100644
>>> --- a/lib/types.ml
>>> +++ b/lib/types.ml
>>> @@ -397,8 +397,8 @@ type guestcaps = {
>>> gcaps_virtio_socket : bool;
>>> gcaps_machine : guestcaps_machine;
>>> gcaps_arch : string;
>>> + gcaps_arch_min_version : int;
>>> gcaps_virtio_1_0 : bool;
>>> - gcaps_default_cpu : bool;
>>> }
>>> and guestcaps_block_type = Virtio_blk | IDE
>>> and guestcaps_net_type = Virtio_net | E1000 | RTL8139
>>> @@ -426,8 +426,8 @@ let string_of_guestcaps gcaps =
>>> gcaps_virtio_socket = %b\n\
>>> gcaps_machine = %s\n\
>>> gcaps_arch = %s\n\
>>> + gcaps_arch_min_version = %d\n\
>>> gcaps_virtio_1_0 = %b\n\
>>> - gcaps_default_cpu = %b\n\
>>> "
>>> (string_of_block_type gcaps.gcaps_block_bus)
>>> (string_of_net_type gcaps.gcaps_net_bus)
>>> @@ -437,8 +437,8 @@ let string_of_guestcaps gcaps =
>>> gcaps.gcaps_virtio_socket
>>> (string_of_machine gcaps.gcaps_machine)
>>> gcaps.gcaps_arch
>>> + gcaps.gcaps_arch_min_version
>>> gcaps.gcaps_virtio_1_0
>>> - gcaps.gcaps_default_cpu
>>>
>>> type target_buses = {
>>> target_virtio_blk_bus : target_bus_slot array;
>>> diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml
>>> index 60977cf5bb..e9c6c8c150 100644
>>> --- a/output/create_libvirt_xml.ml
>>> +++ b/output/create_libvirt_xml.ml
>>> @@ -185,14 +185,14 @@ let create_libvirt_xml ?pool source inspect
>>> ];
>>>
>>> if source.s_cpu_model <> None ||
>>> - not guestcaps.gcaps_default_cpu ||
>>> + guestcaps.gcaps_arch_min_version >= 1 ||
>>> source.s_cpu_topology <> None then (
>>> let cpu_attrs = ref []
>>> and cpu = ref [] in
>>>
>>> (match source.s_cpu_model with
>>> | None ->
>>> - if not guestcaps.gcaps_default_cpu then
>>> + if guestcaps.gcaps_arch_min_version >= 1 then
>>> List.push_back cpu_attrs ("mode", "host-model");
>>> | Some model ->
>>> List.push_back cpu_attrs ("match", "minimum");
>>> diff --git a/output/output_qemu.ml b/output/output_qemu.ml
>>> index b667e782ed..491906ebf9 100644
>>> --- a/output/output_qemu.ml
>>> +++ b/output/output_qemu.ml
>>> @@ -175,9 +175,9 @@ module QEMU = struct
>>>
>>> arg "-m" (Int64.to_string (source.s_memory /^ 1024L /^ 1024L));
>>>
>>> - (match source.s_cpu_model, guestcaps.gcaps_default_cpu with
>>> - | None, true -> ()
>>> - | None, false -> arg "-cpu" "host"
>>> + (match source.s_cpu_model, guestcaps.gcaps_arch_min_version with
>>> + | None, 0 -> ()
>>> + | None, _ -> arg "-cpu" "host"
>>> | Some model, _ -> arg "-cpu" model
>>> );
>>>
>
More information about the Libguestfs
mailing list