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