On 10/20/26 00:15, Rorie Reyes wrote:
> Add a new helper qemuProcessCheckS390CPU() in qemu_process.c that
> wraps the QMP query-cpu-model-comparison probe for s390x and returns
> a virCPUCompareResult directly.  The helper filters out the special
> "type" responsible-property (which signals a CPU generation mismatch
> rather than a missing feature) before reporting incompatibility to
> the caller.
> 
> When reporting incompatibility, the differing properties are listed
> in the error message when available.  If no responsible-properties
> are returned (or all are filtered out), a generic incompatibility
> message is reported instead.
> 
> Wire the helper into qemuProcessUpdateGuestCPU() so that s390x CPU
> model compatibility is checked for real domain start paths (migration,
> snapshot restore) when the QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON
> capability is present.  The host CPU is passed as the first argument
> and the guest CPU as the second, matching
> qemuConnectCPUModelComparison().  The check is skipped in pretend mode
> (VIR_QEMU_PROCESS_START_PRETEND) since that path is used for
> command-line generation and does not involve a running QEMU instance.
> It is also skipped when the guest CPU definition has no model yet (for
> example host-model on a fresh start), since that model is resolved from
> the host CPU later and cannot be incompatible with it.
> 
> Simplify qemuConnectCPUModelComparison() in qemu_driver.c to delegate
> directly to qemuProcessCheckS390CPU().
> 
> Signed-off-by: Rorie Reyes <[email protected]>
> ---
>  src/qemu/qemu_driver.c  | 23 ++-----------
>  src/qemu/qemu_process.c | 75 +++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_process.h |  9 +++++
>  3 files changed, 86 insertions(+), 21 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 199eedca36..ef549a2c02 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12127,27 +12127,8 @@ qemuConnectCPUModelComparison(virQEMUCaps *qemuCaps,
>                                virCPUDef *cpu_b,
>                                bool failIncompatible)
>  {
> -    g_autoptr(qemuProcessQMP) proc = NULL;
> -    virCPUCompareResult result;
> -
> -    if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps),
> -                                   libDir, runUid, runGid, false)))
> -        return VIR_CPU_COMPARE_ERROR;
> -
> -    if (qemuProcessQMPStart(proc) < 0)
> -        return VIR_CPU_COMPARE_ERROR;
> -
> -    result = qemuMonitorGetCPUModelComparison(proc->mon, cpu_a, cpu_b, NULL);
> -
> -    if (result == VIR_CPU_COMPARE_ERROR)
> -        return VIR_CPU_COMPARE_ERROR;
> -
> -    if (failIncompatible && result == VIR_CPU_COMPARE_INCOMPATIBLE) {
> -        virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL);
> -        return VIR_CPU_COMPARE_ERROR;
> -    }
> -
> -    return result;
> +    return qemuProcessCheckS390CPU(qemuCaps, libDir, runUid, runGid,
> +                                   cpu_a, cpu_b, failIncompatible);
>  }

The generic-arch-named function "qemuConnectCPUModelComparison" is now a
wrapper for an specific-arch-named function "qemuProcessCheckS390CPU".
Maybe get rid of qemuConnectCPUModelComparison altogether?  It was only
used in an s390 case anyway.

