1. Patchew
  2. Libvirt
  3. [PATCH v2 0/5] Allow Guest CPU Model with Deprecated Features Disabled
  4. View patch

[PATCH v2 5/5] conf: add deprecated_features attribute

Collin Walling posted 5 patches 1 month ago
There is a newer version of this series
[PATCH v2 5/5] conf: add deprecated_features attribute
Posted by Collin Walling 1 month ago 
Add a new a attribute, deprecated_features='on|off' to the <cpu>
element.  This is used to toggle features flagged as deprecated on the
CPU model on or off.  When this attribute is paired with 'on',
deprecated features will not be filtered.  When paired with 'off', any
CPU features that are flagged as deprecated will be listed under the
CPU model with the 'disable' policy.

Example:

  <cpu mode='host-model' check='partial' deprecated_features='off'/>

The absence of this attribute is equivalent to the 'on' option.

The deprecated features that will populate the domain XML are the same
features that result in the virsh domcapabilities command with the
--disable-deprecated-features argument present.

It is recommended to define a domain XML with this attribute set to
'off' to ensure migration to machines that may outright drop these
features in the future.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 src/conf/cpu_conf.c           | 10 ++++++++++
 src/conf/cpu_conf.h           |  1 +
 src/conf/schemas/cputypes.rng | 12 ++++++++++++
 src/qemu/qemu_process.c       |  4 ++++
 4 files changed, 27 insertions(+)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index dcc164d165..a9910e2195 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -238,6 +238,7 @@ virCPUDefCopyWithoutModel(const virCPUDef *cpu)
     copy->mode = cpu->mode;
     copy->match = cpu->match;
     copy->check = cpu->check;
+    copy->deprecated_feats = cpu->deprecated_feats;
     copy->fallback = cpu->fallback;
     copy->sockets = cpu->sockets;
     copy->dies = cpu->dies;
@@ -450,6 +451,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
         if (virXMLPropEnum(ctxt->node, "check", virCPUCheckTypeFromString,
                            VIR_XML_PROP_NONE, &def->check) < 0)
             return -1;
+
+        if (virXMLPropTristateSwitch(ctxt->node, "deprecated_features",
+                                     VIR_XML_PROP_NONE,
+                                     &def->deprecated_feats) < 0)
+            return -1;
     }
 
     if (def->type == VIR_CPU_TYPE_HOST) {
@@ -748,6 +754,10 @@ virCPUDefFormatBufFull(virBuffer *buf,
             virBufferAsprintf(&attributeBuf, " migratable='%s'",
                               virTristateSwitchTypeToString(def->migratable));
         }
+
+        if (def->deprecated_feats == VIR_TRISTATE_SWITCH_OFF) {
+            virBufferAddLit(&attributeBuf, " deprecated_features='off'");
+        }
     }
 
     /* Format children */
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index f71d942ce6..28e26303ef 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -161,6 +161,7 @@ struct _virCPUDef {
     virCPUMaxPhysAddrDef *addr;
     virHostCPUTscInfo *tsc;
     virTristateSwitch migratable; /* for host-passthrough mode */
+    virTristateSwitch deprecated_feats;
 };
 
 virCPUDef *virCPUDefNew(void);
diff --git a/src/conf/schemas/cputypes.rng b/src/conf/schemas/cputypes.rng
index 3a8910e09f..b94b08eb5a 100644
--- a/src/conf/schemas/cputypes.rng
+++ b/src/conf/schemas/cputypes.rng
@@ -34,6 +34,15 @@
     </attribute>
   </define>
 
+  <define name="cpuDeprecatedFeatures">
+    <attribute name="deprecated_features">
+      <choice>
+        <value>on</value>
+        <value>off</value>
+      </choice>
+    </attribute>
+  </define>
+
   <define name="cpuModel">
     <element name="model">
       <optional>
@@ -439,6 +448,9 @@
       <optional>
         <ref name="cpuCheck"/>
       </optional>
+      <optional>
+        <ref name="cpuDeprecatedFeatures"/>
+      </optional>
       <optional>
         <attribute name="migratable">
           <ref name="virOnOff"/>
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bee7a39e4e..c06ca1a833 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6399,6 +6399,10 @@ qemuProcessUpdateGuestCPU(virDomainDef *def,
                                 &def->os.arch) < 0)
         return -1;
 
