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.
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
> );
>
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
