On 2014/12/20 1:17, Markus Armbruster wrote:
> <arei.gong...@huawei.com> writes:
>
>> From: Gonglei <arei.gong...@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.gong...@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
