On 1/23/23 15:57, Martin Kletzander wrote:
> This is already possible with qemu, and actually already happening with q35
> machines and a specified watchdog since q35 already includes a watchdog we do
> not include in the XML. In order to express such posibility multiple watchdogs
> need to be supported.
>
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 82 +++++++++++++------
> src/conf/domain_conf.h | 8 +-
> src/conf/schemas/domaincommon.rng | 4 +-
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_alias.c | 26 ++++--
> src/qemu/qemu_alias.h | 5 +-
> src/qemu/qemu_command.c | 24 ++++--
> src/qemu/qemu_domain_address.c | 8 +-
> src/qemu/qemu_driver.c | 14 ++--
> src/qemu/qemu_hotplug.c | 61 ++++++--------
> src/qemu/qemu_process.c | 3 +-
> src/qemu/qemu_validate.c | 25 ++++++
> .../watchdog-q35-multiple.x86_64-latest.args | 39 +++++++++
> .../watchdog-q35-multiple.xml | 25 ++++++
> tests/qemuxml2argvtest.c | 1 +
> .../watchdog-q35-multiple.x86_64-latest.xml | 50 +++++++++++
> tests/qemuxml2xmltest.c | 1 +
> 17 files changed, 287 insertions(+), 90 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/watchdog-q35-multiple.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/watchdog-q35-multiple.xml
> create mode 100644 tests/qemuxml2xmloutdata/watchdog-q35-multiple.x86_64-latest.xml
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 733399e6da0d..7c61da1d765b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3875,7 +3875,9 @@ void virDomainDefFree(virDomainDef *def)
> def->blkio.ndevices);
> g_free(def->blkio.devices);
>
> - virDomainWatchdogDefFree(def->watchdog);
> + for (i = 0; i < def->nwatchdogs; i++)
> + virDomainWatchdogDefFree(def->watchdogs[i]);
> + g_free(def->watchdogs);
>
> virDomainMemballoonDefFree(def->memballoon);
> virDomainNVRAMDefFree(def->nvram);
> @@ -4647,10 +4649,10 @@ virDomainDeviceInfoIterateFlags(virDomainDef *def,
> if ((rc = cb(def, &device, &def->fss[i]->info, opaque)) != 0)
> return rc;
> }
> - if (def->watchdog) {
> - device.type = VIR_DOMAIN_DEVICE_WATCHDOG;
> - device.data.watchdog = def->watchdog;
> - if ((rc = cb(def, &device, &def->watchdog->info, opaque)) != 0)
> + device.type = VIR_DOMAIN_DEVICE_WATCHDOG;
> + for (i = 0; i < def->nwatchdogs; i++) {
> + device.data.watchdog = def->watchdogs[i];
> + if ((rc = cb(def, &device, &def->watchdogs[i]->info, opaque)) != 0)
> return rc;
> }
> if (def->memballoon) {
> @@ -18809,24 +18811,21 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
> VIR_FREE(nodes);
>
> /* analysis of the watchdog devices */
> - def->watchdog = NULL;
> - if ((n = virXPathNodeSet("./devices/watchdog", ctxt, &nodes)) < 0)
> + n = virXPathNodeSet("./devices/watchdog", ctxt, &nodes);
> + if (n < 0)
> return NULL;
> - if (n > 1) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("only a single watchdog device is supported"));
> - return NULL;
> - }
> - if (n > 0) {
> + if (n)
> + def->watchdogs = g_new0(virDomainWatchdogDef *, n);
> + for (i = 0; i < n; i++) {
> virDomainWatchdogDef *watchdog;
>
> - watchdog = virDomainWatchdogDefParseXML(xmlopt, nodes[0], ctxt, flags);
> + watchdog = virDomainWatchdogDefParseXML(xmlopt, nodes[i], ctxt, flags);
> if (!watchdog)
> return NULL;
>
> - def->watchdog = watchdog;
> - VIR_FREE(nodes);
> + def->watchdogs[def->nwatchdogs++] = watchdog;
> }
> + VIR_FREE(nodes);
>
> /* analysis of the memballoon devices */
> def->memballoon = NULL;
> @@ -21255,18 +21254,19 @@ virDomainDefCheckABIStabilityFlags(virDomainDef *src,
> dst->redirfilter))
> goto error;
>
> - if ((!src->watchdog && dst->watchdog) ||
> - (src->watchdog && !dst->watchdog)) {
> +
> + if (src->nwatchdogs != dst->nwatchdogs) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("Target domain watchdog count %d "
> - "does not match source %d"),
> - dst->watchdog ? 1 : 0, src->watchdog ? 1 : 0);
> + _("Target domain watchdog device count %zu "
> + "does not match source %zu"),
Since you're touching this commit message, put it onto a single line please.
> + dst->nwatchdogs, src->nwatchdogs);
> goto error;
> }
>
> - if (src->watchdog &&
> - !virDomainWatchdogDefCheckABIStability(src->watchdog, dst->watchdog))
> - goto error;
> + for (i = 0; i < src->nwatchdogs; i++) {
> + if (!virDomainWatchdogDefCheckABIStability(src->watchdogs[i], dst->watchdogs[i]))
> + goto error;
> + }
>
> if ((!src->memballoon && dst->memballoon) ||
> (src->memballoon && !dst->memballoon)) {
> @@ -27517,8 +27517,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
> return -1;
> }
>
> - if (def->watchdog)
> - virDomainWatchdogDefFormat(buf, def->watchdog, flags);
> + for (n = 0; n < def->nwatchdogs; n++)
> + virDomainWatchdogDefFormat(buf, def->watchdogs[n], flags);
>
> if (def->memballoon)
> virDomainMemballoonDefFormat(buf, def->memballoon, flags);
> @@ -30578,3 +30578,33 @@ virDomainDefHasSpiceGraphics(const virDomainDef *def)
>
> return false;
> }
> +
> +
> +ssize_t
> +virDomainWatchdogDefFind(const virDomainDef *def,
> + const virDomainWatchdogDef *watchdog)
> +{
> + size_t i;
> +
> + for (i = 0; i < def->nwatchdogs; i++) {
> + const virDomainWatchdogDef *tmp = def->watchdogs[i];
> +
> + if (tmp->model != watchdog->model)
> + continue;
> +
> + if (tmp->action != watchdog->action)
> + continue;
> +
> + if (watchdog->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> + !virDomainDeviceInfoAddressIsEqual(&watchdog->info, &tmp->info))
> + continue;
> +
> + if (watchdog->info.alias &&
> + STRNEQ_NULLABLE(watchdog->info.alias, tmp->info.alias))
> + continue;
> +
> + return i;
> + }
> +
> + return -1;
> +}
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 3e4985a67da2..da785076151d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3062,12 +3062,14 @@ struct _virDomainDef {
> size_t nsysinfo;
> virSysinfoDef **sysinfo;
>
> + size_t nwatchdogs;
> + virDomainWatchdogDef **watchdogs;
> +
> /* At maximum 2 TPMs on the domain if a TPM Proxy is present. */
> size_t ntpms;
> virDomainTPMDef **tpms;
>
> /* Only 1 */
> - virDomainWatchdogDef *watchdog;
> virDomainMemballoonDef *memballoon;
> virDomainNVRAMDef *nvram;
> virCPUDef *cpu;
> @@ -3526,6 +3528,10 @@ virDomainSoundDef *virDomainSoundDefRemove(virDomainDef *def, size_t idx);
> void virDomainAudioDefFree(virDomainAudioDef *def);
> void virDomainMemballoonDefFree(virDomainMemballoonDef *def);
> void virDomainNVRAMDefFree(virDomainNVRAMDef *def);
> +ssize_t
> +virDomainWatchdogDefFind(const virDomainDef *def,
> + const virDomainWatchdogDef *watchdog)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
> void virDomainWatchdogDefFree(virDomainWatchdogDef *def);
> virDomainVideoDef *virDomainVideoDefNew(virDomainXMLOption *xmlopt);
> void virDomainVideoDefFree(virDomainVideoDef *def);
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index 6cb0a20e1e10..a5d0505d9b9b 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -6428,9 +6428,9 @@
> <ref name="memorydev"/>
> </choice>
> </zeroOrMore>
> - <optional>
> + <zeroOrMore>
> <ref name="watchdog"/>
> - </optional>
> + </zeroOrMore>
> <optional>
> <ref name="memballoon"/>
> </optional>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 576ec8f95f17..2c5489b2b86e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -692,6 +692,7 @@ virDomainVsockDefFree;
> virDomainVsockDefNew;
> virDomainWatchdogActionTypeFromString;
> virDomainWatchdogActionTypeToString;
> +virDomainWatchdogDefFind;
> virDomainWatchdogDefFree;
> virDomainWatchdogModelTypeFromString;
> virDomainWatchdogModelTypeToString;
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index ef8e87ab58a2..809a6f03ed46 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -560,12 +560,26 @@ qemuAssignDeviceShmemAlias(virDomainDef *def,
>
>
> void
> -qemuAssignDeviceWatchdogAlias(virDomainWatchdogDef *watchdog)
> +qemuAssignDeviceWatchdogAlias(virDomainDef *def,
> + virDomainWatchdogDef *watchdog,
> + int idx)
> {
> - /* Currently, there's just one watchdog per domain */
> + ssize_t i = 0;
> +
> + if (watchdog->info.alias)
> + return;
> +
> + if (idx == -1) {
> + for (i = 0; i < def->nwatchdogs; i++) {
> + idx = MAX(idx,
> + qemuDomainDeviceAliasIndex(&def->watchdogs[i]->info,
> + "watchdog"));
The MAX() macro is a glibc-ism, and worse - it evaluates arguments
twice. Please switch to pattern we use elsewhere, e.g. in
qemuAssignDeviceShmemAlias() just above.
> + }
> +
> + idx++;
> + }
>
> - if (!watchdog->info.alias)
> - watchdog->info.alias = g_strdup("watchdog0");
> + watchdog->info.alias = g_strdup_printf("watchdog%d", idx);
> }
>
>
> @@ -671,8 +685,8 @@ qemuAssignDeviceAliases(virDomainDef *def)
> for (i = 0; i < def->nsmartcards; i++) {
> qemuAssignDeviceSmartcardAlias(def->smartcards[i], i);
> }
> - if (def->watchdog) {
> - qemuAssignDeviceWatchdogAlias(def->watchdog);
> + for (i = 0; i < def->nwatchdogs; i++) {
> + qemuAssignDeviceWatchdogAlias(def, def->watchdogs[i], i);
> }
> if (def->memballoon &&
> def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
> diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
> index 6433ae4cecaf..d7f34489f570 100644
> --- a/src/qemu/qemu_alias.h
> +++ b/src/qemu/qemu_alias.h
> @@ -62,7 +62,10 @@ void qemuAssignDeviceShmemAlias(virDomainDef *def,
> virDomainShmemDef *shmem,
> int idx);
>
> -void qemuAssignDeviceWatchdogAlias(virDomainWatchdogDef *watchdog);
> +void
> +qemuAssignDeviceWatchdogAlias(virDomainDef *def,
> + virDomainWatchdogDef *watchdog,
> + int idx);
>
> void qemuAssignDeviceInputAlias(virDomainDef *def,
> virDomainInputDef *input,
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index b96f2d33c158..6e28f8b15e1c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4044,22 +4044,28 @@ qemuBuildWatchdogCommandLine(virCommand *cmd,
> const virDomainDef *def,
> virQEMUCaps *qemuCaps)
> {
> - virDomainWatchdogDef *watchdog = def->watchdog;
> - g_autoptr(virJSONValue) props = NULL;
> + virDomainWatchdogDef *watchdog = NULL;
> const char *action;
> int actualAction;
> + ssize_t i = 0;
>
> - if (!def->watchdog)
> + if (def->nwatchdogs == 0)
> return 0;
>
> - if (qemuCommandAddExtDevice(cmd, &def->watchdog->info, def, qemuCaps) < 0)
> - return -1;
> + for (i = 0; i < def->nwatchdogs; i++) {
> + g_autoptr(virJSONValue) props = NULL;
>
> - if (!(props = qemuBuildWatchdogDevProps(def, watchdog)))
> - return -1;
> + watchdog = def->watchdogs[i];
>
> - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps))
> - return -1;
> + if (qemuCommandAddExtDevice(cmd, &watchdog->info, def, qemuCaps) < 0)
> + return -1;
> +
> + if (!(props = qemuBuildWatchdogDevProps(def, watchdog)))
> + return -1;
> +
> + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps))
> + return -1;
> + }
>
> /* qemu doesn't have a 'dump' action; we tell qemu to 'pause', then
> libvirt listens for the watchdog event, and we perform the dump
Here, I'd mention in the comment that a validator made sure that ALL
watchdogs have the same action. It's not obvious at the first glance.
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index b8d1969fbebd..db4e91501d3f 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -2326,10 +2326,10 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def,
> }
>
> /* A watchdog - check if it is a PCI device */
> - if (def->watchdog &&
> - def->watchdog->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB &&
> - virDeviceInfoPCIAddressIsWanted(&def->watchdog->info)) {
> - if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->watchdog->info) < 0)
> + for (i = 0; i < def->nwatchdogs; i++) {
> + if (def->watchdogs[i]->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB &&
> + virDeviceInfoPCIAddressIsWanted(&def->watchdogs[i]->info) &&
> + qemuDomainPCIAddressReserveNextAddr(addrs, &def->watchdogs[i]->info) < 0)
> return -1;
> }
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d6879175fece..dce3bef5b85b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7258,12 +7258,13 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef,
> break;
>
> case VIR_DOMAIN_DEVICE_WATCHDOG:
> - if (vmdef->watchdog) {
> + if (virDomainWatchdogDefFind(vmdef, dev->data.watchdog) >= 0) {
> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("domain already has a watchdog"));
> + _("device is already in the domain configuration"));
> return -1;
> }
> - vmdef->watchdog = g_steal_pointer(&dev->data.watchdog);
> +
> + VIR_APPEND_ELEMENT(vmdef->watchdogs, vmdef->nwatchdogs, dev->data.watchdog);
> break;
>
> case VIR_DOMAIN_DEVICE_INPUT:
> @@ -7457,12 +7458,13 @@ qemuDomainDetachDeviceConfig(virDomainDef *vmdef,
>
>
> case VIR_DOMAIN_DEVICE_WATCHDOG:
> - if (!vmdef->watchdog) {
> + idx = virDomainWatchdogDefFind(vmdef, dev->data.watchdog);
> + if (idx < 0) {
> virReportError(VIR_ERR_DEVICE_MISSING, "%s",
> - _("domain has no watchdog"));
> + _("no matching watchdog was found"));
> return -1;
> }
> - g_clear_pointer(&vmdef->watchdog, virDomainWatchdogDefFree);
> + VIR_DELETE_ELEMENT(vmdef->watchdogs, idx, vmdef->nwatchdogs);
> break;
>
> case VIR_DOMAIN_DEVICE_INPUT:
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 1fa3cc3ea9b1..b9832ea7815a 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2928,15 +2928,9 @@ qemuDomainAttachWatchdog(virDomainObj *vm,
> virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_WATCHDOG, { .watchdog = watchdog } };
> g_autoptr(virJSONValue) props = NULL;
> bool releaseAddress = false;
> - int rv;
> + int rv = 0;
>
> - if (vm->def->watchdog) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("domain already has a watchdog"));
> - return -1;
> - }
> -
> - qemuAssignDeviceWatchdogAlias(watchdog);
> + qemuAssignDeviceWatchdogAlias(vm->def, watchdog, -1);
>
> if (watchdog->model != VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB) {
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> @@ -2954,10 +2948,13 @@ qemuDomainAttachWatchdog(virDomainObj *vm,
>
> qemuDomainObjEnterMonitor(vm);
>
> - /* QEMU doesn't have a 'dump' action; we tell qemu to 'pause', then
> - libvirt listens for the watchdog event, and we perform the dump
> - ourselves. so convert 'dump' to 'pause' for the qemu cli */
> - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) {
> + if (vm->def->nwatchdogs) {
> + /* Domain already has a watchdog and all must have the same action. */
> + rv = 0;
> + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) {
> + /* QEMU doesn't have a 'dump' action; we tell qemu to 'pause', then
> + libvirt listens for the watchdog event, and we perform the dump
> + ourselves. so convert 'dump' to 'pause' for the qemu cli */
> qemuMonitorActionWatchdog watchdogaction = QEMU_MONITOR_ACTION_WATCHDOG_KEEP;
>
> switch (watchdog->action) {
> @@ -3015,7 +3012,7 @@ qemuDomainAttachWatchdog(virDomainObj *vm,
> goto cleanup;
>
> releaseAddress = false;
> - vm->def->watchdog = watchdog;
> + VIR_APPEND_ELEMENT_COPY(vm->def->watchdogs, vm->def->nwatchdogs, watchdog);
> ret = 0;
>
> cleanup:
> @@ -4848,11 +4845,20 @@ static int
> qemuDomainRemoveWatchdog(virDomainObj *vm,
> virDomainWatchdogDef *watchdog)
> {
> + size_t i = 0;
> +
> VIR_DEBUG("Removing watchdog %s from domain %p %s",
> watchdog->info.alias, vm, vm->def->name);
>
> + for (i = 0; i < vm->def->nwatchdogs; i++) {
> + if (vm->def->watchdogs[i] == watchdog)
> + break;
> + }
> +
> qemuDomainReleaseDeviceAddress(vm, &watchdog->info);
> - g_clear_pointer(&vm->def->watchdog, virDomainWatchdogDefFree);
> + virDomainWatchdogDefFree(watchdog);
> + VIR_DELETE_ELEMENT(vm->def->watchdogs, i, vm->def->nwatchdogs);
> +
> return 0;
> }
>
> @@ -5603,33 +5609,20 @@ qemuDomainDetachPrepWatchdog(virDomainObj *vm,
> virDomainWatchdogDef *match,
> virDomainWatchdogDef **detach)
> {
> - virDomainWatchdogDef *watchdog;
> -
> - *detach = watchdog = vm->def->watchdog;
> + ssize_t idx = virDomainWatchdogDefFind(vm->def, match);
>
> - if (!watchdog) {
> - virReportError(VIR_ERR_DEVICE_MISSING, "%s",
> - _("watchdog device not present in domain configuration"));
> + if (idx < 0) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("no matching watchdog was found"));
> return -1;
> }
>
> - /* While domains can have up to one watchdog, the one supplied by the user
> - * doesn't necessarily match the one domain has. Refuse to detach in such
> - * case. */
> - if (!(watchdog->model == match->model &&
> - watchdog->action == match->action &&
> - virDomainDeviceInfoAddressIsEqual(&match->info, &watchdog->info))) {
> - virReportError(VIR_ERR_DEVICE_MISSING,
> - _("model '%s' watchdog device not present "
> - "in domain configuration"),
> - virDomainWatchdogModelTypeToString(watchdog->model));
> - return -1;
> - }
> + *detach = vm->def->watchdogs[idx];
>
> - if (watchdog->model != VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB) {
> + if ((*detach)->model != VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB) {
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> _("hot unplug of watchdog of model %s is not supported"),
> - virDomainWatchdogModelTypeToString(watchdog->model));
> + virDomainWatchdogModelTypeToString((*detach)->model));
> return -1;
> }
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index ee9f0784d3a3..218231f3596c 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -804,7 +804,8 @@ qemuProcessHandleWatchdog(qemuMonitor *mon G_GNUC_UNUSED,
> qemuDomainSaveStatus(vm);
> }
>
> - if (vm->def->watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) {
> + if (vm->def->nwatchdogs &&
> + vm->def->watchdogs[0]->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) {
> qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_WATCHDOG,
> VIR_DOMAIN_WATCHDOG_ACTION_DUMP, 0, NULL);
> }
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 6e04b22da43f..cb002dee0eb9 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1188,6 +1188,28 @@ qemuValidateDomainDefTPMs(const virDomainDef *def)
> }
>
>
> +static int
> +qemuValidateDomainDefWatchdogs(const virDomainDef *def)
> +{
> + ssize_t i = 0;
> +
> + for (i = 0; i < def->nwatchdogs; i++) {
Here, you can init i to 1, as ...
> + /* 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) {
... for i=0 case, this is never ever gonna be true.
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("watchdogs with different actions are not supported "
> + "with this QEMU binary"));
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +
Michal
