Re: [Qemu-devel] [PATCH 2/2] bootdevice: add check in restore_boot_order()

[Markus Armbruster]

Gonglei <address@hidden> writes:

> On 2015/1/30 15:46, Markus Armbruster wrote:
>
>> Gonglei <address@hidden> writes:
>> 
>>> On 2015/1/30 0:03, Alexander Graf wrote:
>>>
>>>>
>>>>
>>>> On 29.01.15 14:29, address@hidden wrote:
>>>>> From: Gonglei <address@hidden>
>>>>>
>>>>> If boot order is invaild or is set failed,
>>>>> exit qemu.
>>>>>
>>>>> Signed-off-by: Gonglei <address@hidden>
>>>>
>>>> Do we really want to kill the machine only because the boot device
>>>> string doesn't validate?
>>>>
>>>
>>> Not all of the situation. If people want to change boot order by qmp/hmp
>>> command, it just report an error, please see do_boot_set(). But if the boot
>>> order is set in qemu command line, it will exit qemu if the boot
>>> device string
>>> is invalidate, as this patch's situation, which follow the original
>>> processing
>>> way (commit ef3adf68).
>> 
>> I think Alex isn't concerned about the monitor command, but what happens
>> when boot order "once" is reset to "order" on system reset.
>> 
>> -boot errors should have been detected during command line processing
>> (strongly preferred) or initial startup (acceptable).  Detecting
>
> Yes, and it had done it just like that, please see main() of vl.c. So, 
> actually
> it wouldn't fail in the check of restore_boot_order function's calling.
> The only possible fails will happen to call boot_set_handler(). Take
> x86 pc machine example, set_boot_dev() callback  may return errors.

I don't like unreachable error messages.  If qemu_boot_set() can't fail
in restore_boot_order(), then simply assert it doesn't fail, by passing
&error_abort.

>> configuration errors during operation is nasty.  In cases where we can't
>> avoid it (and I'm not sure this is one), we need to consider very
>> carefully whether the error should be fatal.
>
> Indeed, maybe we only need to set boot order failed  and report
> an error message in this scenario, do you agree?
>
> Regards,
> -Gonglei