[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 5/5] bootdevice: add Error **errp
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler |
Date: |
Fri, 19 Dec 2014 18:24:42 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
<address@hidden> writes:
> From: Gonglei <address@hidden>
>
> We can use it for checking when we change traditional
> boot order dynamically and propagate error message
> to the monitor.
> For x86 architecture, we pass &local_err to set_boot_dev()
> when vm startup in pc_coms_init().
>
> Cc: Michael S. Tsirkin <address@hidden>
> Cc: Alexander Graf <address@hidden>
> Cc: Blue Swirl <address@hidden>
> Cc: address@hidden
> Signed-off-by: Gonglei <address@hidden>
> ---
> bootdevice.c | 5 +----
> hw/i386/pc.c | 23 +++++++++++++----------
> hw/ppc/mac_newworld.c | 4 ++--
> hw/ppc/mac_oldworld.c | 5 ++---
> hw/sparc/sun4m.c | 4 ++--
> hw/sparc64/sun4u.c | 4 ++--
> include/sysemu/sysemu.h | 4 ++--
> 7 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/bootdevice.c b/bootdevice.c
> index 9de34ba..5914417 100644
> --- a/bootdevice.c
> +++ b/bootdevice.c
> @@ -63,10 +63,7 @@ void qemu_boot_set(const char *boot_order, Error **errp)
> return;
> }
>
> - if (boot_set_handler(boot_set_opaque, boot_order)) {
> - error_setg(errp, "setting boot device list failed");
> - return;
> - }
> + boot_set_handler(boot_set_opaque, boot_order, errp);
> }
>
> void validate_bootdevices(const char *devices, Error **errp)
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index c0e55a6..6b7fdea 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -282,7 +282,7 @@ static int boot_device2nibble(char boot_device)
> return 0;
> }
>
> -static int set_boot_dev(ISADevice *s, const char *boot_device)
> +static void set_boot_dev(ISADevice *s, const char *boot_device, Error **errp)
> {
> #define PC_MAX_BOOT_DEVICES 3
> int nbds, bds[3] = { 0, };
> @@ -290,25 +290,24 @@ static int set_boot_dev(ISADevice *s, const char
> *boot_device)
>
> nbds = strlen(boot_device);
> if (nbds > PC_MAX_BOOT_DEVICES) {
> - error_report("Too many boot devices for PC");
> - return(1);
> + error_setg(errp, "Too many boot devices for PC");
> + return;
> }
> for (i = 0; i < nbds; i++) {
> bds[i] = boot_device2nibble(boot_device[i]);
> if (bds[i] == 0) {
> - error_report("Invalid boot device for PC: '%c'",
> - boot_device[i]);
> - return(1);
> + error_setg(errp, "Invalid boot device for PC: '%c'",
> + boot_device[i]);
> + return;
> }
> }
> rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
> rtc_set_memory(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 : 0x1));
> - return(0);
> }
>
> -static int pc_boot_set(void *opaque, const char *boot_device)
> +static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
> {
> - return set_boot_dev(opaque, boot_device);
> + set_boot_dev(opaque, boot_device, errp);
> }
>
> typedef struct pc_cmos_init_late_arg {
> @@ -365,6 +364,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
> above_4g_mem_size,
> FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
> static pc_cmos_init_late_arg arg;
> PCMachineState *pc_machine = PC_MACHINE(machine);
> + Error *local_err = NULL;
>
> /* various important CMOS locations needed by PC/Bochs bios */
>
> @@ -412,7 +412,10 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
> above_4g_mem_size,
> object_property_set_link(OBJECT(machine), OBJECT(s),
> "rtc_state", &error_abort);
>
> - if (set_boot_dev(s, boot_device)) {
> + set_boot_dev(s, boot_device, &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.
[...]