[Libguestfs] [PATCH v2v 1/3] v2v: Rename gcaps_default_cpu to gcaps_arch_min_version
Richard W.M. Jones
rjones at redhat.com
Fri Feb 17 09:56:41 UTC 2023
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.
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
> > );
> >
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list