Hi, The commit message suggests this patch is doing several things at once... On Thu, Oct 01, 2026 at 22:36:23 -0400, Rorie Reyes wrote:

The qemu-cpu-model-comparison QMP command can return a "responsible-properties" array identifying which CPU properties caused an incompatibility result. Expose this through the JSON monitor layer by adding an optional `responsibleProps` output parameter (char ***) to qemuMonitorJSONGetCPUModelComparison().

When the caller passes a non-NULL pointer the array is populated from the QMP response; passing NULL preserves the existing behaviour so all current callers are unaffected until they opt in.

This is the main thing.

The existing caller in qemu_monitor.c is updated to pass NULL so the build continues to work.

You don't really need to explicitly say this, it's the minimal change you have to do to make the code compile.

A GenericError response from the QMP command is intercepted and re-reported as VIR_ERR_CPU_INCOMPATIBLE with the QEMU error description. Assuming the JSON query was properly constructed, all GenericErrors should indicate an incompatible CPU model due to QEMU binary limitations (e.g. old binary does not recognize newer features or models).

This sounds completely unrelated and should go to a separate patch. Se below for more comments on this part.

The existing unit test is updated to pass the new argument and extended with additional calls: one that verifies the responsible-properties array is correctly populated from the QMP response, one that verifies it is NULL when the key is absent from the response, one that verifies it is NULL when the array is empty, and cases for "identical", "subset", and "superset" results (including a "type" responsible-property) to cover all result-string mappings.

I would only do the minimal change here and add new tests in a separate patch.

Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com> --- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor_json.c | 26 +++++- src/qemu/qemu_monitor_json.h | 5 +- tests/qemumonitorjsontest.c | 159 +++++++++++++++++++++++++++++++++-- 4 files changed, 182 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2c86cd1971..e3edc88814 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3335,7 +3335,7 @@ qemuMonitorGetCPUModelComparison(qemuMonitor *mon,

QEMU_CHECK_MONITOR(mon);

- return qemuMonitorJSONGetCPUModelComparison(mon, cpu_a, cpu_b, result); + return qemuMonitorJSONGetCPUModelComparison(mon, cpu_a, cpu_b, result, NULL);

qemuMonitorGetCPUModelComparison is just a tiny wrapper around qemuMonitorJSONGetCPUModelComparison, you should propagate the new parameter one layer up right away. That is, squash the second patch into this one.

}

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8e88dc9442..a895341b67 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5689,7 +5689,8 @@ int qemuMonitorJSONGetCPUModelComparison(qemuMonitor *mon, virCPUDef *cpu_a, virCPUDef *cpu_b, - char **result) + char **result, + char ***responsibleProps) { g_autoptr(virJSONValue) model_a = NULL; g_autoptr(virJSONValue) model_b = NULL; @@ -5697,6 +5698,11 @@ qemuMonitorJSONGetCPUModelComparison(qemuMonitor *mon, g_autoptr(virJSONValue) reply = NULL; const char *data_result; virJSONValue *data; + virJSONValue *propsArray; + + *result = NULL; + if (responsibleProps) + *responsibleProps = NULL;

if (!(model_a = qemuMonitorJSONMakeCPUModel(cpu_a, true, false)) || !(model_b = qemuMonitorJSONMakeCPUModel(cpu_b, true, false))) @@ -5711,6 +5717,13 @@ qemuMonitorJSONGetCPUModelComparison(qemuMonitor *mon, if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) return -1;

+ if (qemuMonitorJSONHasError(reply, "GenericError")) { + virJSONValue *error = virJSONValueObjectGet(reply, "error"); + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", + qemuMonitorJSONStringifyError(error)); + return -1; + }

If this really means the CPUs are incompatible rather then an error, you should propagate the info to the caller rather then reporting it as VIR_ERR_CPU_INCOMPATIBLE error here. The caller decides whether CPU incompatibility should be reported as an error or just as a specific return code. But we normally report unknown CPU models and features as errors so I don't think we should handle that differently in s390. That said, leaving just qemuMonitorJSONCheckError should be enough. And if not, the changes should definitely go into a separate patch.

+ if (qemuMonitorJSONCheckError(cmd, reply) < 0) return -1;

@@ -5723,6 +5736,17 @@ qemuMonitorJSONGetCPUModelComparison(qemuMonitor *mon, }

*result = g_strdup(data_result);

If you want to gain some bonus points, you can add a new patch before this one that would refactor qemuMonitor*GetCPUModelComparison so that it doesn't reports the result as a string :-) In other words, move if (STREQ(result, "identical")) return VIR_CPU_COMPARE_IDENTICAL; if (STREQ(result, "superset")) return VIR_CPU_COMPARE_SUPERSET; from qemu_driver.c to the monitor code and get rid of the char **result parameter completely.

+ + propsArray = virJSONValueObjectGetArray(data, "responsible-properties"); + + if (responsibleProps && propsArray && + virJSONValueArraySize(propsArray) > 0 && + !(*responsibleProps = virJSONValueArrayToStringList(propsArray))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid 'responsible-properties' list in query-cpu-model-comparison response"));

This would overwrite the error already reported by virJSONValueArrayToStringList. Just return -1 here.

+ return -1; + }

QEMU reports a special "type" property in the responsible-properties list if the incompatibility reason is not only a set of features that might be allowed to be disabled to get compatible CPUs, but if the models themselves are incompatible. Should we filter it out somehow to avoid reporting "type" as a missing feature?

+ return 0; }

diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 63eded7ecb..9a3695d8e1 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -443,8 +443,9 @@ int qemuMonitorJSONGetCPUModelComparison(qemuMonitor *mon, virCPUDef *cpu_a, virCPUDef *cpu_b, - char **result) - ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + char **result, + char ***responsibleProps) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);