>  
>  
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 3f1960f314..dfb9e0785a 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6708,6 +6708,20 @@ qemuProcessUpdateGuestCPU(virDomainDef *def,
>                                 VIR_QEMU_CAPS_HOST_CPU_FULL, true) < 0)
>              return -1;
>  
> +        if (!(flags & VIR_QEMU_PROCESS_START_PRETEND) &&
> +            def->cpu->model &&
> +            ARCH_IS_S390(hostarch) &&
> +            virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON)) {
> +            virCPUDef *hvCPU = virQEMUCapsGetHostModel(qemuCaps, 
> def->virtType,
> +                                                       
> VIR_QEMU_CAPS_HOST_CPU_REPORTED);
> +
> +            if (hvCPU && hvCPU->fallback == VIR_CPU_FALLBACK_FORBID &&
> +                qemuProcessCheckS390CPU(qemuCaps, cfg->libDir,
> +                                        cfg->user, cfg->group,
> +                                        hvCPU, def->cpu, true) == 
> VIR_CPU_COMPARE_ERROR)
> +                return -1;
> +        }
> +
>          /* When starting a fresh domain we disable all features removed from
>           * the specified CPU model to make sure they are only used if
>           * explicitly requested. But when we are restoring a previously 
> running
> @@ -10589,6 +10603,67 @@ qemuProcessQMPStart(qemuProcessQMP *proc)
>  }
>  
>  
> +virCPUCompareResult
> +qemuProcessCheckS390CPU(virQEMUCaps *qemuCaps,
> +                        const char *libDir,
> +                        uid_t runUid,
> +                        gid_t runGid,
> +                        virCPUDef *cpu_a,
> +                        virCPUDef *cpu_b,
> +                        bool failIncompatible)
> +{
> +    g_autoptr(qemuProcessQMP) proc = NULL;
> +    g_auto(GStrv) responsibleProps = NULL;
> +    virCPUCompareResult result;
> +
> +    if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps),
> +                                   libDir, runUid, runGid, false)))
> +        return VIR_CPU_COMPARE_ERROR;
> +
> +    if (qemuProcessQMPStart(proc) < 0)
> +        return VIR_CPU_COMPARE_ERROR;
> +
> +    result = qemuMonitorGetCPUModelComparison(proc->mon, cpu_a, cpu_b,
> +                                              &responsibleProps);
> +
> +    if (result == VIR_CPU_COMPARE_ERROR)
> +        return VIR_CPU_COMPARE_ERROR;
> +
> +    if (result == VIR_CPU_COMPARE_IDENTICAL || result == 
> VIR_CPU_COMPARE_SUPERSET)
> +        return result;
> +
> +    /* result is VIR_CPU_COMPARE_INCOMPATIBLE — filter out the special "type"
> +     * property which signals a CPU generation mismatch rather than a missing
> +     * feature and should not be reported as such to the user. */
> +    if (responsibleProps) {
> +        size_t i, j;
> +        for (i = 0, j = 0; responsibleProps[i]; i++) {
> +            if (STRNEQ(responsibleProps[i], "type"))
> +                responsibleProps[j++] = responsibleProps[i];
> +            else
> +                g_free(responsibleProps[i]);
> +        }
> +        responsibleProps[j] = NULL;
> +    }

As Jiri mentioned in v1, the filtering-out of the property "type" should
be moved to qemuMonitorJSONGetCPUModelComparison.  The reasoning is that
the code does not react to the presence of this property, so it makes
sense to omit it early.

Then, the checks on result can go away and the rest of this function
shortens to:

if (failIncompatible && result == VIR_CPU_COMPARE_INCOMPATIBLE) {
    ... report error ...
    return VIR_CPU_COMPARE_ERROR;
}

return result;

> +
> +    if (failIncompatible) {
> +        if (responsibleProps && responsibleProps[0]) {
> +            g_autofree char *props = g_strjoinv(", ", responsibleProps);
> +
> +            virReportError(VIR_ERR_CPU_INCOMPATIBLE,
> +                           _("guest CPU model is not compatible with the 
> destination host CPU model, differing properties: %1$s"),
> +                           props);
> +        } else {
> +            virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
> +                           _("guest CPU model is not compatible with the 
> destination host CPU model"));
> +        }

The text "destination host" is no longer valid since this is used for
non-migration cases.

> +        return VIR_CPU_COMPARE_ERROR;
> +    }
> +
> +    return VIR_CPU_COMPARE_INCOMPATIBLE;
> +}
> +
> +
>  void
>  qemuProcessHandleNbdkitExit(qemuNbdkitProcess *nbdkit,
>                              virDomainObj *vm)
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index 5d8f1f89d5..d9dd5e185e 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -258,6 +258,15 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuProcessQMP, 
> qemuProcessQMPFree);
>  
>  int qemuProcessQMPStart(qemuProcessQMP *proc);
>  
> +virCPUCompareResult
> +qemuProcessCheckS390CPU(virQEMUCaps *qemuCaps,
> +                        const char *libDir,
> +                        uid_t runUid,
> +                        gid_t runGid,
> +                        virCPUDef *cpu_a,
> +                        virCPUDef *cpu_b,
> +                        bool failIncompatible);
> +
>  void qemuProcessCleanupMigrationJob(virQEMUDriver *driver,
>                                      virDomainObj *vm);
>  


-- 
Regards,
  Collin

Reply via email to