<[email protected]> writes: > From: Gonglei <[email protected]> > > We can use it for checking when we change traditional > boot order dynamically and propagate error message > to the monitor.
In case you need to respin for some other reason: consider replacing "We can use it" by something like "Will be useful", to make it clearer that we can't change boot order dynamically, yet. > > Signed-off-by: Gonglei <[email protected]> > --- > bootdevice.c | 10 +++++----- > include/sysemu/sysemu.h | 2 +- > vl.c | 15 +++++++++++++-- > 3 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/bootdevice.c b/bootdevice.c > index aae4cac..184348e 100644 > --- a/bootdevice.c > +++ b/bootdevice.c > @@ -55,7 +55,7 @@ int qemu_boot_set(const char *boot_order) > return boot_set_handler(boot_set_opaque, boot_order); > } > > -void validate_bootdevices(const char *devices) > +void validate_bootdevices(const char *devices, Error **errp) > { > /* We just do some generic consistency checks */ > const char *p; > @@ -72,12 +72,12 @@ void validate_bootdevices(const char *devices) > * features. > */ > if (*p < 'a' || *p > 'p') { > - fprintf(stderr, "Invalid boot device '%c'\n", *p); > - exit(1); > + error_setg(errp, "Invalid boot device '%c'", *p); > + return; > } > if (bitmap & (1 << (*p - 'a'))) { > - fprintf(stderr, "Boot device '%c' was given twice\n", *p); > - exit(1); > + error_setg(errp, "Boot device '%c' was given twice", *p); > + return; > } > bitmap |= 1 << (*p - 'a'); > } > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 84798ef..1382d63 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -217,7 +217,7 @@ void device_add_bootindex_property(Object *obj, int32_t > *bootindex, > const char *name, const char *suffix, > DeviceState *dev, Error **errp); > void restore_boot_order(void *opaque); > -void validate_bootdevices(const char *devices); > +void validate_bootdevices(const char *devices, Error **errp); > > /* handler to set the boot_device order for a specific type of QEMUMachine */ > /* return 0 if success */ > diff --git a/vl.c b/vl.c > index f665621..f0cdb63 100644 > --- a/vl.c > +++ b/vl.c > @@ -4087,16 +4087,27 @@ int main(int argc, char **argv, char **envp) > if (opts) { > char *normal_boot_order; > const char *order, *once; > + Error *local_err = NULL; > > order = qemu_opt_get(opts, "order"); > if (order) { > - validate_bootdevices(order); > + validate_bootdevices(order, &local_err); > + if (local_err) { > + error_report("%s", error_get_pretty(local_err)); > + error_free(local_err); > + exit(1); > + } I wouldn't bother freeing stuff right before exit(). But it's not wrong. > boot_order = order; > } > > once = qemu_opt_get(opts, "once"); > if (once) { > - validate_bootdevices(once); > + validate_bootdevices(once, &local_err); > + if (local_err) { > + error_report("%s", error_get_pretty(local_err)); > + error_free(local_err); > + exit(1); > + } > normal_boot_order = g_strdup(boot_order); > boot_order = once; > qemu_register_reset(restore_boot_order, normal_boot_order); Likewise.
