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

[Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx>
> ---
>   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