On 2015/2/2 17:37, Markus Armbruster wrote: > Gonglei <arei.gong...@huawei.com> writes: > >> On 2015/1/30 20:32, Markus Armbruster wrote: >> >>> Gonglei <arei.gong...@huawei.com> writes: >>> >>>> On 2015/1/30 20:01, Markus Armbruster wrote: >>>> >>>>> Gonglei <arei.gong...@huawei.com> writes: >>>>> >>>>>> On 2015/1/30 15:46, Markus Armbruster wrote: >>>>>> >>>>>>> Gonglei <arei.gong...@huawei.com> writes: >>>>>>> >>>>>>>> On 2015/1/30 0:03, Alexander Graf wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 29.01.15 14:29, arei.gong...@huawei.com wrote: >>>>>>>>>> From: Gonglei <arei.gong...@huawei.com> >>>>>>>>>> >>>>>>>>>> If boot order is invaild or is set failed, >>>>>>>>>> exit qemu. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Gonglei <arei.gong...@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? >> >> Either 'once' option or 'order' option can take effect for -boot at >> the same time, >> that is say initial startup processing can check only one. Besides, >> the check is just for >> corresponding machine type, so command line processing also can't do it. > > I challenge your idea that we can't check this before the guest starts > running. > > qemu_boot_set() can fail for two reasons:
There is a third reason that boot_set_handler is not null, but fails in really executing time. You can see my above example about function set_boot_dev(), the handler of pc machine. > > * validate_bootdevices() fails > > Should never happen, because we've called it in main() already, > treating failure as fatal error. Yes. > > * boot_set_handler is null > > MachineClass method init() may set this. main() could *easily* test > whether it did! If it didn't, and -boot once is given, error out. > Similar checks exist already, e.g. drive_check_orphaned(), > net_check_clients(). They only warn, but that's detail. I agree, just need to report the error message. Regards, -Gonglei