GHashTable * qemuMonitorJSONGetCommandLineOptions(qemuMonitor *mon); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index a963029c69..ee7123d4c8 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c

Move the new tests into a separate patch, please.

@@ -2623,27 +2623,174 @@ testQemuMonitorJSONqemuMonitorJSONGetCPUModelComparison(const void *opaque) g_autoptr(virCPUDef) cpu_a = virCPUDefNew(); g_autoptr(virCPUDef) cpu_b = virCPUDefNew(); g_autofree char *result = NULL; + g_auto(GStrv) responsibleProps = NULL;

I would suggest defining a code block around each test ({ ... }) and move result and responsibleProps inside it so that they are automatically freed after each test.

if (!(test = qemuMonitorTestNewSchema(data->xmlopt, data->schema))) return -1;

+ cpu_a->model = g_strdup("cpu_a"); + cpu_b->model = g_strdup("cpu_b"); + + /* incompatible, NULL responsibleProps — must still work */ if (qemuMonitorTestAddItem(test, "query-cpu-model-comparison", - "{\"return\":{\"result\":\"test\"}}") < 0) + "{\"return\":{\"result\":\"incompatible\"," + "\"responsible-properties\":[\"prop_a\",\"prop_b\"]}}") < 0) return -1;

- cpu_a->model = g_strdup("cpu_a"); - cpu_b->model = g_strdup("cpu_b"); + if (qemuMonitorJSONGetCPUModelComparison(qemuMonitorTestGetMonitor(test), + cpu_a, cpu_b, &result, NULL) < 0) + return -1; + + if (!result || STRNEQ(result, "incompatible")) {

Use STRNEQ_NULLABLE throughout all this code instead (although ideally the string comparison would not be here at all if qemuMonitorJSONGetCPUModelComparison is refactored as I suggested earlier). ...

+ /* superset with "type" prop — generation-mismatch error path */ + if (qemuMonitorTestAddItem(test, "query-cpu-model-comparison", + "{\"return\":{\"result\":\"superset\"," + "\"responsible-properties\":[\"type\"]}}") < 0) + return -1;

Is this even possible? From qapi/machine.json I would expect the "type" property to be only present for incompatible CPUs. ... Jirka

On Thu, Oct 01, 2026 at 22:36:25 -0400, Rorie Reyes wrote:

Add a new internal qemuCPUModelCompatResult enum to qemu_process.h with values for IDENTICAL, SUBSET, SUPERSET, INCOMPATIBLE, and ERROR, and a new shared helper qemuProcessCPUModelComparison() in qemu_process.c that wraps the QMP probe and maps the result string to the enum.

Refactor qemuConnectCPUModelComparison() in qemu_driver.c to delegate to the new helper, eliminating the duplicated QMP probe boilerplate.

Instead of reusing internals from qemuConnectCPUModelComparison in other places, I think we should incorporate the function inside qemuDomainCheckCPU. Or rather rename qemuDomainCheckCPU to emphasise it is x86 specific, create similar function for s390 and call them both from qemuDomainCheckCPU and adapt qemuConnectCompareHypervisorCPU to use only this new function. See my reply to the last patch for details.

qemuConnectCPUModelComparison() maps the internal enum back to virCPUCompareResult, treating SUBSET as VIR_CPU_COMPARE_INCOMPATIBLE to preserve existing public API behaviour.

Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com> --- src/qemu/qemu_driver.c | 20 +++++++---------- src/qemu/qemu_process.c | 50 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.h | 21 +++++++++++++++++ 3 files changed, 79 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7332ec0256..56a254f0ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12127,25 +12127,21 @@ qemuConnectCPUModelComparison(virQEMUCaps *qemuCaps, virCPUDef *cpu_b, bool failIncompatible) { - g_autoptr(qemuProcessQMP) proc = NULL; - g_autofree char *result = NULL; - - if (!(proc = qemuProcessQMPNew(virQEMUCapsGetBinary(qemuCaps), - libDir, runUid, runGid, false))) - return VIR_CPU_COMPARE_ERROR; - - if (qemuProcessQMPStart(proc) < 0) - return VIR_CPU_COMPARE_ERROR; + qemuCPUModelCompatResult cmp = + qemuProcessCPUModelComparison(qemuCaps, libDir, runUid, runGid, + cpu_a, cpu_b, NULL);

- if (qemuMonitorGetCPUModelComparison(proc->mon, cpu_a, cpu_b, &result, NULL) < 0) + if (cmp == QEMU_CPU_COMPAT_ERROR) return VIR_CPU_COMPARE_ERROR;

- if (STREQ(result, "identical")) + if (cmp == QEMU_CPU_COMPAT_IDENTICAL) return VIR_CPU_COMPARE_IDENTICAL;

- if (STREQ(result, "superset")) + if (cmp == QEMU_CPU_COMPAT_SUPERSET) return VIR_CPU_COMPARE_SUPERSET;

Hmm, this is the kind of refactoring I suggested in my reply to the first patch. I'd just push it a few layers down directly to the monitor code.

+ /* cpu_a is the host. SUBSET means the host lacks features the guest + * needs — treat as incompatible, same as INCOMPATIBLE. */ if (failIncompatible) { virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); return VIR_CPU_COMPARE_ERROR; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 57a3ed3720..90b8acc6c2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c ... @@ -10583,6 +10591,48 @@ qemuProcessQMPStart(qemuProcessQMP *proc) }

+qemuCPUModelCompatResult +qemuProcessCPUModelComparison(virQEMUCaps *qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDef *cpu_a, + virCPUDef *cpu_b, + char ***responsibleProps) +{

...

+ switch (compatResult) { + case QEMU_CPU_COMPAT_INCOMPATIBLE: + case QEMU_CPU_COMPAT_IDENTICAL: + case QEMU_CPU_COMPAT_SUPERSET: + case QEMU_CPU_COMPAT_SUBSET: + return compatResult; + case QEMU_CPU_COMPAT_ERROR: + case QEMU_CPU_COMPAT_LAST: + default: + VIR_WARN("unknown query-cpu-model-comparison result '%s', treating as incompatible", + result); + return QEMU_CPU_COMPAT_INCOMPATIBLE; + }

No matter how you finally end up decoding the strings to the enum, unknown results from query-cpu-model-comparison should be properly reported as errors. Jirka

On Thu, Oct 01, 2026 at 22:36:26 -0400, Rorie Reyes wrote:

When migrating an s390x guest to a target s390x host, it is possible the migration will fail ungracefully due to incompatible CPU models.

Add a pre-migration check for the s390x architecture: when the guest uses a custom or host-model CPU, look up the QEMU capabilities from the driver cache and, if the QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON capability is present, invoke a CPU model comparison between the guest and target hypervisor CPU models.

The check runs before qemuProcessInit() so that the guest CPU from the migration cookie is compared before it can be resolved to the destination host model. The comparison result is distinguished by four cases:

- IDENTICAL or SUBSET: guest CPU model is presumed to be compatible with target host's model. - SUPERSET: guest CPU model is newer than target host's model, or target host does not support features guest requires. - INCOMPATIBLE: guest CPU model cannot run on target host. Report the list of features the target is missing (if any). - Probe error: propagated as VIR_ERR_CPU_INCOMPATIBLE from the monitor layer.

Signed-off-by: Rorie Reyes <rreyes@linux.ibm.com> --- src/qemu/qemu_migration.c | 114 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c

qemu_migration.c is a wrong place to handle this. We have a code that already checks CPU compatibility for any QEMU start (fresh start, migration, snapshot, restore) and there's no reason for s390 to come up with its own place to deal with this. Also this path would only address migration, while I believe we want similar behavior for snapshots and restore as well. The CPU compatibility code is in qemuProcessUpdateGuestCPU which calls to qemuDomainCheckCPU. So if we refactor qemuDomainCheckCPU in a way I described in my previous reply, we should get migration checks for free. The flags parameter in qemuProcessUpdateGuestCPU says whether it was called during a fresh start or not and can be used to tune the behavior if needed.

index 4a43ab83b0..b77bdd4370 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3308,6 +3308,90 @@ qemuMigrationDstPrepareAnyBlockDirtyBitmaps(virDomainObj *vm, }

+/** + * qemuMigrationDstCheckCPUModelCompat: + * + * Run a short-lived QMP probe to compare @guestCPU against @hvCPU on the + * destination host. Returns 0 if the guest CPU is compatible (identical + * or a subset of what the host provides), -1 if incompatible. + * + * On incompatibility an error is reported via virReportError before returning + * -1. Probe errors (e.g. the destination QEMU does not recognise a CPU + * property) are reported as VIR_ERR_CPU_INCOMPATIBLE by the monitor layer and + * are propagated as-is. + */ +static int +qemuMigrationDstCheckCPUModelCompat(virQEMUCaps *qemuCaps, + const char *libDir, + uid_t runUid, + gid_t runGid, + virCPUDef *hvCPU, + virCPUDef *guestCPU) +{ + g_autofree char **responsibleProps = NULL; + qemuCPUModelCompatResult cmp = qemuProcessCPUModelComparison(qemuCaps, libDir, + runUid, runGid, + guestCPU, hvCPU, + &responsibleProps); + + switch (cmp) { + case QEMU_CPU_COMPAT_IDENTICAL: + case QEMU_CPU_COMPAT_SUBSET: + /* Guest CPU model is presumed to be compatible with target host's model. */ + return 0; + + case QEMU_CPU_COMPAT_SUPERSET: + /* Either guest CPU model is newer than target host's model, or target + * host does not support features required by guest. */ + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", + _("guest CPU model is a newer generation than the destination host CPU model")); + return -1; + + case QEMU_CPU_COMPAT_INCOMPATIBLE: + /* Guest CPU model cannot run on target host. Report the list of + * features the target is missing (if any). + * + * s390x will report the property 'type' when CPU models are + * incompatible. Remove it from the responsibleProps list so it + * is not reported as a missing feature. */ + if (responsibleProps) { + size_t i = 0; + size_t j = 0; + + while (responsibleProps[i]) { + if (STREQ(responsibleProps[i], "type")) { + g_free(responsibleProps[i++]); + } else { + responsibleProps[j++] = responsibleProps[i++]; + } + } + responsibleProps[j] = NULL;

OK, this is the filtering I talked about in patch 1 and I think we should do it in the json monitor unless we need to see that info somewhere else, which I believe is not the case.

+ + if (responsibleProps[0]) { + g_autofree char *missing_str = g_strjoinv(", ", responsibleProps); + virReportError(VIR_ERR_CPU_INCOMPATIBLE, + _("destination host CPU model is missing features required by the guest CPU model: %1$s"), + missing_str); + return -1; + } + } ...

Jirka