[libvirt] [PATCH 2/4] Add support for reboot-timeout

[Michal Privoznik]

On 18.09.2012 17:36, 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     |  9 +++++++++
>  docs/schemas/domaincommon.rng | 20 ++++++++++++++++++++
>  src/conf/domain_conf.c        | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h        | 15 +++++++++++++++
>  4 files changed, 84 insertions(+)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 51f897c..b4050cf 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -106,6 +106,7 @@
>      <bootmenu enable='yes'/>
>      <smbios mode='sysinfo'/>
>      <bios useserial='yes'/>
> +    <reboot-timeout enabled='yes' delay='0'/>
>    </os>
>    ...</pre>
> 
> @@ -177,6 +178,14 @@
>          <a href="#elementCharSerial">serial port</a> defined.
>          <span class="since">Since 0.9.4</span>
>        </dd>
> +      <dt><code>reboot-timeout</code></dt>
> +      <dd>This element controls whether and after how long the guest should
> +        start booting again in case the boot fails (according to BIOS). The
> +        feature depends on mandatory parameter <code>enabled</enabled> with
> +        values <code>yes</code> and <code>no</code>. When enabled, value of
> +        the second parameter <code>delay</code> (in milliseconds) controls how
> +        long before the boot starts again. Maximum delay is <code>65535</code>
> +        milliseconds. <span class="since">QEMU since 0.10.2.</span></dd>
>      </dl>


Milliseconds is a bit overkill (too fine) for me. But I can see where is
this coming from. So I can live with that.

> 
>      <h4><a name="elementsOSBootloader">Host bootloader</a></h4>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index aafb10c..af3d856 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -264,6 +264,9 @@
>          <optional>
>            <ref name="bios"/>
>          </optional>
> +        <optional>
> +          <ref name="reboot-timeout"/>
> +        </optional>
>        </interleave>
>      </element>
>    </define>
> @@ -3199,6 +3202,23 @@
>      </element>
>    </define>
> 
> +  <define name="reboot-timeout">
> +    <element name="reboot-timeout">
> +      <attribute name="enabled">
> +        <choice>
> +          <value>yes</value>
> +          <value>no</value>
> +        </choice>
> +      </attribute>
> +      <optional>
> +        <attribute name="delay">
> +          <ref name="unsignedShort"/>
> +        </attribute>
> +      </optional>
> +      <empty/>
> +    </element>
> +  </define>
> +

Do we need 'enabled' and 'delay' at the same time? I mean, what if
'delay' would take [-1,65535] with -1 meaning its disabled?



>    <define name="address">
>      <element name="address">
>        <choice>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 35814fb..98a39bb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -107,6 +107,11 @@ VIR_ENUM_IMPL(virDomainBootMenu, VIR_DOMAIN_BOOT_MENU_LAST,
>                "yes",
>                "no")
> 
> +VIR_ENUM_IMPL(virDomainRebootTimeout, VIR_DOMAIN_REBOOT_TIMEOUT_LAST,
> +              "default",
> +              "yes",
> +              "no")
> +
>  VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
>                "acpi",
>                "apic",
> @@ -9788,6 +9793,32 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>          def->os.smbios_mode = VIR_DOMAIN_SMBIOS_NONE; /* not present */
>      }
> 
> +    tmp = virXPathString("string(./os/reboot-timeout/@enabled)", ctxt);
> +    if (tmp) {
> +        int num = virDomainRebootTimeoutTypeFromString(tmp);
> +
> +        if (num < 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("unknown value for reboot-timeout "
> +                             "enabled parameter '%s'"), tmp);
> +            goto error;
> +        }
> +
> +        VIR_FREE(tmp);
> +        def->os.rebootTimeout.enabled = num;
> +
> +        if (num == VIR_DOMAIN_REBOOT_TIMEOUT_ENABLED) {
> +            if (virXPathInt("string(./os/reboot-timeout/@delay)", ctxt, &num) < 0 ||
> +                num < 0 || num > 0xffff) {

s/0xffff/65535/

> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("reboot-timeout delay value must "
> +                                 "be in range [0,65535] and not"));

unfinished sentence

> +                goto error;
> +            }
> +            def->os.rebootTimeout.delay = num;
> +        }
> +    }
> +
>      /* Extract custom metadata */
>      if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL) {
>          def->metadata = xmlCopyNode(node, 1);
> @@ -13514,6 +13545,15 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>          virBufferAsprintf(buf, "    <smbios mode='%s'/>\n", mode);
>      }
> 
> +    if (def->os.rebootTimeout.enabled) {
> +        virBufferAsprintf(buf, "    <reboot-timeout enabled='%s'",
> +                          virDomainRebootTimeoutTypeToString(def->os.rebootTimeout.enabled));
> +        if (def->os.rebootTimeout.enabled == VIR_DOMAIN_REBOOT_TIMEOUT_ENABLED)
> +            virBufferAsprintf(buf, " delay='%d'",
> +                              def->os.rebootTimeout.delay);
> +        virBufferAddLit(buf, "/>\n");
> +    }
> +
>      virBufferAddLit(buf, "  </os>\n");
> 
>      if (def->features) {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 510406a..c831d6d 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1363,6 +1363,14 @@ enum virDomainBootMenu {
>      VIR_DOMAIN_BOOT_MENU_LAST
>  };
> 
> +enum virDomainRebootTimeout {
> +    VIR_DOMAIN_REBOOT_TIMEOUT_DEFAULT = 0,
> +    VIR_DOMAIN_REBOOT_TIMEOUT_ENABLED,
> +    VIR_DOMAIN_REBOOT_TIMEOUT_DISABLED,
> +
> +    VIR_DOMAIN_REBOOT_TIMEOUT_LAST
> +};
> +
>  enum virDomainFeature {
>      VIR_DOMAIN_FEATURE_ACPI,
>      VIR_DOMAIN_FEATURE_APIC,
> @@ -1444,6 +1452,12 @@ struct _virDomainOSDef {
>      char *bootloaderArgs;
>      int smbios_mode;
> 
> +    struct {
> +        /* enum virDomainRebootTimeout */
> +        int enabled;
> +        int delay;
> +    } rebootTimeout;
> +
>      virDomainBIOSDef bios;
>  };
> 
> @@ -2134,6 +2148,7 @@ VIR_ENUM_DECL(virDomainTaint)
>  VIR_ENUM_DECL(virDomainVirt)
>  VIR_ENUM_DECL(virDomainBoot)
>  VIR_ENUM_DECL(virDomainBootMenu)
> +VIR_ENUM_DECL(virDomainRebootTimeout)
>  VIR_ENUM_DECL(virDomainFeature)
>  VIR_ENUM_DECL(virDomainApicEoi)
>  VIR_ENUM_DECL(virDomainLifecycle)
> 

Take this as half ACK; I mean, the code is okay, but I am not sure about
XML. Does anybody have any suggestions?

Michal