[libvirt] [PATCH] Support QEMU/KVM watchdog device
Time to brush off this old patch. This adds simple support for the QEMU/KVM emulated hardware watchdog device (qemu commit 9dd986ccf68f142aaafe543d80cf877716d91d4e). Event notification isn't supported. This just lets you configure a domain with a <watchdog.../> device, select the model and action. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
On Wed, Oct 21, 2009 at 01:32:49PM +0100, Richard W.M. Jones wrote:
Time to brush off this old patch. This adds simple support for the QEMU/KVM emulated hardware watchdog device (qemu commit 9dd986ccf68f142aaafe543d80cf877716d91d4e).
Event notification isn't supported. This just lets you configure a domain with a <watchdog.../> device, select the model and action. [...] + /* analysis of the watchdog devices */ + def->watchdog = NULL; + if ((n = virXPathNodeSet(conn, "./devices/watchdog", ctxt, &nodes)) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract watchdog devices")); + goto error; + }
Hum, I'm afraid this will lead to errros for any defintition without a watchdog ! Since that's completely optional we really should not error there IMHO and just skip it.
+ if (n > 1) { + virDomainReportError (conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("only a single watchdog device is supported")); + goto error; + } + if (n > 0) { + virDomainWatchdogDefPtr watchdog = + virDomainWatchdogDefParseXML (conn, nodes[0], flags); + if (!watchdog) + goto error; + + def->watchdog = watchdog; + VIR_FREE(nodes); + }
It looks a bit more complex than it should but rereading it that's right. ACK when the previous parsing problem is fixed or verified, it would be good to get some regression tests to verify the XML parsing, saving and QEmu command line generation. As well as extending the domain.rng as you raised on IRC :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Wed, Oct 21, 2009 at 04:42:06PM +0200, Daniel Veillard wrote:
On Wed, Oct 21, 2009 at 01:32:49PM +0100, Richard W.M. Jones wrote:
+ if ((n = virXPathNodeSet(conn, "./devices/watchdog", ctxt, &nodes)) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract watchdog devices")); + goto error; + }
Hum, I'm afraid this will lead to errros for any defintition without a watchdog ! Since that's completely optional we really should not error there IMHO and just skip it.
Trouble is, the code preceeding this would fail first. It seems to be a generic problem with all those calls to virXPathNodeSet in that case.
As well as extending the domain.rng as you raised on IRC :-)
The attached patch includes changes to the domain.rng and adds XML <-> qemu command line tests. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
On Wed, Oct 21, 2009 at 03:52:18PM +0100, Richard W.M. Jones wrote:
On Wed, Oct 21, 2009 at 04:42:06PM +0200, Daniel Veillard wrote:
On Wed, Oct 21, 2009 at 01:32:49PM +0100, Richard W.M. Jones wrote:
+ if ((n = virXPathNodeSet(conn, "./devices/watchdog", ctxt, &nodes)) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract watchdog devices")); + goto error; + }
Hum, I'm afraid this will lead to errros for any defintition without a watchdog ! Since that's completely optional we really should not error there IMHO and just skip it.
Trouble is, the code preceeding this would fail first. It seems to be a generic problem with all those calls to virXPathNodeSet in that case.
Yup, I'm sending a separate patch to cover this,
As well as extending the domain.rng as you raised on IRC :-)
The attached patch includes changes to the domain.rng and adds XML <-> qemu command line tests. [...] --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1013,6 +1013,27 @@ </attribute> </element> </define> + <define name="watchdog"> + <element name="watchdog"> + <attribute name="model"> + <choice> + <value>i6300esb</value> + <value>ib700</value> + </choice> + </attribute> + <optional> + <attribute name="action"> + <choice> + <value>reset</value> + <value>shutdown</value> + <value>poweroff</value> + <value>pause</value> + <value>none</value> + </choice> + </attribute> + </optional> + </element> + </define> <define name="parallel"> <element name="parallel"> <ref name="qemucdev"/> @@ -1139,6 +1160,9 @@ <ref name="serial"/> </choice> </zeroOrMore> + <optional> + <ref name="watchdog"/> + </optional> </interleave> </element> </define>
okay but indentation seems funky, can you double check ? [...]
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 65d4d14..1b16aa9 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -212,6 +212,7 @@ mymain(int argc, char **argv) DO_TEST("parallel-tcp", 0); DO_TEST("console-compat", 0); DO_TEST("sound", 0); + DO_TEST("watchdog", 0);
DO_TEST("hostdev-usb-product", 0); DO_TEST("hostdev-usb-address", 0); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog.args b/tests/qemuxml2argvdata/qemuxml2argv-watchdog.args new file mode 100644 index 0000000..40a656b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -watchdog ib700 -watchdog-action poweroff diff --git a/tests/qemuxml2argvdata/qemuxml2argv-watchdog.xml b/tests/qemuxml2argvdata/qemuxml2argv-watchdog.xml new file mode 100644 index 0000000..9b2ffdf --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-watchdog.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + </disk> + <watchdog model='ib700' action='poweroff'/> + </devices> +</domain> --
ACK, looks fine to me ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Wed, Oct 21, 2009 at 05:28:49PM +0200, Daniel Veillard wrote:
okay but indentation seems funky, can you double check ?
Tabs/spaces thing. I'll fix it up before pushing. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
participants (2)
-
Daniel Veillard -
Richard W.M. Jones