From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51750) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y29Bs-0002N7-VI for qemu-devel@nongnu.org; Fri, 19 Dec 2014 20:50:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y29Bm-0007Ag-0q for qemu-devel@nongnu.org; Fri, 19 Dec 2014 20:50:44 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:51538) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y29Bl-0007A2-Cy for qemu-devel@nongnu.org; Fri, 19 Dec 2014 20:50:37 -0500 Message-ID: <5494D5DA.5050304@huawei.com> Date: Sat, 20 Dec 2014 09:50:18 +0800 From: Gonglei MIME-Version: 1.0 References: <1418813672-14768-1-git-send-email-arei.gonglei@huawei.com> <1418813672-14768-3-git-send-email-arei.gonglei@huawei.com> <878ui3y2ws.fsf@blackfin.pond.sub.org> In-Reply-To: <878ui3y2ws.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset="GB2312" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/5] bootdevice: add Error **errp argument for validate_bootdevices() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: "peter.maydell@linaro.org" , "Huangpeng (Peter)" , "pbonzini@redhat.com" , "qemu-devel@nongnu.org" , "mst@redhat.com" On 2014/12/20 1:17, Markus Armbruster wrote: > writes: > >> From: Gonglei >> >> 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 >> --- >> 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