[RFC PATCH 0/1] support deprecated-props from query-cpu-model-expansion
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
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@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
Polite ping. Would like some feedback / discussion on this before I move forward. Thanks! :)
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
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.
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
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. 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
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.
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
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. 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
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.
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
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. 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
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@lists.libvirt.org To unsubscribe send an email to devel-leave@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 :|
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@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
With regards, Daniel
Thanks for your response! -- Regards, Collin
participants (3)
-
Collin Walling -
Daniel P. Berrangé -
Jiri Denemark