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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list