Re: [PATCH 2/7] Support multiple watchdog devices
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
> ---
> 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, , >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, , >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, , >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, )) < 0)
> +n = virXPathNodeSet("./devices/watchdog", ctxt, );
> +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;
> }
[PATCH 2/7] Support multiple watchdog devices
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
---
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, , >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, , >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, , >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, )) < 0)
+n = virXPathNodeSet("./devices/watchdog", ctxt, );
+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"),
+ 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) ||