+    if (def->cpu->deprecated_feats == VIR_TRISTATE_SWITCH_OFF) {
+        virQEMUCapsUpdateCPUDeprecatedFeatures(qemuCaps, def->virtType, def->cpu);
+    }
+
     return 0;
 }
 
-- 
2.45.1
Re: [PATCH v2 5/5] conf: add deprecated_features attribute
Posted by Jiri Denemark 4 weeks, 1 day ago 
On Mon, Nov 25, 2024 at 14:46:39 -0500, Collin Walling wrote:
> Add a new a attribute, deprecated_features='on|off' to the <cpu>
> element.  This is used to toggle features flagged as deprecated on the
> CPU model on or off.  When this attribute is paired with 'on',
> deprecated features will not be filtered.  When paired with 'off', any
> CPU features that are flagged as deprecated will be listed under the
> CPU model with the 'disable' policy.
> 
> Example:
> 
>   <cpu mode='host-model' check='partial' deprecated_features='off'/>
> 
> The absence of this attribute is equivalent to the 'on' option.
> 
> The deprecated features that will populate the domain XML are the same
> features that result in the virsh domcapabilities command with the
> --disable-deprecated-features argument present.
> 
> It is recommended to define a domain XML with this attribute set to
> 'off' to ensure migration to machines that may outright drop these
> features in the future.

In addition to the comments from Boris...

> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index dcc164d165..a9910e2195 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c
> @@ -238,6 +238,7 @@ virCPUDefCopyWithoutModel(const virCPUDef *cpu)
>      copy->mode = cpu->mode;
>      copy->match = cpu->match;
>      copy->check = cpu->check;
> +    copy->deprecated_feats = cpu->deprecated_feats;
>      copy->fallback = cpu->fallback;
>      copy->sockets = cpu->sockets;
>      copy->dies = cpu->dies;
> @@ -450,6 +451,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
>          if (virXMLPropEnum(ctxt->node, "check", virCPUCheckTypeFromString,
>                             VIR_XML_PROP_NONE, &def->check) < 0)
>              return -1;
> +
> +        if (virXMLPropTristateSwitch(ctxt->node, "deprecated_features",
> +                                     VIR_XML_PROP_NONE,
> +                                     &def->deprecated_feats) < 0)
> +            return -1;
>      }
>  
>      if (def->type == VIR_CPU_TYPE_HOST) {
> @@ -748,6 +754,10 @@ virCPUDefFormatBufFull(virBuffer *buf,
>              virBufferAsprintf(&attributeBuf, " migratable='%s'",
>                                virTristateSwitchTypeToString(def->migratable));
>          }
> +
> +        if (def->deprecated_feats == VIR_TRISTATE_SWITCH_OFF) {

the attribute should be formatted even if it is on and ignored only when
it's absent.

> +            virBufferAddLit(&attributeBuf, " deprecated_features='off'");
> +        }
>      }
>  
>      /* Format children */

Jirka
Re: [PATCH v2 5/5] conf: add deprecated_features attribute
Posted by Boris Fiuczynski 4 weeks, 1 day ago 
On 11/25/24 20:46, Collin Walling wrote:
> Add a new a attribute, deprecated_features='on|off' to the <cpu>
> element.  This is used to toggle features flagged as deprecated on the
> CPU model on or off.  When this attribute is paired with 'on',
> deprecated features will not be filtered.  When paired with 'off', any
> CPU features that are flagged as deprecated will be listed under the
> CPU model with the 'disable' policy.
> 
> Example:
> 
>    <cpu mode='host-model' check='partial' deprecated_features='off'/>
> 
> The absence of this attribute is equivalent to the 'on' option.
> 
> The deprecated features that will populate the domain XML are the same
> features that result in the virsh domcapabilities command with the
> --disable-deprecated-features argument present.
> 
> It is recommended to define a domain XML with this attribute set to
> 'off' to ensure migration to machines that may outright drop these
> features in the future.

I think there is a validation missing that QEMU provides the capability 
QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS when setting the 
attribute to 'off'.

