On Tue, Nov 12, 2019 at 5:15 PM Markus Armbruster <armbru@redhat.com> wrote:
I apologize for my slooow response; KVM Forum happened.
It doesn't matter. I attended KVM forum, too :)

Han Han <hhan@redhat.com> writes:

> On Tue, Oct 15, 2019 at 4:04 PM Michal Privoznik <mprivozn@redhat.com>
> wrote:
>
>> On 10/15/19 7:23 AM, Han Han wrote:
>> > Hi Michal,
>> > Any more advice update?
>>
>> Well, as I've said earlier, since we document that -1 is accepted value
>> and it means that it suppresses automatic reboots we need a way to
>> preserve this behaviour. For instance, what happens if you don't put
>> reboot-timeout onto the cmd line at all? Does qemu use some default and
>>
> If no  reboot-timeout, qemu will not reboot by default. I have updated the
> qemu doc:
> https://github.com/qemu/qemu/commit/bbd9e6985ff342cbe15b9cb7eb30e842796fbbe8

Since your message, upstream QEMU got this:

commit 20a192203222efde055df688cc344f9efb87c372
Author: Dr. David Alan Gilbert <dgilbert@redhat.com>
Date:   Fri Oct 25 17:57:06 2019 +0100

    fw_cfg: Allow reboot-timeout=-1 again

    Commit ee5d0f89de3e53cdb0dc added range checking on reboot-timeout
    to only allow the range 0..65535; however both qemu and libvirt document
    the special value -1  to mean don't reboot.
    Allow it again.

    Fixes: ee5d0f89de3e53cdb0dc ("fw_cfg: Fix -boot reboot-timeout error checking")
    RH bz: https://bugzilla.redhat.com/show_bug.cgi?id=1765443
    Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
    Message-Id: <20191025165706.177653-1-dgilbert@redhat.com>
    Suggested-by: Laszlo Ersek <lersek@redhat.com>
    Message-Id: <37ac197c-f20e-dd05-ff6a-13a2171c7148@redhat.com>
    [PMD: Applied Laszlo's suggestions]
    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
    Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Commit bbd9e6985f "qemu-options.hx: Update for reboot-timeout parameter"
could be reverted now.  Rewriting the parapgraph for clarity would be
better, though.

Agree

>> reboot anyways? If it doesn't reboot then -1 should mean to not put
>>
> I think they use the same defaults and will not reboot by default. That can
> be checked by
> the code before and after qemu commit ee5d0f8. The values passed to
> fw_cfg_add_file are
> same by default.
>
> Markus Armbruster,
> Could you please confirm that, for cases of
> - reboot-timeout=-1 before ee5d0f8
> - reboot-timeout by default before ee5d0f8
> - reboot-timeout by default after  ee5d0f8
> they all will not reboot?

Is this still necessary now that reboot-timeout=-1 has been fixed in
upstream QEMU?
It is not necessary to introduce change to libvirt in reboot-timeout now.
BTW, the default value of reboot-timeout is -1, right?

>> reboot-timeout onto the cmd line. However, if it does reboot then we
>> need to talk to qemu developers to provide us a way to suppress
>> automatic reboots.



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

Email: hhan@redhat.com
Phone: +861065339333