On 2015/2/3 15:49, Markus Armbruster wrote: > You're right. pc.c's set_boot_dev() fails when its boot order argument > is invalid. > > The boot order interface is crap, because it makes detecting > configuration errors early hard. Two solutions: > > A. It may be hard, but not too hard for the determined > > 1. If "once" is given, register reset handler to restore boot order. > > 2. Pass the normal boot order to machine creation. Should fail when > the normal boot order is invalid. > > 3. If "once" is given, set it with qemu_boot_set(). Fails when the > once boot order is invalid. > > 4. Start the machine. > > 5. On reset, the reset handler calls qemu_boot_set() to restore boot > order. Should never fail. >
What about the below patch? diff --git a/vl.c b/vl.c index 983259b..7d37191 100644 --- a/vl.c +++ b/vl.c @@ -126,6 +126,7 @@ int main(int argc, char **argv) @@ -126,6 +126,7 @@ int main(int argc, char **argv) --- a/vl.c +++ b/vl.c @@ -126,6 +126,7 @@ int main(int argc, char **argv) static const char *data_dir[16]; static int data_dir_idx; +const char *once = NULL; const char *bios_name = NULL; enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB; DisplayType display_type = DT_DEFAULT; @@ -4046,7 +4047,7 @@ int main(int argc, char **argv, char **envp) opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL); if (opts) { char *normal_boot_order; - const char *order, *once; + const char *order; Error *local_err = NULL; order = qemu_opt_get(opts, "order"); @@ -4067,7 +4068,6 @@ int main(int argc, char **argv, char **envp) exit(1); } normal_boot_order = g_strdup(boot_order); - boot_order = once; qemu_register_reset(restore_boot_order, normal_boot_order); } @@ -4246,6 +4246,15 @@ int main(int argc, char **argv, char **envp) net_check_clients(); + if (once) { + Error *local_err = NULL; + qemu_boot_set(once, &local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + exit(1); + } + } + Regards, -Gonglei > B. Fix the crappy interface > > Separate parameter validation from the actual action. Only > validation may fail. Validate before starting the guest. > >>> * 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