Message ID | 1418813672-14768-3-git-send-email-arei.gonglei@huawei.com |
---|---|
State | New |
Headers | show |
<arei.gonglei@huawei.com> writes: > From: Gonglei <arei.gonglei@huawei.com> > > 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 <arei.gonglei@huawei.com> > --- > 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.
On 2014/12/20 1:17, Markus Armbruster wrote: > <arei.gonglei@huawei.com> writes: > >> From: Gonglei <arei.gonglei@huawei.com> >> >> 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. > Actually, we had HMP command to change boot order dynamically at present. >> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> --- >> 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. > Ok, I can remove error_free(local_err) in spite of some other places also doing it like this in vl.c. :) >> 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. OK. Regards, -Gonglei
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); + } 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);