5:24 p.m.
Overview
========
QEMU will soon support reporting an optional array of deprecated features for an expanded
CPU model via the query-cpu-model-expansion command. The intended use of this data is to
make it easier for a user to define a CPU model with features flagged as deprecated set to
disabled, thus rendering the guest migratable to future hardware that will out-right drop
support for said features.
Attached to this cover letter is only half of the bigger picture. I've updated the
CPU model expansion ABI to parse the new array (if it's available) and store the
result in a string list within the qemuMonitorCPUModelInfo struct. I also propose an
approach on how to store/retrieve the list of deprecated features in the qemuCaps cache
file. All feedback on this patch is certainly welcome. Please note: I do not provide any
updates to the respective qemuCaps tests right now.
The main goal of this post is to discuss the other half of the design: reporting and
enabling a CPU model with deprecated features disabled. I believe the ideal solution
involves a way for the user to easily configure their guest with this new data. Two ideas
I currently have are outlined below. Other approaches are encouraged.
Notes
=====
- In my example below, I am running on a z14.2 machine.
- The features that are flagged as deprecated for this model are: bpb, csske, cte, te.
- The discussion regarding the QEMU changes can be found here:
https://mail.gnu.org/archive/html/qemu-devel/2024-04/msg04605.html
Example of Desired Changes
==========================
Here is what I'd like the resulting guest's transient XML to look like for the
<cpu> section (I have trimmed the features list for brevity):
...
<cpu mode='custom' match='exact' check='partial'>
<model fallback='forbid'>z14.2-base</model>
<feature policy='require' name='aen'/>
<feature policy='require' name='cmmnt'/>
<feature policy='require' name='aefsi'/>
...
<feature policy='disable' name='cte'/> ***
<feature policy='require' name='ais'/>
<feature policy='disable' name='bpb'/> ***
<feature policy='require' name='ctop'/>
<feature policy='require' name='gs'/>
<feature policy='require' name='ppa15'/>
<feature policy='require' name='zpci'/>
<feature policy='require' name='sea_esop2'/>
<feature policy='disable' name='te'/> ***
<feature policy='require' name='cmm'/>
<feature policy='disable' name='csske'/> ***
</cpu>
...
Ideas
=====
New Host CPU Model
------------------
Create a new CPU model that is a mirror of the host CPU model with deprecated features
turned off. Let's call this model "host-recommended". A user may define
this model in the guest XML as they would any other CPU model:
<cpu mode='host-recommended' check='partial'/>
Just as how host-model works, anything defined nested in the <cpu> tag will be
ignored.
This model could potentially be listed in the domcapabilities output after the
host-model:
<cpu>
<mode name='host-passthrough' supported='yes'>
...
</mode>
...
<mode name='host-model' supported='yes'>
...
</mode>
<mode name='host-recommended' supported='yes'>
...
<feature policy='disable' name='cte'/>
<feature policy='require' name='ais'/>
<feature policy='disable' name='bpb'/>
<feature policy='require' name='ctop'/>
<feature policy='require' name='gs'/>
<feature policy='require' name='ppa15'/>
<feature policy='require' name='zpci'/>
<feature policy='require' name='sea_esop2'/>
<feature policy='disable' name='te'/>
<feature policy='require' name='cmm'/>
<feature policy='disable' name='csske'/>
</cpu>
New Nested Element Under <cpu>
------------------------------
Create a new optional XML element nested under the <cpu> tag that may be used to
disable deprecated features. This approach is more explicit compared to creating a new
CPU model, and allows the user to disable these features when defining a specific model
other than host-model. Here is an example of what the guest's defined XML for the CPU
could look like:
<cpu mode='host-model' check='partial'>
<deprecated_features>off</deprecated_features>
</cpu>
However, a conflict arises with this approach: parameter priority. It would need to be
discussed what the expected behavior should be if a user defines a guest with both a mode
to disable deprecated features and any deprecated features listed with the
'require' policy, e.g.:
<cpu mode='custom' match='exact' check='partial'>
<model fallback='allow'>z13.2-base</model>
<!-- which one takes priority? -->
<deprecated_features>off</deprecated_features>
<feature policy='require' name='csske'/>
</cpu>
Another conflict is setting this option to "on" would have no effect on the CPU
model (I can't think of a reason why someone would want to explicitly enable these
features). This may not communicate well to the user.
To report these features, a <deprecatedProperties> tag could be added to the
domcapabilities output using the same format I use in my proposed patch for the qemu
capabilities file:
<cpu>
<mode name='host-passthrough' supported='yes'>
...
</mode>
...
<mode name='host-model' supported='yes'>
...
</mode>
<deprecatedProperties>
<property name='bpb'/>
<property name='te'/>
<property name='cte'/>
<property name='csske'/>
</deprecatedProperties>
</cpu>
Please let me know your thoughts. Once an approach is agreed upon, I will begin
development.
Collin Walling (1):
qemu: monitor: parse deprecated-props from query-cpu-model-expansion
response
src/qemu/qemu_capabilities.c | 30 ++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++-----
3 files changed, 56 insertions(+), 5 deletions(-)
--
2.43.0
5:24 p.m.
New subject: [RFC PATCH 1/1] qemu: monitor: parse deprecated-props from query-cpu-model-expansion response
query-cpu-model-expansion may report an array of deprecated properties.
This array is optional, and may not be supported for a particular
architecture or reported for a particular CPU model. If the output is
present, then capture it and store in a qemuMonitorCPUModelInfo struct
for later use.
Baseline does not intend to use this feature, so pass NULL in place of
the respective parameter in the relevant functions.
The deprecated features will be retained in qemuCaps->kvm->hostCPU.info
and will be stored in the capabilities cache file under the <hostCPU>
element using the following format:
<deprecatedProperties>
<property name='bpb'/>
<property name='csske'/>
<property name='cte'/>
<property name='te'/>
</deprecatedProperties>
Signed-off-by: Collin Walling <walling(a)linux.ibm.com>
---
src/qemu/qemu_capabilities.c | 30 ++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++-----
3 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 45525db803..5c36db27f8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4028,6 +4028,24 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps,
}
}
+ ctxt->node = hostCPUNode;
+
+ if (virXPathNodeSet("./deprecatedProperties", ctxt, &nodes) > 0
&&
+ (n = virXPathNodeSet("./property", ctxt, &nodes)) > 0) {
+ hostCPU->deprecated_props = g_new0(char *, n);
+ hostCPU->ndeprecated_props = n;
+
+ for (i = 0; i < n; i++) {
+ ctxt->node = nodes[i];
+
+ if (!(hostCPU->deprecated_props[i] = virXMLPropString(ctxt->node,
"name"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing 'name' attribute for a host CPU
model deprecated property in QEMU capabilities cache"));
+ return -1;
+ }
+ }
+ }
+
caps->hostCPU.info = g_steal_pointer(&hostCPU);
return 0;
}
@@ -4760,6 +4778,18 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsAccel *caps,
virBufferAddLit(buf, "/>\n");
}
+ if (model->deprecated_props) {
+ virBufferAddLit(buf, "<deprecatedProperties>\n");
+ virBufferAdjustIndent(buf, 2);
+
+ for (i = 0; i < model->ndeprecated_props; i++)
+ virBufferAsprintf(buf, "<property name='%s'/>\n",
+ model->deprecated_props[i]);
+
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</deprecatedProperties>\n");
+ }
+
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</hostCPU>\n");
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 6e81945201..1a2444b721 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1140,6 +1140,8 @@ struct _qemuMonitorCPUModelInfo {
char *name;
size_t nprops;
qemuMonitorCPUProperty *props;
+ size_t ndeprecated_props;
+ char **deprecated_props;
bool migratability;
};
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index eb84a3d938..39dc709693 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5006,6 +5006,7 @@ qemuMonitorJSONParseCPUModelData(virJSONValue *data,
bool fail_no_props,
virJSONValue **cpu_model,
virJSONValue **cpu_props,
+ virJSONValue **cpu_deprecated_props,
const char **cpu_name)
{
if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) {
@@ -5027,6 +5028,12 @@ qemuMonitorJSONParseCPUModelData(virJSONValue *data,
return -1;
}
+ /* Unconditionally check for the deprecated-props array, as it is not
+ * a guarantee response even if QEMU supports it. */
+ if (cpu_deprecated_props)
+ *cpu_deprecated_props = virJSONValueObjectGetArray(*cpu_model,
+
"deprecated-props");
+
return 0;
}
@@ -5034,6 +5041,7 @@ qemuMonitorJSONParseCPUModelData(virJSONValue *data,
static int
qemuMonitorJSONParseCPUModel(const char *cpu_name,
virJSONValue *cpu_props,
+ virJSONValue *cpu_deprecated_props,
qemuMonitorCPUModelInfo **model_info)
{
g_autoptr(qemuMonitorCPUModelInfo) machine_model = NULL;
@@ -5052,6 +5060,16 @@ qemuMonitorJSONParseCPUModel(const char *cpu_name,
return -1;
}
+ if (cpu_deprecated_props) {
+ size_t ndeprecated_props = virJSONValueArraySize(cpu_deprecated_props);
+ machine_model->deprecated_props = g_new0(char *, ndeprecated_props);
+
+ if (!(machine_model->deprecated_props =
virJSONValueArrayToStringList(cpu_deprecated_props)))
+ return -1;
+
+ machine_model->ndeprecated_props = ndeprecated_props;
+ }
+
*model_info = g_steal_pointer(&machine_model);
return 0;
}
@@ -5116,6 +5134,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitor *mon,
g_autoptr(virJSONValue) fullData = NULL;
virJSONValue *cpu_model;
virJSONValue *cpu_props = NULL;
+ virJSONValue *cpu_deprecated_props = NULL;
const char *cpu_name = "";
int rc;
@@ -5129,7 +5148,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitor *mon,
if (qemuMonitorJSONParseCPUModelData(data, "query-cpu-model-expansion",
fail_no_props, &cpu_model, &cpu_props,
- &cpu_name) < 0)
+ &cpu_deprecated_props, &cpu_name) <
0)
return -1;
/* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion
@@ -5147,11 +5166,11 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitor *mon,
if (qemuMonitorJSONParseCPUModelData(fullData,
"query-cpu-model-expansion",
fail_no_props, &cpu_model,
&cpu_props,
- &cpu_name) < 0)
+ &cpu_deprecated_props, &cpu_name)
< 0)
return -1;
}
- return qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_info);
+ return qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, cpu_deprecated_props,
model_info);
}
@@ -5188,10 +5207,10 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitor *mon,
if (qemuMonitorJSONParseCPUModelData(data, "query-cpu-model-baseline",
false, &cpu_model, &cpu_props,
- &cpu_name) < 0)
+ NULL, &cpu_name) < 0)
return -1;
- return qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, baseline);
+ return qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, NULL, baseline);
}
--
2.43.0
6:29 a.m.
New subject: [RFC PATCH 0/1] support deprecated-props from
query-cpu-model-expansion
Polite ping. Would like some feedback / discussion on this before I
move forward. Thanks! :)
9:40 a.m.
New subject: [RFC PATCH 0/1] support deprecated-props from
query-cpu-model-expansion
On Tue, May 07, 2024 at 18:24:20 -0400, Collin Walling wrote:
QEMU will soon support reporting an optional array of deprecated
features for an expanded CPU model via the query-cpu-model-expansion
command. The intended use of this data is to make it easier for a
user to define a CPU model with features flagged as deprecated set to
disabled, thus rendering the guest migratable to future hardware that
will out-right drop support for said features.
So is the list of deprecated features a static list common to all CPU
models or does it depend on the CPU model? If it's static getting the
list via another QMP command (in addition to query-cpu-model-expansion)
would make sense. I would expect the query-cpu-model-expansion to limit
deprecated features to only those actually used by the expanded CPU
model, which would make constructing a complete list quite hard.
On the other hand, if the deprecated features depend on a CPU model,
we'd need a way to list all CPU models and their deprecated features to
be able to report such info in domain capabilities XML. Ideally without
having to call query-cpu-model-expansion on every single CPU model.
Notes
=====
- In my example below, I am running on a z14.2 machine.
- The features that are flagged as deprecated for this model are: bpb, csske, cte, te.
OK, so the deprecated features seem to depend on a specific CPU model.
Does query-cpu-model-expansion list them even if they are not specified
in the CPU model we want to expand? If not, we need another interface to
be able to report this info.
Ideas
=====
New Host CPU Model
------------------
Create a new CPU model that is a mirror of the host CPU model with deprecated features
turned off. Let's call this model "host-recommended". A user may define
this model in the guest XML as they would any other CPU model:
<cpu mode='host-recommended' check='partial'/>
Just as how host-model works, anything defined nested in the <cpu> tag will be
ignored.
This model could potentially be listed in the domcapabilities output after the
host-model:
<cpu>
<mode name='host-passthrough' supported='yes'>
...
</mode>
...
<mode name='host-model' supported='yes'>
...
</mode>
<mode name='host-recommended' supported='yes'>
...
<feature policy='disable' name='cte'/>
<feature policy='require' name='ais'/>
<feature policy='disable' name='bpb'/>
<feature policy='require' name='ctop'/>
<feature policy='require' name='gs'/>
<feature policy='require' name='ppa15'/>
<feature policy='require' name='zpci'/>
<feature policy='require' name='sea_esop2'/>
<feature policy='disable' name='te'/>
<feature policy='require' name='cmm'/>
<feature policy='disable' name='csske'/>
</cpu>
New Nested Element Under <cpu>
------------------------------
Create a new optional XML element nested under the <cpu> tag that may be used to
disable deprecated features. This approach is more explicit compared to creating a new
CPU model, and allows the user to disable these features when defining a specific model
other than host-model. Here is an example of what the guest's defined XML for the CPU
could look like:
<cpu mode='host-model' check='partial'>
<deprecated_features>off</deprecated_features>
</cpu>
However, a conflict arises with this approach: parameter priority. It would need to be
discussed what the expected behavior should be if a user defines a guest with both a mode
to disable deprecated features and any deprecated features listed with the
'require' policy, e.g.:
<cpu mode='custom' match='exact' check='partial'>
<model fallback='allow'>z13.2-base</model>
<!-- which one takes priority? -->
<deprecated_features>off</deprecated_features>
<feature policy='require' name='csske'/>
</cpu>
Another conflict is setting this option to "on" would have no effect on the CPU
model (I can't think of a reason why someone would want to explicitly enable these
features). This may not communicate well to the user.
I think have a separate configuration is better as it does not limit the
used to just a single CPU model. But a nested element with a text node
looks strange. An optional attribute for the <cpu> element would be
better.
For host-model the expected behavior would be to either keep or
drop deprecated features from the CPU definition. When combined with
custom CPU mode where such feature may be already present I can imagine
three different options. Either keep deprecated features (that is do
nothing, just like we do now), drop such features silently, or fail to
start a domain in case the definition uses a deprecated feature.
We could even create the attribute, but limit it only to host-model,
which would be mostly equivalent to your "host-recomended" mode, but
extensible to other modes in the future.
To report these features, a <deprecatedProperties> tag could be
added to the domcapabilities output using the same format I use in my proposed patch for
the qemu capabilities file:
<cpu>
<mode name='host-passthrough' supported='yes'>
...
</mode>
...
<mode name='host-model' supported='yes'>
...
</mode>
<deprecatedProperties>
<property name='bpb'/>
<property name='te'/>
<property name='cte'/>
<property name='csske'/>
</deprecatedProperties>
</cpu>
We should stick with "features" rather than calling them "properties"
here to avoid confusion. Also this schema would mean the list of
deprecated features is indeed the same for all CPU models, which does
not seem to be the case here.
And one more interesting point: what to do with the baseline CPU
computation which expects to see host-model definitions from all hosts.
We'd need a way to provide the info about deprecated features to the
input data for baseline computation. In other words, to the host-model
definition itself. We could, for example, add a deprecated='yes|no'
attribute to each feature in the host-model definition. But this won't
really work when the list of deprecated CPU features depends on a CPU
model as the baseline may choose a different CPU model than what is used
by a host-model. So perhaps we may just rely on the host computing the
baseline CPU to handle deprecated features on the result and document
this computation should be done on the most up-to-date host so to avoid
missing features that were deprecated in a later update.
I feel like there's more questions than answers in my reply. Sorry about
that and the delay.
Jirka
10:42 a.m.
New subject: [RFC PATCH 0/1] support deprecated-props from
query-cpu-model-expansion
On 6/3/24 10:40 AM, Jiri Denemark wrote:
On Tue, May 07, 2024 at 18:24:20 -0400, Collin Walling wrote:
> QEMU will soon support reporting an optional array of deprecated
> features for an expanded CPU model via the query-cpu-model-expansion
> command. The intended use of this data is to make it easier for a
> user to define a CPU model with features flagged as deprecated set to
> disabled, thus rendering the guest migratable to future hardware that
> will out-right drop support for said features.
So is the list of deprecated features a static list common to all CPU
models or does it depend on the CPU model? If it's static getting the
list via another QMP command (in addition to query-cpu-model-expansion)
would make sense. I would expect the query-cpu-model-expansion to limit
deprecated features to only those actually used by the expanded CPU
model, which would make constructing a complete list quite hard.
On the other hand, if the deprecated features depend on a CPU model,
we'd need a way to list all CPU models and their deprecated features to
be able to report such info in domain capabilities XML. Ideally without
having to call query-cpu-model-expansion on every single CPU model.
> Notes
> =====
>
> - In my example below, I am running on a z14.2 machine.
>
> - The features that are flagged as deprecated for this model are: bpb, csske, cte,
te.
OK, so the deprecated features seem to depend on a specific CPU model.
Does query-cpu-model-expansion list them even if they are not specified
in the CPU model we want to expand? If not, we need another interface to
be able to report this info.
The QEMU portion is designed for s390x such that there is a static list
of hardcoded feature bits that are flagged for deprecation. This list
can be updated in follow-up patches as more features need to be flagged.
The query-cpu-model-expansion *response* has been modified to now
include an /optional/ "deprecated props" string array (no additional
parameter is required in the query JSON). This array will be present if
and only if there is data to be reported. If an architecture does not
support reporting deprecated features, or if there are no deprecated
features to report, then array is simply omitted from the response.
The deprecated features are filtered against the full-expansion features
for the particular CPU model. In the example for a z14, we would expect
all four currently deprecated features reported: bpb, csske, cte, and
te, since those features are part of the z14 full expansion. On an
older model, say a z900, you'd only see bpb because the others are not
present in the full-expansion.
> Ideas
> =====
>
> New Host CPU Model
> ------------------
>
> Create a new CPU model that is a mirror of the host CPU model with deprecated
features turned off. Let's call this model "host-recommended". A user may
define this model in the guest XML as they would any other CPU model:
>
> <cpu mode='host-recommended' check='partial'/>
>
> Just as how host-model works, anything defined nested in the <cpu> tag will be
ignored.
>
> This model could potentially be listed in the domcapabilities output after the
host-model:
>
> <cpu>
> <mode name='host-passthrough' supported='yes'>
> ...
> </mode>
> ...
> <mode name='host-model' supported='yes'>
> ...
> </mode>
> <mode name='host-recommended' supported='yes'>
> ...
> <feature policy='disable' name='cte'/>
> <feature policy='require' name='ais'/>
> <feature policy='disable' name='bpb'/>
> <feature policy='require' name='ctop'/>
> <feature policy='require' name='gs'/>
> <feature policy='require' name='ppa15'/>
> <feature policy='require' name='zpci'/>
> <feature policy='require' name='sea_esop2'/>
> <feature policy='disable' name='te'/>
> <feature policy='require' name='cmm'/>
> <feature policy='disable' name='csske'/>
> </cpu>
>
>
> New Nested Element Under <cpu>
> ------------------------------
>
> Create a new optional XML element nested under the <cpu> tag that may be used
to disable deprecated features. This approach is more explicit compared to creating a new
CPU model, and allows the user to disable these features when defining a specific model
other than host-model. Here is an example of what the guest's defined XML for the CPU
could look like:
>
> <cpu mode='host-model' check='partial'>
> <deprecated_features>off</deprecated_features>
> </cpu>
>
> However, a conflict arises with this approach: parameter priority. It would need to
be discussed what the expected behavior should be if a user defines a guest with both a
mode to disable deprecated features and any deprecated features listed with the
'require' policy, e.g.:
>
> <cpu mode='custom' match='exact' check='partial'>
> <model fallback='allow'>z13.2-base</model>
> <!-- which one takes priority? -->
> <deprecated_features>off</deprecated_features>
> <feature policy='require' name='csske'/>
> </cpu>
>
> Another conflict is setting this option to "on" would have no effect on the
CPU model (I can't think of a reason why someone would want to explicitly enable these
features). This may not communicate well to the user.
I think have a separate configuration is better as it does not limit the
used to just a single CPU model. But a nested element with a text node
looks strange. An optional attribute for the <cpu> element would be
better.
> For host-model the expected behavior would be to either keep or
drop deprecated features from the CPU definition. When combined with
custom CPU mode where such feature may be already present I can imagine
three different options. Either keep deprecated features (that is do
nothing, just like we do now), drop such features silently, or fail to
start a domain in case the definition uses a deprecated feature.
I fear the last option may break some things?
We could even create the attribute, but limit it only to host-model,
which would be mostly equivalent to your "host-recomended" mode, but
extensible to other modes in the future.
I think this is the better approach.
In tandem to your suggestion to add a new attribute to the cpu element,
a quick mock-up of it could look like this:
<cpu mode='host-model' deprecated_features='off'>
and output an error if any other mode is used.
> To report these features, a <deprecatedProperties> tag
could be added to the domcapabilities output using the same format I use in my proposed
patch for the qemu capabilities file:
>
> <cpu>
> <mode name='host-passthrough' supported='yes'>
> ...
> </mode>
> ...
> <mode name='host-model' supported='yes'>
> ...
> </mode>
> <deprecatedProperties>
> <property name='bpb'/>
> <property name='te'/>
> <property name='cte'/>
> <property name='csske'/>
> </deprecatedProperties>
> </cpu>
We should stick with "features" rather than calling them
"properties"
here to avoid confusion. Also this schema would mean the list of
deprecated features is indeed the same for all CPU models, which does
not seem to be the case here.
Noted to use "features" instead of "properties".
And you are correct that this makes the assumption that these features
are available for *all* CPU models -- I did not think of this
perspective. This is populated from a query-cpu-model-expansion based
on "host-model".
What would be a better way to report these features in the
domcapabilities response? Perhaps they should only be nested under the
host-model?
...another thought would be to implement a new feature policy
"deprecated" that would allow these XML features to be applied to any
CPU model. This would flag the feature to either be disabled if it's
available for that model, or ignore it otherwise. The validity of the
feature would be checked against the list of deprecated features (to
avoid typos or having a user define an imaginary feature).
Additionally, the feature policy would evaluate to "disabled" for a live
guest XML to alleviate migration issues on machines that do not know
about this new policy.
Consider the following for domcapabilities output:
<cpu>
<mode name='host-passthrough' supported='yes'>
...
</mode>
...
<mode name='host-model' supported='yes'>
...
</mode>
<deprecatedFeatures>
<feature policy='deprecated' name='bpb'/>
<feature policy='deprecated' name='te'/>
<feature policy='deprecated' name='cte'/>
<feature policy='deprecated' name='csske'/>
</deprecatedProperties>
</cpu>
The benefit here is that this would let a user easily copy+paste these
features into their guest XML and not have to consider whether or not
these features are available for the CPU model they wish to use.
Alternatively, the <mode name='host-model' ...> could include the
aforementioned deprecated_features='off' attribute and the nested
features would include the analogous features with policy='disabled'.
This provides the benefit of having a near ready-to-use CPU model XML
with deprecated features that can be provided to the baseline input.
I know I sort of brain-dumped a lot here. Let me know if you'd like for
me to format these ideas into a fashion similar to the initial cover
(maybe an RFC v2) to make discussion threads more readable.
And one more interesting point: what to do with the baseline CPU
computation which expects to see host-model definitions from all hosts.
We'd need a way to provide the info about deprecated features to the
input data for baseline computation. In other words, to the host-model
definition itself. We could, for example, add a deprecated='yes|no'
attribute to each feature in the host-model definition. But this won't
really work when the list of deprecated CPU features depends on a CPU
model as the baseline may choose a different CPU model than what is used
by a host-model. So perhaps we may just rely on the host computing the
baseline CPU to handle deprecated features on the result and document
this computation should be done on the most up-to-date host so to avoid
missing features that were deprecated in a later update.
The way I imagined this working was to either use the "host-recommended"
CPU model by default (but I believe we are nix'ing that approach), OR
just use the host-model as-is today and rely on the input XML containing
the appropriate list of deprecated features (with policy=disabled). The
input XML containing this CPU model could be extracted from a live guest
that has deprecated features disabled.
I feel like there's more questions than answers in my reply.
Sorry about
that and the delay.
No worries at all! This is exactly why I wanted to post this as an RFC
and before diving too deeply into the code. Your questions and feedback
are welcome here :)
In summary:
- s/properties/features
- potentially add a new attribute to the cpu element to disable
deprecated features instead of a nested element (Jirka's idea)
- discuss the potential of a new feature policy "deprecated" (Collin's
idea)
- how to handle baseline needs more discussion
To echo part of the discussion from Daniel's feedback, he seems in favor
of providing an explicit indication on the cpu definition that
deprecated features are on|off. I think this works in tandem with a
deprecated_features attribute in the <cpu> element?
Jirka
Thanks for the response!
--
Regards,
Collin
6:22 a.m.
New subject: [RFC PATCH 0/1] support deprecated-props from
query-cpu-model-expansion
On Tue, Jun 04, 2024 at 11:42:25 -0400, Collin Walling wrote:
The QEMU portion is designed for s390x such that there is a static
list
of hardcoded feature bits that are flagged for deprecation. This list
can be updated in follow-up patches as more features need to be flagged.
Good, a single static list should definitely be easier to handle.
The query-cpu-model-expansion *response* has been modified to now
include an /optional/ "deprecated props" string array (no additional
parameter is required in the query JSON). This array will be present if
and only if there is data to be reported. If an architecture does not
support reporting deprecated features, or if there are no deprecated
features to report, then array is simply omitted from the response.
The deprecated features are filtered against the full-expansion features
for the particular CPU model. In the example for a z14, we would expect
all four currently deprecated features reported: bpb, csske, cte, and
te, since those features are part of the z14 full expansion. On an
older model, say a z900, you'd only see bpb because the others are not
present in the full-expansion.
In that case I think it would be nice if QEMU provided a way of getting
the list itself without any filtering. Mainly to make sure we're
providing a completely list for users which may decide to avoid using
such features in their non-host-model CPU definitions. And I can imagine
management layers might want to get a complete list too.
>> Another conflict is setting this option to "on"
would have no
>> effect on the CPU model (I can't think of a reason why someone
>> would want to explicitly enable these features). This may not
>> communicate well to the user.
>
> I think have a separate configuration is better as it does not limit the
> used to just a single CPU model. But a nested element with a text node
> looks strange. An optional attribute for the <cpu> element would be
> better.
>> For host-model the expected behavior would be to either keep or
> drop deprecated features from the CPU definition. When combined with
> custom CPU mode where such feature may be already present I can imagine
> three different options. Either keep deprecated features (that is do
> nothing, just like we do now), drop such features silently, or fail to
> start a domain in case the definition uses a deprecated feature.
I fear the last option may break some things?
The idea is the behavior would be controllable via a special value in
that attribute, which a used would have to explicitly specify to get
such behavior. In other words, a user would provide a custom CPU
definition and ask for deprecated features to be removed no matter what
they are currently. But I don't think we need to care about it now
because...
> We could even create the attribute, but limit it only to
host-model,
> which would be mostly equivalent to your "host-recomended" mode, but
> extensible to other modes in the future.
I think this is the better approach.
Agreed. Just do the minimal required thing while making it extensible.
In tandem to your suggestion to add a new attribute to the cpu
element,
a quick mock-up of it could look like this:
<cpu mode='host-model' deprecated_features='off'>
Right. Although you may have fun coming with an attribute name, because
we are not very consistent with naming multi-word attributes. Some
people don't like underscores while others don't like camelCase :-)
Anyway, I think you should at least be consistent and use a single name
for the attribute in domain XML and the element in domcapabilities XML.
>> To report these features, a <deprecatedProperties> tag
could be
>> added to the domcapabilities output using the same format I use in
>> my proposed patch for the qemu capabilities file:
>>
>> <cpu>
>> <mode name='host-passthrough' supported='yes'>
>> ...
>> </mode>
>> ...
>> <mode name='host-model' supported='yes'>
>> ...
>> </mode>
>> <deprecatedProperties>
>> <property name='bpb'/>
>> <property name='te'/>
>> <property name='cte'/>
>> <property name='csske'/>
>> </deprecatedProperties>
>> </cpu>
>
> We should stick with "features" rather than calling them
"properties"
> here to avoid confusion. Also this schema would mean the list of
> deprecated features is indeed the same for all CPU models, which does
> not seem to be the case here.
Noted to use "features" instead of "properties".
And you are correct that this makes the assumption that these features
are available for *all* CPU models -- I did not think of this
perspective. This is populated from a query-cpu-model-expansion based
on "host-model".
What would be a better way to report these features in the
domcapabilities response? Perhaps they should only be nested under the
host-model?
Well, the list is static and common to all CPU models so keeping it as
another element under <cpu> as you suggested looks fine.
...another thought would be to implement a new feature policy
"deprecated" that would allow these XML features to be applied to any
CPU model. This would flag the feature to either be disabled if it's
available for that model, or ignore it otherwise. The validity of the
feature would be checked against the list of deprecated features (to
avoid typos or having a user define an imaginary feature).
Additionally, the feature policy would evaluate to "disabled" for a live
guest XML to alleviate migration issues on machines that do not know
about this new policy.
Consider the following for domcapabilities output:
<cpu>
<mode name='host-passthrough' supported='yes'>
...
</mode>
...
<mode name='host-model' supported='yes'>
...
</mode>
<deprecatedFeatures>
<feature policy='deprecated' name='bpb'/>
<feature policy='deprecated' name='te'/>
<feature policy='deprecated' name='cte'/>
<feature policy='deprecated' name='csske'/>
</deprecatedProperties>
</cpu>
I think this is a little bit too complicated.
The benefit here is that this would let a user easily copy+paste
these
features into their guest XML and not have to consider whether or not
these features are available for the CPU model they wish to use.
They should always be able to mark all deprecated features as disabled
if they want. I mean a feature not used by a specific CPU model is
disabled and disabling it explicitly in the XML does not change
anything. Or is s390x different and using a specific CPU model would
also limit what extra features you can list even if you just disable
them?
Alternatively, the <mode name='host-model' ...> could
include the
aforementioned deprecated_features='off' attribute and the nested
features would include the analogous features with policy='disabled'.
This provides the benefit of having a near ready-to-use CPU model XML
with deprecated features that can be provided to the baseline input.
Introducing a new value for an attribute would cause parse failures when
the XML is passed to an older libvirt. I think we should just take
host-model CPUs from all hosts, compute the baseline and drop deprecated
features from the result. This would of course need a new flag for the
baseline API. This way we could also take host-model from a single host
and let deprecated features be removed from it. Internally either the
baseline QMP command would also provide a list of deprecated features or
we could just take the result and let QEMU expand it to get the list.
The way I imagined this working was to either use the
"host-recommended"
CPU model by default (but I believe we are nix'ing that approach), OR
just use the host-model as-is today and rely on the input XML containing
the appropriate list of deprecated features (with policy=disabled). The
input XML containing this CPU model could be extracted from a live guest
that has deprecated features disabled.
Or we could perhaps add a flag for domcapabilities API to show the
host-model with deprecated features already disabled. This way we could
avoid introducing a new policy while letting the originating host limit
the disabled features itself. Or we could do both... limit the input and
also removed the deprecated features from the result :-)
To echo part of the discussion from Daniel's feedback, he seems
in favor
of providing an explicit indication on the cpu definition that
deprecated features are on|off. I think this works in tandem with a
deprecated_features attribute in the <cpu> element?
Yeah, I think that's exactly what the attribute can do.
Jirka
4:42 p.m.
New subject: [RFC PATCH 0/1] support deprecated-props from
query-cpu-model-expansion
On 6/13/24 7:22 AM, Jiri Denemark wrote:
On Tue, Jun 04, 2024 at 11:42:25 -0400, Collin Walling wrote:
> The QEMU portion is designed for s390x such that there is a static list
> of hardcoded feature bits that are flagged for deprecation. This list
> can be updated in follow-up patches as more features need to be flagged.
Good, a single static list should definitely be easier to handle.
> The query-cpu-model-expansion *response* has been modified to now
> include an /optional/ "deprecated props" string array (no additional
> parameter is required in the query JSON). This array will be present if
> and only if there is data to be reported. If an architecture does not
> support reporting deprecated features, or if there are no deprecated
> features to report, then array is simply omitted from the response.
>
> The deprecated features are filtered against the full-expansion features
> for the particular CPU model. In the example for a z14, we would expect
> all four currently deprecated features reported: bpb, csske, cte, and
> te, since those features are part of the z14 full expansion. On an
> older model, say a z900, you'd only see bpb because the others are not
> present in the full-expansion.
In that case I think it would be nice if QEMU provided a way of getting
the list itself without any filtering. Mainly to make sure we're
providing a completely list for users which may decide to avoid using
such features in their non-host-model CPU definitions. And I can imagine
management layers might want to get a complete list too.
Okay. I will need to bring this up on the QEMU devel list to figure out
the best approach to report this list. I do not believe this hinders
development for designing a way for a user to specify host-model with
deprecated features disabled, but I understand that this would be
valuable to users who want to define a custom model with deprecated
features off and management applications may want this list.
I'll mention the action items for this request at the bottom of this email.
>>> Another conflict is setting this option to "on"
would have no
>>> effect on the CPU model (I can't think of a reason why someone
>>> would want to explicitly enable these features). This may not
>>> communicate well to the user.
>>
>> I think have a separate configuration is better as it does not limit the
>> used to just a single CPU model. But a nested element with a text node
>> looks strange. An optional attribute for the <cpu> element would be
>> better.
>>> For host-model the expected behavior would be to either keep or
>> drop deprecated features from the CPU definition. When combined with
>> custom CPU mode where such feature may be already present I can imagine
>> three different options. Either keep deprecated features (that is do
>> nothing, just like we do now), drop such features silently, or fail to
>> start a domain in case the definition uses a deprecated feature.
>
> I fear the last option may break some things?
The idea is the behavior would be controllable via a special value in
that attribute, which a used would have to explicitly specify to get
such behavior. In other words, a user would provide a custom CPU
definition and ask for deprecated features to be removed no matter what
they are currently. But I don't think we need to care about it now
because...
>> We could even create the attribute, but limit it only to host-model,
>> which would be mostly equivalent to your "host-recomended" mode, but
>> extensible to other modes in the future.
>
> I think this is the better approach.
Agreed. Just do the minimal required thing while making it extensible.
Sounds good!
> In tandem to your suggestion to add a new attribute to the cpu
element,
> a quick mock-up of it could look like this:
>
> <cpu mode='host-model' deprecated_features='off'>
Right. Although you may have fun coming with an attribute name, because
we are not very consistent with naming multi-word attributes. Some
people don't like underscores while others don't like camelCase :-)
Anyway, I think you should at least be consistent and use a single name
for the attribute in domain XML and the element in domcapabilities XML.
Sounds fun! I'll try to come up with a reasonable name on the
first run and discuss afterwards.
>>> To report these features, a <deprecatedProperties>
tag could be
>>> added to the domcapabilities output using the same format I use in
>>> my proposed patch for the qemu capabilities file:
>>>
>>> <cpu>
>>> <mode name='host-passthrough' supported='yes'>
>>> ...
>>> </mode>
>>> ...
>>> <mode name='host-model' supported='yes'>
>>> ...
>>> </mode>
>>> <deprecatedProperties>
>>> <property name='bpb'/>
>>> <property name='te'/>
>>> <property name='cte'/>
>>> <property name='csske'/>
>>> </deprecatedProperties>
>>> </cpu>
>>
>> We should stick with "features" rather than calling them
"properties"
>> here to avoid confusion. Also this schema would mean the list of
>> deprecated features is indeed the same for all CPU models, which does
>> not seem to be the case here.
>
> Noted to use "features" instead of "properties".
>
> And you are correct that this makes the assumption that these features
> are available for *all* CPU models -- I did not think of this
> perspective. This is populated from a query-cpu-model-expansion based
> on "host-model".
>
> What would be a better way to report these features in the
> domcapabilities response? Perhaps they should only be nested under the
> host-model?
Well, the list is static and common to all CPU models so keeping it as
another element under <cpu> as you suggested looks fine.
Okay. I will retain this output (with a better element name). If this
no longer makes sense as development progresses, I can easily drop it :)
> ...another thought would be to implement a new feature policy
> "deprecated" that would allow these XML features to be applied to any
> CPU model. This would flag the feature to either be disabled if it's
> available for that model, or ignore it otherwise. The validity of the
> feature would be checked against the list of deprecated features (to
> avoid typos or having a user define an imaginary feature).
> Additionally, the feature policy would evaluate to "disabled" for a live
> guest XML to alleviate migration issues on machines that do not know
> about this new policy.
>
> Consider the following for domcapabilities output:
>
> <cpu>
> <mode name='host-passthrough' supported='yes'>
> ...
> </mode>
> ...
> <mode name='host-model' supported='yes'>
> ...
> </mode>
> <deprecatedFeatures>
> <feature policy='deprecated' name='bpb'/>
> <feature policy='deprecated' name='te'/>
> <feature policy='deprecated' name='cte'/>
> <feature policy='deprecated' name='csske'/>
> </deprecatedProperties>
> </cpu>
I think this is a little bit too complicated.
Noted.
> The benefit here is that this would let a user easily copy+paste
these
> features into their guest XML and not have to consider whether or not
> these features are available for the CPU model they wish to use.
They should always be able to mark all deprecated features as disabled
if they want. I mean a feature not used by a specific CPU model is
disabled and disabling it explicitly in the XML does not change
anything. Or is s390x different and using a specific CPU model would
also limit what extra features you can list even if you just disable
them?
s390x is not any different. If you specify, say, a z14 and
provide+disable a CPU feature that was introduced in a future model,
(e.g. msa9, a gen15a feature), the guest will operate without any issues.
To be concise, here is the output of z14 with msa9=on|off:
# qemu-system-s390x ... -cpu z14,msa9=on
qemu-system-s390x: can't apply global z14-s390x-cpu.msa9=on: Group
'msa9' is not available for CPU model 'z14', it was introduced with
later models.
# qemu-system-s390x ... -cpu z14,msa9=off
/guest starts up successfully/
A bit of insight: future CPU models that outright drop the deprecated
features should have similar behavior: feature=on should say something
like "no longer supported" (outside the scope of what we're aiming to do
here), feature=off should just start the guest successfully --
definitely within scope :)
> Alternatively, the <mode name='host-model' ...>
could include the
> aforementioned deprecated_features='off' attribute and the nested
> features would include the analogous features with policy='disabled'.
> This provides the benefit of having a near ready-to-use CPU model XML
> with deprecated features that can be provided to the baseline input.
Introducing a new value for an attribute would cause parse failures when
the XML is passed to an older libvirt. I think we should just take
host-model CPUs from all hosts, compute the baseline and drop deprecated
features from the result. This would of course need a new flag for the
baseline API. This way we could also take host-model from a single host
and let deprecated features be removed from it. Internally either the
baseline QMP command would also provide a list of deprecated features or
we could just take the result and let QEMU expand it to get the list.
I like this idea. If it's acceptable to add a new flag to this command,
that'd be really nice. s390x utilizes the hypervisor-cpu-baseline
command, but would this flag need to be added to the analogous
cpu-baseline command as well?
> The way I imagined this working was to either use the
"host-recommended"
> CPU model by default (but I believe we are nix'ing that approach), OR
> just use the host-model as-is today and rely on the input XML containing
> the appropriate list of deprecated features (with policy=disabled). The
> input XML containing this CPU model could be extracted from a live guest
> that has deprecated features disabled.
Or we could perhaps add a flag for domcapabilities API to show the
host-model with deprecated features already disabled. This way we could
avoid introducing a new policy while letting the originating host limit
the disabled features itself. Or we could do both... limit the input and
also removed the deprecated features from the result :-)
So if I understand this correctly, you're saying there could be two ways
of defining a guest with deprecated features turned off:
1. define the guest with host-model and the aforementioned
"deprecated_features=off" attribute
--- OR ---
2. define the guest by copy+pasting the resulting host-model from a
virsh domcapabiliites --deprecated-features-off (pardon the messy
prototype flag name :P )
> To echo part of the discussion from Daniel's feedback, he
seems in favor
> of providing an explicit indication on the cpu definition that
> deprecated features are on|off. I think this works in tandem with a
> deprecated_features attribute in the <cpu> element?
Yeah, I think that's exactly what the attribute can do.
Jirka
Awesome! I think my only lingering question is the one above to make
sure I'm aligning my understanding with your feedback.
This information should be sufficient for me to continue development and
propose a reasonable v1 for this. Here's the plan:
- implement the "deprecated_features" attribute for host-model *only*
(with a better name)
- add a new flag for baseline that will ensure the result turns off
deprecated features for the resulting model (should be easy since I
can piggyback off the features flag that already invokes a model
expansion)
- add a new flag for domcapabilities that will present the host-model
with deprecated features paired with the disabled policy
. . .
Another item to implement is a way to get the full list of deprecated
features independent of the CPU model. I can start this discussion on
the QEMU devel list while working on the items above. Here is the plan:
- discuss over on the qemu-devel list what would be the best way to
reliably report all deprecated features, independent of the CPU model
- implement QEMU changes and implement updates to the appropriate ABI
in libvirt
Thank you for your time and feedback on this!
--
Regards,
Collin
12:53 p.m.
New subject: [RFC PATCH 0/1] support deprecated-props from
query-cpu-model-expansion
On Tue, May 07, 2024 at 06:24:20PM -0400, Collin Walling wrote:
Notes
=====
- In my example below, I am running on a z14.2 machine.
- The features that are flagged as deprecated for this model are: bpb, csske, cte, te.
- The discussion regarding the QEMU changes can be found here:
https://mail.gnu.org/archive/html/qemu-devel/2024-04/msg04605.html
Example of Desired Changes
==========================
Here is what I'd like the resulting guest's transient XML to look like for the
<cpu> section (I have trimmed the features list for brevity):
...
<cpu mode='custom' match='exact' check='partial'>
<model fallback='forbid'>z14.2-base</model>
<feature policy='require' name='aen'/>
<feature policy='require' name='cmmnt'/>
<feature policy='require' name='aefsi'/>
...
<feature policy='disable' name='cte'/> ***
<feature policy='require' name='ais'/>
<feature policy='disable' name='bpb'/> ***
<feature policy='require' name='ctop'/>
<feature policy='require' name='gs'/>
<feature policy='require' name='ppa15'/>
<feature policy='require' name='zpci'/>
<feature policy='require' name='sea_esop2'/>
<feature policy='disable' name='te'/> ***
<feature policy='require' name='cmm'/>
<feature policy='disable' name='csske'/> ***
</cpu>
...
New Host CPU Model
------------------
Create a new CPU model that is a mirror of the host CPU model with
deprecated features turned off. Let's call this model "host-recommended".
A user may define this model in the guest XML as they would any other
CPU model:
<cpu mode='host-recommended' check='partial'/>
Just as how host-model works, anything defined nested in the <cpu> tag will be
ignored.
This model could potentially be listed in the domcapabilities output after the
host-model:
<cpu>
<mode name='host-passthrough' supported='yes'>
...
</mode>
...
<mode name='host-model' supported='yes'>
...
</mode>
<mode name='host-recommended' supported='yes'>
...
<feature policy='disable' name='cte'/>
<feature policy='require' name='ais'/>
<feature policy='disable' name='bpb'/>
<feature policy='require' name='ctop'/>
<feature policy='require' name='gs'/>
<feature policy='require' name='ppa15'/>
<feature policy='require' name='zpci'/>
<feature policy='require' name='sea_esop2'/>
<feature policy='disable' name='te'/>
<feature policy='require' name='cmm'/>
<feature policy='disable' name='csske'/>
</cpu>
Downside of a new "host-recommended" model is that we now have the
task of wiring it up into various mgmt applications.
New Nested Element Under <cpu>
------------------------------
Create a new optional XML element nested under the <cpu> tag that may be used to
disable deprecated features. This approach is more explicit compared to creating a new
CPU model, and allows the user to disable these features when defining a specific model
other than host-model. Here is an example of what the guest's defined XML for the CPU
could look like:
<cpu mode='host-model' check='partial'>
<deprecated_features>off</deprecated_features>
</cpu>
However, a conflict arises with this approach: parameter priority. It would need to be
discussed what the expected behavior should be if a user defines a guest with both a mode
to disable deprecated features and any deprecated features listed with the
'require' policy, e.g.:
<cpu mode='custom' match='exact' check='partial'>
<model fallback='allow'>z13.2-base</model>
<!-- which one takes priority? -->
<deprecated_features>off</deprecated_features>
<feature policy='require' name='csske'/>
</cpu>
IMHO 'deprecate_features' controls the initial expansion
of the 'host-model', and then '<feature>' fine tunes it.
IOW, your example here disables all the deprecated features
by default, and the user has then turned back on the csske
deprecated feature.
I wouldn't call this a conflict - its just a documentation
task to clarify the prioritization.
Another conflict is setting this option to "on" would have
no effect on the CPU model (I can't think of a reason why
someone would want to explicitly enable these features).
This may not communicate well to the user.
Well it depends on what we declare the "default" behaviour
to be.
We only guarantee the host-model -> custom expansion within
the scope of a running guest.
IOW, if you boot a guest, shut it down, boot it again, we
do NOT guarantee that you'll get the same expansion both
times.
With this in mind, we could take the view that disabling
deprecated features should be the default behaviour, and
thus an option to turn "on" deprecated features becomes
relevant.
In theory this could even be a host level tunable in
qemu.conf of whether the host admin wants deprecated
features skipped by default or not.
Either way, the existance of the <deprecation_fetures>
field on the *live* XML, will give a clear statement of
what behaviour libvirt applied.
To report these features, a <deprecatedProperties> tag could be added to the
domcapabilities output using the same format I use in my proposed patch for the qemu
capabilities file:
<cpu>
<mode name='host-passthrough' supported='yes'>
...
</mode>
...
<mode name='host-model' supported='yes'>
...
</mode>
<deprecatedProperties>
<property name='bpb'/>
<property name='te'/>
<property name='cte'/>
<property name='csske'/>
</deprecatedProperties>
</cpu>
Yes, I think domcapabilities should be reporting on deprecated
features, but s/Properties/Features/ and s/property/feature/
Please let me know your thoughts. Once an approach is agreed upon, I will begin
development.
Collin Walling (1):
qemu: monitor: parse deprecated-props from query-cpu-model-expansion
response
src/qemu/qemu_capabilities.c | 30 ++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++-----
3 files changed, 56 insertions(+), 5 deletions(-)
--
2.43.0
_______________________________________________
Devel mailing list -- devel(a)lists.libvirt.org
To unsubscribe send an email to devel-leave(a)lists.libvirt.org
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:42 a.m.
New subject: [RFC PATCH 0/1] support deprecated-props from
query-cpu-model-expansion
On 6/3/24 1:53 PM, Daniel P. Berrangé wrote:
On Tue, May 07, 2024 at 06:24:20PM -0400, Collin Walling wrote:
> Notes
> =====
>
> - In my example below, I am running on a z14.2 machine.
>
> - The features that are flagged as deprecated for this model are: bpb, csske, cte,
te.
>
> - The discussion regarding the QEMU changes can be found here:
https://mail.gnu.org/archive/html/qemu-devel/2024-04/msg04605.html
>
>
> Example of Desired Changes
> ==========================
>
> Here is what I'd like the resulting guest's transient XML to look like for
the <cpu> section (I have trimmed the features list for brevity):
>
> ...
> <cpu mode='custom' match='exact' check='partial'>
> <model fallback='forbid'>z14.2-base</model>
> <feature policy='require' name='aen'/>
> <feature policy='require' name='cmmnt'/>
> <feature policy='require' name='aefsi'/>
> ...
> <feature policy='disable' name='cte'/> ***
> <feature policy='require' name='ais'/>
> <feature policy='disable' name='bpb'/> ***
> <feature policy='require' name='ctop'/>
> <feature policy='require' name='gs'/>
> <feature policy='require' name='ppa15'/>
> <feature policy='require' name='zpci'/>
> <feature policy='require' name='sea_esop2'/>
> <feature policy='disable' name='te'/> ***
> <feature policy='require' name='cmm'/>
> <feature policy='disable' name='csske'/> ***
> </cpu>
> ...
> New Host CPU Model
> ------------------
>
> Create a new CPU model that is a mirror of the host CPU model with
> deprecated features turned off. Let's call this model
"host-recommended".
> A user may define this model in the guest XML as they would any other
> CPU model:
>
> <cpu mode='host-recommended' check='partial'/>
>
> Just as how host-model works, anything defined nested in the <cpu> tag will be
ignored.
>
> This model could potentially be listed in the domcapabilities output after the
host-model:
>
> <cpu>
> <mode name='host-passthrough' supported='yes'>
> ...
> </mode>
> ...
> <mode name='host-model' supported='yes'>
> ...
> </mode>
> <mode name='host-recommended' supported='yes'>
> ...
> <feature policy='disable' name='cte'/>
> <feature policy='require' name='ais'/>
> <feature policy='disable' name='bpb'/>
> <feature policy='require' name='ctop'/>
> <feature policy='require' name='gs'/>
> <feature policy='require' name='ppa15'/>
> <feature policy='require' name='zpci'/>
> <feature policy='require' name='sea_esop2'/>
> <feature policy='disable' name='te'/>
> <feature policy='require' name='cmm'/>
> <feature policy='disable' name='csske'/>
> </cpu>
Downside of a new "host-recommended" model is that we now have the
task of wiring it up into various mgmt applications.
Fair. I believe Jiri as also against this new model as well. Let's nix
adding a new model and discuss other approaches.
> New Nested Element Under <cpu>
> ------------------------------
>
> Create a new optional XML element nested under the <cpu> tag that may be used
to disable deprecated features. This approach is more explicit compared to creating a new
CPU model, and allows the user to disable these features when defining a specific model
other than host-model. Here is an example of what the guest's defined XML for the CPU
could look like:
>
> <cpu mode='host-model' check='partial'>
> <deprecated_features>off</deprecated_features>
> </cpu>
>
> However, a conflict arises with this approach: parameter priority. It would need to
be discussed what the expected behavior should be if a user defines a guest with both a
mode to disable deprecated features and any deprecated features listed with the
'require' policy, e.g.:
>
> <cpu mode='custom' match='exact' check='partial'>
> <model fallback='allow'>z13.2-base</model>
> <!-- which one takes priority? -->
> <deprecated_features>off</deprecated_features>
> <feature policy='require' name='csske'/>
> </cpu>
IMHO 'deprecate_features' controls the initial expansion
of the 'host-model', and then '<feature>' fine tunes it.
IOW, your example here disables all the deprecated features
by default, and the user has then turned back on the csske
deprecated feature.
I wouldn't call this a conflict - its just a documentation
task to clarify the prioritization.
Fair enough. As long as it the behavior is consistent and documented,
then all should be well with the world.
> Another conflict is setting this option to "on" would
have
> no effect on the CPU model (I can't think of a reason why
> someone would want to explicitly enable these features).
> This may not communicate well to the user.
Well it depends on what we declare the "default" behaviour
to be.
We only guarantee the host-model -> custom expansion within
the scope of a running guest.
IOW, if you boot a guest, shut it down, boot it again, we
do NOT guarantee that you'll get the same expansion both
times.
With this in mind, we could take the view that disabling
deprecated features should be the default behaviour, and
thus an option to turn "on" deprecated features becomes
relevant.
In theory this could even be a host level tunable in
qemu.conf of whether the host admin wants deprecated
features skipped by default or not.
Either way, the existance of the <deprecation_fetures>
field on the *live* XML, will give a clear statement of
what behaviour libvirt applied.
Okay, so what I'm taking away from this is that there should be *some*
sort of indication on the live guest XML that makes it clear that the
feature list includes deprecated features turned on|off.
>
> To report these features, a <deprecatedProperties> tag could be added to the
domcapabilities output using the same format I use in my proposed patch for the qemu
capabilities file:
>
> <cpu>
> <mode name='host-passthrough' supported='yes'>
> ...
> </mode>
> ...
> <mode name='host-model' supported='yes'>
> ...
> </mode>
> <deprecatedProperties>
> <property name='bpb'/>
> <property name='te'/>
> <property name='cte'/>
> <property name='csske'/>
> </deprecatedProperties>
> </cpu>
Yes, I think domcapabilities should be reporting on deprecated
features, but s/Properties/Features/ and s/property/feature/
In summary, and I'll attempt to incorporate Jiri's feedback here as well:
- s/properties/features
- ensure a clear indication that the live guest XML includes deprecated
features on|off (ideas: via a nested element; an additional cpu
attribute -- Jiri seems in favor of the latter)
- domcapabilities should report deprecated features (but how and where
they should be placed needs to be discussed to make it clear which model
those features were reported on)
- how to handle baseline needs more discussion (mentioned in Jiri's
feedback)
>
> Please let me know your thoughts. Once an approach is agreed upon, I will begin
development.
>
> Collin Walling (1):
> qemu: monitor: parse deprecated-props from query-cpu-model-expansion
> response
>
> src/qemu/qemu_capabilities.c | 30 ++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor.h | 2 ++
> src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++-----
> 3 files changed, 56 insertions(+), 5 deletions(-)
>
> --
> 2.43.0
> _______________________________________________
> Devel mailing list -- devel(a)lists.libvirt.org
> To unsubscribe send an email to devel-leave(a)lists.libvirt.org
With regards,
Daniel
Thanks for your response!
--
Regards,
Collin
185
days inactive
228
days old
8 comments
3 participants
participants (3)
-
Collin Walling -
Daniel P. Berrangé -
Jiri Denemark