On 8/8/24 00:00, Kamil Szczęk wrote:
Up until now, we've assumed that all x86 machines have a PS/2 controller built-in. This assumption was correct until QEMU v4.2 introduced a new x86-based machine type - microvm.
Due to this assumption, a pair of unnecessary PS/2 inputs are implicitly added to all microvm domains. This patch fixes that by whitelisting machine types which are known to include the i8042 PS/2 controller.
Signed-off-by: Kamil Szczęk <kamil@szczek.dev> ---
Side note - for sending two or more patches we use a cover letter.
src/qemu/qemu_capabilities.c | 20 ++++++++++++++++++++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_domain.c | 28 +++++++++++++++++++++++++--- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_validate.c | 3 +-- 5 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 29dfe8d35a..19a057d94d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1359,6 +1359,13 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "vhost-user-vga", QEMU_CAPS_DEVICE_VHOST_USER_VGA }, { "ramfb", QEMU_CAPS_DEVICE_RAMFB }, { "max-arm-cpu", QEMU_CAPS_ARM_MAX_CPU }, + /* + * The i8042 controller is a built-in device and is not user-creatable. + * However, since not all machine types include this controller, you should + * avoid checking for this capability directly. + * + * Prefer using virQEMUCapsSupportsI8042() instead. + */
You can leave this comment here, but we don't usually look here. I'd also add a note next to the capability definition: diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d77a4bf4d9..044d712f50 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -535,7 +535,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_SMP_DIES, /* -smp dies= */ /* 350 */ - QEMU_CAPS_DEVICE_I8042, /* PS/2 controller */ + QEMU_CAPS_DEVICE_I8042, /* PS/2 controller, use virQEMUCapsSupportsI8042() to query this capability */ QEMU_CAPS_OBJECT_RNG_BUILTIN, /* -object rng-builtin */ X_QEMU_CAPS_VIRTIO_NET_FAILOVER, /* virtio-net-*.failover */ QEMU_CAPS_DEVICE_TPM_SPAPR, /* -device tpm-spapr */
{ "i8042", QEMU_CAPS_DEVICE_I8042 }, { "rng-builtin", QEMU_CAPS_OBJECT_RNG_BUILTIN }, { "tpm-spapr", QEMU_CAPS_DEVICE_TPM_SPAPR }, @@ -6006,6 +6013,19 @@ virQEMUCapsSupportsVmport(virQEMUCaps *qemuCaps, STREQ(def->os.machine, "isapc"); }
+bool +virQEMUCapsSupportsI8042(virQEMUCaps *qemuCaps, + const virDomainDef *def) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_I8042)) + return false; + + return qemuDomainIsI440FX(def) || + qemuDomainIsQ35(def) || + qemuDomainIsXenFV(def) || + STREQ(def->os.machine, "isapc"); +} +
/* * The preferred machine to use if none is listed explicitly diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 51d951771d..d77a4bf4d9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -717,6 +717,9 @@ void virQEMUCapsInitProcessCapsInterlock(virQEMUCaps *qemuCaps); bool virQEMUCapsSupportsVmport(virQEMUCaps *qemuCaps, const virDomainDef *def);
+bool virQEMUCapsSupportsI8042(virQEMUCaps *qemuCaps, + const virDomainDef *def); + const char *virQEMUCapsGetBinary(virQEMUCaps *qemuCaps); virArch virQEMUCapsGetArch(virQEMUCaps *qemuCaps); unsigned int virQEMUCapsGetVersion(virQEMUCaps *qemuCaps); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 198ab99aef..56f09699db 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3923,9 +3923,10 @@ virXMLNamespace virQEMUDriverDomainXMLNamespace = {
static int -qemuDomainDefAddImplicitInputDevice(virDomainDef *def) +qemuDomainDefAddImplicitInputDevice(virDomainDef *def, + virQEMUCaps *qemuCaps) { - if (ARCH_IS_X86(def->os.arch)) { + if (virQEMUCapsSupportsI8042(qemuCaps, def)) { if (virDomainDefMaybeAddInput(def, VIR_DOMAIN_INPUT_TYPE_MOUSE, VIR_DOMAIN_INPUT_BUS_PS2) < 0) @@ -4164,7 +4165,7 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver, bool addITCOWatchdog = false;
/* add implicit input devices */ - if (qemuDomainDefAddImplicitInputDevice(def) < 0) + if (qemuDomainDefAddImplicitInputDevice(def, qemuCaps) < 0) return -1;
/* Add implicit PCI root controller if the machine has one */ @@ -9126,6 +9127,21 @@ qemuDomainMachineIsMipsMalta(const char *machine, return false; }
+static bool +qemuDomainMachineIsXenFV(const char *machine, + const virArch arch) +{ + if (!ARCH_IS_X86(arch)) + return false; + + if (STREQ(machine, "xenfv") || + STRPREFIX(machine, "xenfv-")) { + return true; + } + + return false; +} +
/* You should normally avoid this function and use * qemuDomainHasBuiltinIDE() instead. */ @@ -9212,6 +9228,12 @@ qemuDomainIsLoongArchVirt(const virDomainDef *def) return qemuDomainMachineIsLoongArchVirt(def->os.machine, def->os.arch); }
+bool +qemuDomainIsXenFV(const virDomainDef *def) +{ + return qemuDomainMachineIsXenFV(def->os.machine, def->os.arch); +} +
bool qemuDomainHasPCIRoot(const virDomainDef *def) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a5092dd7f0..ff45fb63ca 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -834,6 +834,7 @@ bool qemuDomainIsLoongArchVirt(const virDomainDef *def); bool qemuDomainIsRISCVVirt(const virDomainDef *def); bool qemuDomainIsPSeries(const virDomainDef *def); bool qemuDomainIsMipsMalta(const virDomainDef *def); +bool qemuDomainIsXenFV(const virDomainDef *def); bool qemuDomainHasPCIRoot(const virDomainDef *def); bool qemuDomainHasPCIeRoot(const virDomainDef *def); bool qemuDomainHasBuiltinIDE(const virDomainDef *def); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 0e8f0f977f..7730344c52 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4868,8 +4868,7 @@ qemuValidateDomainDeviceDefInput(const virDomainInputDef *input, int cap; int ccwCap;
- if (input->bus == VIR_DOMAIN_INPUT_BUS_PS2 && !ARCH_IS_X86(def->os.arch) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_I8042)) { + if (input->bus == VIR_DOMAIN_INPUT_BUS_PS2 && !virQEMUCapsSupportsI8042(qemuCaps, def)) {
Long line.
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("%1$s is not supported by this QEMU binary"), virDomainInputBusTypeToString(input->bus));
Otherwise looks good. Michal