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

[Markus Armbruster <armbru@redhat.com>]

Gonglei <arei.gonglei@huawei.com> writes:

> On 2015/1/30 20:01, Markus Armbruster wrote:
>
>> Gonglei <arei.gonglei@huawei.com> writes:
>> 
>>> On 2015/1/30 15:46, Markus Armbruster wrote:
>>>
>>>> Gonglei <arei.gonglei@huawei.com> writes:
>>>>
>>>>> On 2015/1/30 0:03, Alexander Graf wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On 29.01.15 14:29, arei.gonglei@huawei.com wrote:
>>>>>>> From: Gonglei <arei.gonglei@huawei.com>
>>>>>>>
>>>>>>> If boot order is invaild or is set failed,
>>>>>>> exit qemu.
>>>>>>>
>>>>>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>>>>>>
>>>>>> 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.
>> 
>
> Sorry, I meant the validate_bootdevices() can't fail in restore_boot_order(),
> but boot_set_handler(boot_set_opaque, boot_order, errp) may fail, such as
> set_boot_dev(). For example:
> x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 4096 -boot
> menu=on,order=nbcdep,once=c -monitor stdio -vnc :0
> QEMU 2.2.50 monitor - type 'help' for more information
> (qemu) system_reset
> (qemu) qemu-system-x86_64: Too many boot devices for PC

The value of parameter order should be checked "during command line
processing (strongly preferred) or initial startup (acceptable)" if at
all possible.  Is it possible?