On 25.07.24 09:35, Markus Armbruster wrote:
Markus Armbruster <arm...@redhat.com> writes:

Collin Walling <wall...@linux.ibm.com> writes:

On 7/24/24 3:56 AM, Markus Armbruster wrote:
Collin Walling <wall...@linux.ibm.com> writes:
Let me try to explain the purpose of @deprecated-props and see if it
helps bring us closer to some semblance of a mutual understanding so we
can work together on a concise documentation for this field.

s390 has been announcing features as deprecated for some time now, which
was fine as a way to let users know that they should tune their guests
to no longer user these features.  Now that we are approaching the
release of generations that will drop these deprecated features
outright, we encounter an issue: if users have not been mindful with
disabling these announced-deprecated-features, then their guests running
on older models will not be able to migrate to machines running on newer
hardware.

To alleviate this, I've added the @deprecated-props array to the
CpuModelInfo struct, and this field is populated by a
query-cpu-model-expansion* return.  It is up the the user/management app
to make use of this data.

On the libvirt side (currently in development), I am able to easily
retrieve the host-model with a full expansion, parse the
@deprecated-props, and then cache them for later use (e.g. when
reporting the host-model with these features disabled, or enabling a
user to define their domain with deprecated-features disabled via a
convenient XML attribute).

tl;dr @deprecated-props is only reported via a
query-cpu-model-expansion, and it is up to the user/management app to
figure out what to do with them.

Got it.

Permit me a digression.  In QAPI/QMP, we do something similar: we expose
deprecation in introspection (query-qmp-schema), and what to do with the
information is up to the management application.  We provide one more
tool to it: policy for handling deprecated interfaces, set with -compat.
It permits "testing the future".  See qapi/compat.json for details.
Whether such a thing would be usful in your case I can't say.

On closer examination, more questions on CpuModelInfo emerge.  Uses:


I will attempt to expand on each input @model (CpuModelInfo) as if they
were documented in the file.

* query-cpu-model-comparison both arguments

   Documentation doesn't say how exactly the command uses the members of
   CpuModelInfo, i.e. @name, @props, @deprecated-props.  Can you tell me?


Note: Compares ModelA and ModelB.

Both @models must include @name.  @props is optional.  @deprecated-props
is ignored.

@name: the name of the CPU model definition to look up.  The definition
will be compared against the generation, GA level, and a static set of
properties of the opposing model.

@props: a set of additional properties to include in the model's set of
properties to be compared.

@deprecated-props: ignored.  The user should consider these properties
beforehand and decide if these properties should be disabled/omitted on
the respective model.

* query-cpu-model-expansion argument @model and return value member
   @model.

   The other argument is the expansion type, on which the value of return
   value model.deprecated-props depends, I believe.  Fine.

   Documentation doesn't say how exactly the command uses the members of
   CpuModelInfo arguments, i.e. @name, @props, @deprecated-props.  Can
   you tell me?


The @model must include @name.  @props is optional.  @deprecated-props
is ignored.

@name: the name of the CPU model definition to look up.  The definition
is associated with a set of properties that will populate the return data.

@props: a set of additional properties to include in the model's set of
expanded properties.

@deprecated-props: ignored.  The user should consider these properties
beforehand and decide if these properties should be disabled/omitted on
the model.

Return value member @model will have @name, may have @props and
@deprecated-props.

Absent @props is the same as {}.  Only x86 uses {}.

Absent @deprecated-props is the same as {}.  No target uses {}.  Can be
present only on S390.

Aside: returning the same thing in two different ways, like absent and
{}, is slightly more complex than necessary.  But let's ignore that
here.

* query-cpu-model-baseline both arguments and return value member
   @model.

   Same, except we don't have an expansion type here.  So same question,
   plus another one: how does return value model.deprecated-props behave?


Note: Creates a baseline model based on ModelA and ModelB.

The @models must include @name.  @props is optional.  @deprecated-props
is ignored.

@name: the name of the CPU model definition to look up.  The definition,
GA level, and a static set of properties will be used to determine the
maximum model between ModelA and ModelB.

@props: a set of additional properties to include in the model's set of
properties to be baselined.

@deprecated-props: ignored.  The user should consider these properties
beforehand and decide if these properties should be disabled/omitted on
the respective model.

Return value member @model is just like in query-cpu-model-expansion.

Unlike query-cpu-model-expansion, we don't have an expansion type.  The
value of @deprecated-props depends on the expansion type.  Do we assume
a type?  Which one?

If you can't answer my questions, we need to find someone who can.


Hopefully this provides clarity on how CpuModelInfo and its respective
fields are used in each command.  @David should be able to fill in any
missing areas / expand / offer corrections.

[...]

This helps, thanks!

Arguments that are silently ignored is bad interface design.

Observe: when CpuModelInfo is an argument, @deprecated-props is always
ignored.  When it's a return value, absent means {}, and it can be
present only for certain targets (currently S390).

The reason we end up with an argument we ignore is laziness: we use the
same type for both roles.  We can fix that easily:

     { 'struct': 'CpuModel',
       'data': { 'name': 'str',
                 '*props': 'any' } }

     { 'struct': 'CpuModelInfo',
       'base': 'CpuModel',
       'data': { '*deprecated-props': ['str'] } }

Use CpuModel for arguments, CpuModelInfo for return values.

Since @deprecated-props is used only by some targets, I'd make it
conditional, i.e. 'if': 'TARGET_S390X'.

If we want just query-cpu-model-expansion return deprecated properties,
we can instead move @deprecated-props from CpuModelInfo to
CpuModelExpansionInfo.

That might a bit more sense, because deprecated-props does not make any sense as input parameter, for example.

--
Cheers,

David / dhildenb


Reply via email to