Re: [libvirt] [PATCH 2/4] Add support for reboot-timeout
On 09/18/2012 06:50 PM, Daniel P. Berrange wrote:
On Tue, Sep 18, 2012 at 05:36:47PM +0200, 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'/>
I think it could be a bit misleading to have this element here. To
me it suggests that it applies to OS reboots in general, where as
it only really applies to the BIOS boot. Also having both an 'enabled'
flag and 'delay' attribute is redundant. Surely delay=0 is the default
and any non-zero value signifies it is enabled.
I think I'd prefer to see
<bootmenu enable='yes|no' rebootDelay='NNN'/>
leave out the rebootDelay attribute when formatting XML if it
is zero
Daniel
Thanks for suggestions, both of you.
Inintially, I had that as reboot-timeout=[-1,65535], but since '0' is
not the default [1], but a legitimate option (reboot immediately, which
I guess I should've described in the commit message), I had a special
value '-2' for the default and that's pretty awful without having
virObject construct that due to virDomainDef initialization.
With '-1' being the default, I had to come up with a system where
zero(es) could be the default, but it would enable the use of '0' as an
option as well. One version I've sent, the second one is having
rebootTimeout as an int pointer and allocating the value when
reboot-timeout is specified in the XML, which seems to me like an
overkill for just one int.
Martin
[1] http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg00815.html