Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/conf/domain_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b03a3ff011b4..5c0b837697a0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12058,6 +12058,8 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, virDomainAudioDefFree(def); return NULL; } + + static virDomainWatchdogDef * virDomainWatchdogDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr node, -- 2.40.0

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5072798cb73d..53a0874556a1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2929,7 +2929,7 @@ qemuDomainAttachWatchdog(virDomainObj *vm, if (watchdog->model != VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("hotplug of watchdog of model %1$s is not supported"), + _("hotplug of watchdog model '%1$s' is not supported"), virDomainWatchdogModelTypeToString(watchdog->model)); goto cleanup; } -- 2.40.0

We can launch qemu with it, but it will not work since it's not even probed by the kernel at the mapped address with different machine types since they are expected to be connected to ISA and not even its newer LPC counterpart found on q35. And it does not exist on non-x86 architectures. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/qemu/qemu_validate.c | 7 +++++++ .../watchdog-q35-multiple.x86_64-latest.args | 4 ++-- tests/qemuxml2argvdata/watchdog-q35-multiple.xml | 2 +- .../watchdog-q35-multiple.x86_64-latest.xml | 4 +++- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b8d5e9bd74e9..6ebd2ba0101f 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2320,6 +2320,13 @@ qemuValidateDomainWatchdogDef(const virDomainWatchdogDef *dev, break; case VIR_DOMAIN_WATCHDOG_MODEL_IB700: + if (!qemuDomainIsI440FX(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("%1$s model of watchdog cannot be used with this machine type"), + virDomainWatchdogModelTypeToString(dev->model)); + return -1; + } + if (dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("%1$s model of watchdog does not support configuring the address"), diff --git a/tests/qemuxml2argvdata/watchdog-q35-multiple.x86_64-latest.args b/tests/qemuxml2argvdata/watchdog-q35-multiple.x86_64-latest.args index 655d206421b6..4c14a9552088 100644 --- a/tests/qemuxml2argvdata/watchdog-q35-multiple.x86_64-latest.args +++ b/tests/qemuxml2argvdata/watchdog-q35-multiple.x86_64-latest.args @@ -31,8 +31,8 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -device '{"driver":"pcie-root-port","port":9,"chassis":3,"id":"pci.3","bus":"pcie.0","addr":"0x1.0x1"}' \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.2","addr":"0x1"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ --device '{"driver":"ib700","id":"watchdog0"}' \ --device '{"driver":"i6300esb","id":"watchdog1","bus":"pci.2","addr":"0x2"}' \ +-device '{"driver":"i6300esb","id":"watchdog0","bus":"pci.2","addr":"0x2"}' \ +-device '{"driver":"i6300esb","id":"watchdog1","bus":"pci.2","addr":"0x3"}' \ -global ICH9-LPC.noreboot=off \ -watchdog-action poweroff \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/watchdog-q35-multiple.xml b/tests/qemuxml2argvdata/watchdog-q35-multiple.xml index af0cb169f0d9..1ed3aff41482 100644 --- a/tests/qemuxml2argvdata/watchdog-q35-multiple.xml +++ b/tests/qemuxml2argvdata/watchdog-q35-multiple.xml @@ -18,7 +18,7 @@ <controller type='usb' index='0'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <watchdog model='ib700' action='poweroff'/> + <watchdog model='i6300esb' action='poweroff'/> <watchdog model='i6300esb' action='poweroff'/> <memballoon model='none'/> </devices> diff --git a/tests/qemuxml2xmloutdata/watchdog-q35-multiple.x86_64-latest.xml b/tests/qemuxml2xmloutdata/watchdog-q35-multiple.x86_64-latest.xml index 7e60c6e6517d..1ea084854ee6 100644 --- a/tests/qemuxml2xmloutdata/watchdog-q35-multiple.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/watchdog-q35-multiple.x86_64-latest.xml @@ -41,10 +41,12 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <audio id='1' type='none'/> - <watchdog model='ib700' action='poweroff'/> <watchdog model='i6300esb' action='poweroff'> <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> </watchdog> + <watchdog model='i6300esb' action='poweroff'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x03' function='0x0'/> + </watchdog> <watchdog model='itco' action='poweroff'/> <memballoon model='none'/> </devices> -- 2.40.0

This makes it also work during attach. Also add a test for attaching a watchdog with incompatible action. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2187278 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/qemu/qemu_validate.c | 22 ++++++++++--------- tests/qemuhotplugtest.c | 3 +++ .../qemuhotplug-watchdog-reset.xml | 1 + ...plug-base-live+watchdog+watchdog-reset.xml | 1 + 4 files changed, 17 insertions(+), 10 deletions(-) create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-watchdog-reset.xml create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog+watchdog-reset.xml diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index a03f7056b191..b2c3fd1785bc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1114,16 +1114,6 @@ qemuValidateDomainDefWatchdogs(const virDomainDef *def) size_t i = 0; for (i = 0; i < def->nwatchdogs; i++) { - /* We could theoretically support different watchdogs having dump and - * pause, but let's be honest, we support multiple watchdogs only - * because we need to be able to add a second, implicit one, not because - * it is a brilliant idea to have multiple watchdogs. */ - if (def->watchdogs[i]->action != def->watchdogs[0]->action) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("watchdogs with different actions are not supported " - "with this QEMU binary")); - return -1; - } if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_ITCO) { if (found_itco) { @@ -2308,6 +2298,18 @@ static int qemuValidateDomainWatchdogDef(const virDomainWatchdogDef *dev, const virDomainDef *def) { + /* We could theoretically support different watchdogs having dump and + * pause, but let's be honest, we support multiple watchdogs only + * because we need to be able to add a second, implicit one, not because + * it is a brilliant idea to have multiple watchdogs. */ + if (def->nwatchdogs && + def->watchdogs[0]->action != dev->action) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("watchdogs with different actions are not supported " + "with this QEMU binary")); + return -1; + } + switch ((virDomainWatchdogModel) dev->model) { case VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB: if (dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 63a6833cfde7..6aaccce55b39 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -908,6 +908,9 @@ mymain(void) DO_TEST_DETACH("x86_64", "base-live", "watchdog-user-alias-full", false, false, "device_del", QMP_DEVICE_DELETED("ua-UserWatchdog") QMP_OK); + /* attaching a watchdog with different action should fail */ + DO_TEST_ATTACH("x86_64", "base-live+watchdog", "watchdog-reset", true, false, NULL); + DO_TEST_ATTACH("x86_64", "base-live", "guestfwd", false, true, "getfd", QMP_OK, "chardev-add", QMP_OK, diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-watchdog-reset.xml b/tests/qemuhotplugtestdevices/qemuhotplug-watchdog-reset.xml new file mode 100644 index 000000000000..cb1c072e1ac2 --- /dev/null +++ b/tests/qemuhotplugtestdevices/qemuhotplug-watchdog-reset.xml @@ -0,0 +1 @@ +<watchdog model='i6300esb' action='reset'/> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog+watchdog-reset.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog+watchdog-reset.xml new file mode 120000 index 000000000000..d3d1764fd897 --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+watchdog+watchdog-reset.xml @@ -0,0 +1 @@ +qemuhotplug-base-live+watchdog.xml \ No newline at end of file -- 2.40.0

On 4/19/23 16:07, Martin Kletzander wrote: We usually us virBitmap for this... It should be fairly trivial to rewrite. Michal

On 4/20/23 08:42, Martin Kletzander wrote: Perfect. Michal