[PATCH v2 2/3] qemu: process: refactor deprecated features code

Collin Walling posted 3 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 2/3] qemu: process: refactor deprecated features code
Posted by Collin Walling 3 months, 2 weeks ago
Group up the deprecated features code into a single block to keep things
clean; only check if the deprecated_features attribute is present
once and then do relevent work.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 src/qemu/qemu_process.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e4f8cb8c01..b4989f7ab8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6588,19 +6588,18 @@ qemuProcessUpdateGuestCPU(virDomainDef *def,
                                 &def->os.arch) < 0)
         return -1;
 
-    if (def->cpu->deprecated_feats &&
-        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS)) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("toggling deprecated features for CPU model is unsupported"));
-        return -1;
-    }
-
     if (def->cpu->deprecated_feats) {
 
         virCPUFeaturePolicy policy = VIR_CPU_FEATURE_REQUIRE;
         if (def->cpu->deprecated_feats == VIR_TRISTATE_SWITCH_OFF)
             policy = VIR_CPU_FEATURE_DISABLE;
 
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("toggling deprecated features for CPU model is unsupported"));
+            return -1;
+        }
+
         virQEMUCapsUpdateCPUDeprecatedFeatures(qemuCaps, def->virtType,
                                                def->cpu, policy);
     }
-- 
2.47.1
Re: [PATCH v2 2/3] qemu: process: refactor deprecated features code
Posted by Collin Walling 3 months, 2 weeks ago
On 5/22/25 1:33 AM, Collin Walling wrote:
> Group up the deprecated features code into a single block to keep things
> clean; only check if the deprecated_features attribute is present
> once and then do relevent work.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>  src/qemu/qemu_process.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index e4f8cb8c01..b4989f7ab8 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6588,19 +6588,18 @@ qemuProcessUpdateGuestCPU(virDomainDef *def,
>                                  &def->os.arch) < 0)
>          return -1;
>  
> -    if (def->cpu->deprecated_feats &&
> -        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("toggling deprecated features for CPU model is unsupported"));
> -        return -1;
> -    }
> -
>      if (def->cpu->deprecated_feats) {
>  
>          virCPUFeaturePolicy policy = VIR_CPU_FEATURE_REQUIRE;
>          if (def->cpu->deprecated_feats == VIR_TRISTATE_SWITCH_OFF)
>              policy = VIR_CPU_FEATURE_DISABLE;
>  
> +        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_DEPRECATED_PROPS)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("toggling deprecated features for CPU model is unsupported"));
> +            return -1;
> +        }
> +
>          virQEMUCapsUpdateCPUDeprecatedFeatures(qemuCaps, def->virtType,
>                                                 def->cpu, policy);
>      }


Of course it's not until after I send out the patches that I realize
setting this means it will break s390 guests running on a QEMU without
deprecated features support (due to next patch setting the flag by default).

When I first put in that caps check, it was to make setting the
deprecated_features flag reactive if no change could possibly happen
e.g. if x86 sets the attribute, user will get an error letting them know
no changes can occur.

I'll amend this with a quick fix in order to not hold things up.

-- 
Regards,
  Collin