[libvirt] [PATCH v2 1/3] Add support for reboot-timeout

[Peter Krempa]

On 09/19/12 19:22, Martin Kletzander wrote:
> Whenever the guest machine fails to boot, new parameter (reboot-timeout)
> controls whether it should reboot and after how many ms it should do so.
>
> Docs included.
> ---
>   docs/formatdomain.html.in     | 11 ++++++++---
>   docs/schemas/domaincommon.rng | 24 ++++++++++++++++++------
>   src/conf/domain_conf.c        | 34 ++++++++++++++++++++++++++++------
>   src/conf/domain_conf.h        |  3 +++
>   4 files changed, 57 insertions(+), 15 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 51f897c..d021837 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -105,7 +105,7 @@
>       <boot dev='cdrom'/>
>       <bootmenu enable='yes'/>
>       <smbios mode='sysinfo'/>
> -    <bios useserial='yes'/>
> +    <bios useserial='yes' reboot-timeout='0'/>
>     </os>
>     ...</pre>
>
> @@ -175,8 +175,13 @@
>           Serial Graphics Adapter which allows users to see BIOS messages
>           on a serial port. Therefore, one needs to have
>           <a href="#elementCharSerial">serial port</a> defined.
> -        <span class="since">Since 0.9.4</span>
> -      </dd>
> +        <span class="since">Since 0.9.4</span>.
> +        <span class="since">Since 0.10.2 (QEMU only)</span> there is
> +        another attribute, <code>reboot-timeout</code> that controls
> +        whether and after how long the guest should start booting
> +        again in case the boot fails (according to BIOS). The value is
> +        in milliseconds with maximum of <code>65535</code> and special
> +        value <code>-1</code> disables the reboot.
>       </dl>
>
>       <h4><a name="elementsOSBootloader">Host bootloader</a></h4>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index aafb10c..ed25f58 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3190,12 +3190,19 @@
>
>     <define name="bios">
>       <element name="bios">
> -      <attribute name="useserial">
> -        <choice>
> -          <value>yes</value>
> -          <value>no</value>
> -        </choice>
> -      </attribute>
> +      <optional>
> +        <attribute name="useserial">
> +          <choice>
> +            <value>yes</value>
> +            <value>no</value>
> +          </choice>
> +        </attribute>
> +      </optional>
> +      <optional>
> +        <attribute name="reboot-timeout">

As DanPB pointed out, this should be changed to camel case.

> +          <ref name="RebootTimeout"/>
> +        </attribute>
> +      </optional>
>       </element>
>     </define>
>
> @@ -3469,6 +3476,11 @@
>         <param name='minInclusive'>-1</param>
>       </data>
>     </define>
> +  <define name="RebootTimeout">
> +    <data type="short">
> +      <param name="minInclusive">-1</param>
> +    </data>
> +  </define>
>     <define name="PortNumber">
>       <data type="short">
>         <param name="minInclusive">-1</param>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 35814fb..4714312 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8136,7 +8136,7 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
>   {
>       xmlNodePtr *nodes = NULL;
>       int i, n;
> -    char *bootstr;
> +    char *bootstr, *tmp;
>       char *useserial = NULL;
>       int ret = -1;
>       unsigned long deviceBoot, serialPorts;
> @@ -8214,6 +8214,22 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
>           }
>       }
>
> +    tmp = virXPathString("string(./os/bios[1]/@reboot-timeout)", ctxt);
> +    if (tmp) {
> +        /* that was really just for the check if it is there */
> +        VIR_FREE(tmp);
> +
> +        if ((virXPathInt("string(./os/bios[1]/@reboot-timeout)",

Ew. Doing the XPath query twice seems like a waste to me. What about 
using virStrToLong_i on the string instead of querying again?

> +                         ctxt, &def->os.bios.rt_delay) < 0) ||
> +            def->os.bios.rt_delay < -1 || def->os.bios.rt_delay > 65535) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("invalid value for reboot-timeout, "
> +                             "must be in range [-1,65535]"));
> +            goto cleanup;
> +        }
> +        def->os.bios.rt_set = true;
> +    }
> +
>       *bootCount = deviceBoot;
>       ret = 0;
>
> @@ -13494,11 +13510,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>               virBufferAsprintf(buf, "    <bootmenu enable='%s'/>\n", enabled);
>           }
>
> -        if (def->os.bios.useserial) {
> -            const char *useserial = (def->os.bios.useserial ==
> -                                     VIR_DOMAIN_BIOS_USESERIAL_YES ? "yes"
> -                                                                   : "no");
> -            virBufferAsprintf(buf, "    <bios useserial='%s'/>\n", useserial);
> +        if (def->os.bios.useserial || def->os.bios.rt_set) {
> +            virBufferAddLit(buf, "    <bios");
> +            if (def->os.bios.useserial)
> +                virBufferAsprintf(buf, " useserial='%s'",
> +                                  (def->os.bios.useserial ==
> +                                   VIR_DOMAIN_BIOS_USESERIAL_YES ? "yes"
> +                                                                   : "no"));
> +            if (def->os.bios.rt_set)
> +                virBufferAsprintf(buf, " reboot-timeout='%d'", def->os.bios.rt_delay);
> +
> +            virBufferAddLit(buf, "/>\n");
>           }
>       }
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 510406a..d719d57 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1420,6 +1420,9 @@ typedef struct _virDomainBIOSDef virDomainBIOSDef;
>   typedef virDomainBIOSDef *virDomainBIOSDefPtr;
>   struct _virDomainBIOSDef {
>       int useserial;
> +    /* reboot-timeout parameters */
> +    bool rt_set;
> +    int rt_delay;
>   };
>
>   /* Operating system configuration data & machine / arch */
>

ACK if you don't do the xpath query twice and (carefully) rename the 
element.

Peter