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

[Han Han]

Hi Michal,
Any more advice update?

On Tue, Oct 8, 2019 at 8:42 PM Han Han <hhan at redhat.com> wrote:

>
>
> 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
>


-- 
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/20191015/04402b89/attachment-0001.htm>