[libvirt] [PATCH] Support QEMU/KVM watchdog device

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/