Please also add tests running with and without a QEMU supporting this 
capability.

> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>   src/conf/cpu_conf.c           | 10 ++++++++++
>   src/conf/cpu_conf.h           |  1 +
>   src/conf/schemas/cputypes.rng | 12 ++++++++++++
>   src/qemu/qemu_process.c       |  4 ++++
>   4 files changed, 27 insertions(+)
> 
> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index dcc164d165..a9910e2195 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c
> @@ -238,6 +238,7 @@ virCPUDefCopyWithoutModel(const virCPUDef *cpu)
>       copy->mode = cpu->mode;
>       copy->match = cpu->match;
>       copy->check = cpu->check;
> +    copy->deprecated_feats = cpu->deprecated_feats;
>       copy->fallback = cpu->fallback;
>       copy->sockets = cpu->sockets;
>       copy->dies = cpu->dies;
> @@ -450,6 +451,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
>           if (virXMLPropEnum(ctxt->node, "check", virCPUCheckTypeFromString,
>                              VIR_XML_PROP_NONE, &def->check) < 0)
>               return -1;
> +
> +        if (virXMLPropTristateSwitch(ctxt->node, "deprecated_features",
> +                                     VIR_XML_PROP_NONE,
> +                                     &def->deprecated_feats) < 0)
> +            return -1;
>       }
>   
>       if (def->type == VIR_CPU_TYPE_HOST) {
> @@ -748,6 +754,10 @@ virCPUDefFormatBufFull(virBuffer *buf,
>               virBufferAsprintf(&attributeBuf, " migratable='%s'",
>                                 virTristateSwitchTypeToString(def->migratable));
>           }
> +
> +        if (def->deprecated_feats == VIR_TRISTATE_SWITCH_OFF) {
> +            virBufferAddLit(&attributeBuf, " deprecated_features='off'");
> +        }
>       }
>   
>       /* Format children */
> diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
> index f71d942ce6..28e26303ef 100644
> --- a/src/conf/cpu_conf.h
> +++ b/src/conf/cpu_conf.h
> @@ -161,6 +161,7 @@ struct _virCPUDef {
>       virCPUMaxPhysAddrDef *addr;
>       virHostCPUTscInfo *tsc;
>       virTristateSwitch migratable; /* for host-passthrough mode */
> +    virTristateSwitch deprecated_feats;
>   };
>   
>   virCPUDef *virCPUDefNew(void);
> diff --git a/src/conf/schemas/cputypes.rng b/src/conf/schemas/cputypes.rng
> index 3a8910e09f..b94b08eb5a 100644
> --- a/src/conf/schemas/cputypes.rng
> +++ b/src/conf/schemas/cputypes.rng
> @@ -34,6 +34,15 @@
>       </attribute>
>     </define>
>   
> +  <define name="cpuDeprecatedFeatures">
> +    <attribute name="deprecated_features">
> +      <choice>
> +        <value>on</value>
> +        <value>off</value>
> +      </choice>
> +    </attribute>
> +  </define>
> +
>     <define name="cpuModel">
>       <element name="model">
>         <optional>
> @@ -439,6 +448,9 @@
>         <optional>
>           <ref name="cpuCheck"/>
>         </optional>
> +      <optional>
> +        <ref name="cpuDeprecatedFeatures"/>
> +      </optional>

<optional>
   <attribute name="deprecated_features">
     <ref name="virOnOff"/>
   </attribute>
</optional>

This to me looks simpler and the above hunk adding cpuDeprecatedFeatures 
can be deleted.

>         <optional>
>           <attribute name="migratable">
>             <ref name="virOnOff"/>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index bee7a39e4e..c06ca1a833 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6399,6 +6399,10 @@ qemuProcessUpdateGuestCPU(virDomainDef *def,
>                                   &def->os.arch) < 0)
>           return -1;
>   
> +    if (def->cpu->deprecated_feats == VIR_TRISTATE_SWITCH_OFF) {
> +        virQEMUCapsUpdateCPUDeprecatedFeatures(qemuCaps, def->virtType, def->cpu);
> +    }
> +
>       return 0;
>   }
>   


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.