[libvirt] [PATCH 1/3] conf: Set rebootTimeout valid range to 0..0xffff

[Han Han]

On Tue, Oct 8, 2019 at 5:50 PM Michal Privoznik <mprivozn at redhat.com> wrote:

> On 10/8/19 10:36 AM, Han Han wrote:
> > Adjust valid range of rebootTimeout according to qemu-4.0.0 commit
> > ee5d0f89de3.
> >
> > Signed-off-by: Han Han <hhan at redhat.com>
> > ---
> >   src/conf/domain_conf.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index a53cd6a725..57ab254f52 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -18090,10 +18090,12 @@ virDomainDefParseBootXML(xmlXPathContextPtr
> ctxt,
> >               /* that was really just for the check if it is there */
> >
> >               if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) <
> 0 ||
> > -                def->os.bios.rt_delay < -1 || def->os.bios.rt_delay >
> 65535) {
> > +                def->os.bios.rt_delay < 0 || def->os.bios.rt_delay >
> 65535) {
> >                   virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> >                                  _("invalid value for rebootTimeout, "
> > -                                 "must be in range [-1,65535]"));
> > +                                 "must be in range [0,65535]. "
> > +                                 "To disable reboot, "
> > +                                 "just remove this attribute."));
> >                   return -1;
> >               }
> >               def->os.bios.rt_set = true;
> >
>
> Firstly¸patch 2/3 must come before 1/3 because we require patches to be
> able to compile & run 'make syntax-check check' successfuly after every
> single one.
>
> But more serious problem is, that we document that -1 is a special value
> that disables automatic reboot. So did QEMU just lose functionality
> there? If they have some other way to prevent automatic reboot on failed
>
Yes.
The qemu commit ee5d0f89de3 says:
" This patch checks for conversion errors properly, and reject all values
  outside 0...0xffff."
And check the definition of fw_cfg_reboot(), you can found the default value
passed to pointer argument is alwarys -1 before or after the commit.

Test on qemu-4.0.0:
# qemu-system-x86_64 -boot reboot-timeout=-1 /tmp/new
WARNING: Image format was not specified for '/tmp/new' and probing guessed
raw.
         Automatically detecting the format is dangerous for raw images,
write operations on block 0 will be restricted.
         Specify the 'raw' format explicitly to remove the restrictions.
qemu-system-x86_64: reboot timeout is invalid,it should be a value between
0 and 65535

# echo $?
1

> boot, then we need to use that if user requested -1.
>
> Michal
>


-- 
Best regards,
-----------------------------------
Han Han
Quality Engineer
Redhat.

Email: hhan at redhat.com
Phone: +861065339333
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191008/992f3f57/attachment-0001.htm>