[Qemu-devel] [PULL 0/5] bootdevice patches

[Qemu-devel] [PULL 0/5] bootdevice patches

[arei.gonglei]

From: root <root@ceth6.(none)>

This is my first pull request as a submaintainer. Those patches just
move boot order related code to bootdevice.c and add a Error **errp
argument for corresponding functions so that it can propagate error messages
to the caller. Please pull.

Regards,
-Gonglei

The following changes since commit 54600752a1dd67844c2cf3c467db562c39499838:

  Merge remote-tracking branch 'remotes/rth/tags/x86-next-20141214' into staging (2014-12-15 11:11:52 +0000)

are available in the git repository at:


  https://github.com/gongleiarei/qemu.git tags/bootdevice-next-20141216

for you to fetch changes up to 8bcee1828dd00c03cea03a4fa53e48b58dca49af:

  bootdevice: add Error **errp argument for QEMUBootSetHandler (2014-12-16 16:52:39 +0800)

----------------------------------------------------------------
bootdevice: refactor and improvement

----------------------------------------------------------------

Gonglei (5):
  bootdevice: move code about bootorder from vl.c to bootdevice.c
  bootdevice: add Error **errp argument for validate_bootdevices()
  bootdevice: add Error **errp argument for qemu_boot_set()
  bootdevice: add validate check for qemu_boot_set()
  bootdevice: add Error **errp argument for QEMUBootSetHandler

 bootdevice.c            |   73 ++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/pc.c            |   21 ++++++-------
 hw/ppc/mac_newworld.c   |    4 +--
 hw/ppc/mac_oldworld.c   |    5 ++-
 hw/sparc/sun4m.c        |    4 +--
 hw/sparc64/sun4u.c      |    4 +--
 include/hw/hw.h         |    6 ----
 include/sysemu/sysemu.h |    7 +++++
 monitor.c               |   14 ++++-----
 vl.c                    |   77 ++++++++---------------------------------------
 10 files changed, 116 insertions(+), 99 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PULL 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

First, we can downsize vl.c, make it simpler by
little and little. Second, I can maintain those code
and make some improvement.

Cc: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 bootdevice.c            |   62 +++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/hw.h         |    6 -----
 include/sysemu/sysemu.h |    8 ++++++
 vl.c                    |   62 -----------------------------------------------
 4 files changed, 70 insertions(+), 68 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index b29970c..aae4cac 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -25,6 +25,7 @@
 #include "sysemu/sysemu.h"
 #include "qapi/visitor.h"
 #include "qemu/error-report.h"
+#include "hw/hw.h"
 
 typedef struct FWBootEntry FWBootEntry;
 
@@ -37,6 +38,67 @@ struct FWBootEntry {
 
 static QTAILQ_HEAD(, FWBootEntry) fw_boot_order =
     QTAILQ_HEAD_INITIALIZER(fw_boot_order);
+static QEMUBootSetHandler *boot_set_handler;
+static void *boot_set_opaque;
+
+void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
+{
+    boot_set_handler = func;
+    boot_set_opaque = opaque;
+}
+
+int qemu_boot_set(const char *boot_order)
+{
+    if (!boot_set_handler) {
+        return -EINVAL;
+    }
+    return boot_set_handler(boot_set_opaque, boot_order);
+}
+
+void validate_bootdevices(const char *devices)
+{
+    /* We just do some generic consistency checks */
+    const char *p;
+    int bitmap = 0;
+
+    for (p = devices; *p != '\0'; p++) {
+        /* Allowed boot devices are:
+         * a-b: floppy disk drives
+         * c-f: IDE disk drives
+         * g-m: machine implementation dependent drives
+         * n-p: network devices
+         * It's up to each machine implementation to check if the given boot
+         * devices match the actual hardware implementation and firmware
+         * features.
+         */
+        if (*p < 'a' || *p > 'p') {
+            fprintf(stderr, "Invalid boot device '%c'\n", *p);
+            exit(1);
+        }
+        if (bitmap & (1 << (*p - 'a'))) {
+            fprintf(stderr, "Boot device '%c' was given twice\n", *p);
+            exit(1);
+        }
+        bitmap |= 1 << (*p - 'a');
+    }
+}
+
+void restore_boot_order(void *opaque)
+{
+    char *normal_boot_order = opaque;
+    static int first = 1;
+
+    /* Restore boot order and remove ourselves after the first boot */
+    if (first) {
+        first = 0;
+        return;
+    }
+
+    qemu_boot_set(normal_boot_order);
+
+    qemu_unregister_reset(restore_boot_order, normal_boot_order);
+    g_free(normal_boot_order);
+}
 
 void check_boot_index(int32_t bootindex, Error **errp)
 {
diff --git a/include/hw/hw.h b/include/hw/hw.h
index 33bdb92..c78adae 100644
--- a/include/hw/hw.h
+++ b/include/hw/hw.h
@@ -41,12 +41,6 @@ typedef void QEMUResetHandler(void *opaque);
 void qemu_register_reset(QEMUResetHandler *func, void *opaque);
 void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
 
-/* handler to set the boot_device order for a specific type of QEMUMachine */
-/* return 0 if success */
-typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
-void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
-int qemu_boot_set(const char *boot_order);
-
 #ifdef NEED_CPU_H
 #if TARGET_LONG_BITS == 64
 #define VMSTATE_UINTTL_V(_f, _s, _v)                                  \
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 9fea3bc..84798ef 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -216,6 +216,14 @@ void del_boot_device_path(DeviceState *dev, const char *suffix);
 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);
+
+/* handler to set the boot_device order for a specific type of QEMUMachine */
+/* return 0 if success */
+typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
+void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
+int qemu_boot_set(const char *boot_order);
 
 QemuOpts *qemu_get_machine_opts(void);
 
diff --git a/vl.c b/vl.c
index 113e98e..f665621 100644
--- a/vl.c
+++ b/vl.c
@@ -196,9 +196,6 @@ NodeInfo numa_info[MAX_NODES];
 uint8_t qemu_uuid[16];
 bool qemu_uuid_set;
 
-static QEMUBootSetHandler *boot_set_handler;
-static void *boot_set_opaque;
-
 static NotifierList exit_notifiers =
     NOTIFIER_LIST_INITIALIZER(exit_notifiers);
 
@@ -1198,65 +1195,6 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type,
 
 }
 
-void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
-{
-    boot_set_handler = func;
-    boot_set_opaque = opaque;
-}
-
-int qemu_boot_set(const char *boot_order)
-{
-    if (!boot_set_handler) {
-        return -EINVAL;
-    }
-    return boot_set_handler(boot_set_opaque, boot_order);
-}
-
-static void validate_bootdevices(const char *devices)
-{
-    /* We just do some generic consistency checks */
-    const char *p;
-    int bitmap = 0;
-
-    for (p = devices; *p != '\0'; p++) {
-        /* Allowed boot devices are:
-         * a-b: floppy disk drives
-         * c-f: IDE disk drives
-         * g-m: machine implementation dependent drives
-         * n-p: network devices
-         * It's up to each machine implementation to check if the given boot
-         * devices match the actual hardware implementation and firmware
-         * features.
-         */
-        if (*p < 'a' || *p > 'p') {
-            fprintf(stderr, "Invalid boot device '%c'\n", *p);
-            exit(1);
-        }
-        if (bitmap & (1 << (*p - 'a'))) {
-            fprintf(stderr, "Boot device '%c' was given twice\n", *p);
-            exit(1);
-        }
-        bitmap |= 1 << (*p - 'a');
-    }
-}
-
-static void restore_boot_order(void *opaque)
-{
-    char *normal_boot_order = opaque;
-    static int first = 1;
-
-    /* Restore boot order and remove ourselves after the first boot */
-    if (first) {
-        first = 0;
-        return;
-    }
-
-    qemu_boot_set(normal_boot_order);
-
-    qemu_unregister_reset(restore_boot_order, normal_boot_order);
-    g_free(normal_boot_order);
-}
-
 static QemuOptsList qemu_smp_opts = {
     .name = "smp-opts",
     .implied_opt_name = "cpus",
-- 
1.7.9.5

[Qemu-devel] [PULL 2/5] bootdevice: add Error **errp argument for validate_bootdevices()

[arei.gonglei]

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.

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..09dac74 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(order, &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);
-- 
1.7.9.5

[Qemu-devel] [PULL 3/5] bootdevice: add Error **errp argument for qemu_boot_set()

[arei.gonglei]

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.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 bootdevice.c            |   14 ++++++++++----
 include/sysemu/sysemu.h |    2 +-
 monitor.c               |   14 ++++++--------
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index 184348e..7f07507 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -47,12 +47,18 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
     boot_set_opaque = opaque;
 }
 
-int qemu_boot_set(const char *boot_order)
+void qemu_boot_set(const char *boot_order, Error **errp)
 {
     if (!boot_set_handler) {
-        return -EINVAL;
+        error_setg(errp, "no function defined to set boot device list for"
+                         " this architecture");
+        return;
+    }
+
+    if (boot_set_handler(boot_set_opaque, boot_order)) {
+        error_setg(errp, "setting boot device list failed");
+        return;
     }
-    return boot_set_handler(boot_set_opaque, boot_order);
 }
 
 void validate_bootdevices(const char *devices, Error **errp)
@@ -94,7 +100,7 @@ void restore_boot_order(void *opaque)
         return;
     }
 
-    qemu_boot_set(normal_boot_order);
+    qemu_boot_set(normal_boot_order, NULL);
 
     qemu_unregister_reset(restore_boot_order, normal_boot_order);
     g_free(normal_boot_order);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 1382d63..ee7fee7 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -223,7 +223,7 @@ void validate_bootdevices(const char *devices, Error **errp);
 /* return 0 if success */
 typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
-int qemu_boot_set(const char *boot_order);
+void qemu_boot_set(const char *boot_order, Error **errp);
 
 QemuOpts *qemu_get_machine_opts(void);
 
diff --git a/monitor.c b/monitor.c
index b37ddda..62e21c5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1489,17 +1489,15 @@ static void do_ioport_write(Monitor *mon, const QDict *qdict)
 
 static void do_boot_set(Monitor *mon, const QDict *qdict)
 {
-    int res;
+    Error *local_err = NULL;
     const char *bootdevice = qdict_get_str(qdict, "bootdevice");
 
-    res = qemu_boot_set(bootdevice);
-    if (res == 0) {
-        monitor_printf(mon, "boot device list now set to %s\n", bootdevice);
-    } else if (res > 0) {
-        monitor_printf(mon, "setting boot device list failed\n");
+    qemu_boot_set(bootdevice, &local_err);
+    if (local_err) {
+        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
+        error_free(local_err);
     } else {
-        monitor_printf(mon, "no function defined to set boot device list for "
-                       "this architecture\n");
+        monitor_printf(mon, "boot device list now set to %s\n", bootdevice);
     }
 }
 
-- 
1.7.9.5

[Qemu-devel] [PULL 4/5] bootdevice: add validate check for qemu_boot_set()

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 bootdevice.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/bootdevice.c b/bootdevice.c
index 7f07507..9de34ba 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -49,12 +49,20 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
 
 void qemu_boot_set(const char *boot_order, Error **errp)
 {
+    Error *local_err = NULL;
+
     if (!boot_set_handler) {
         error_setg(errp, "no function defined to set boot device list for"
                          " this architecture");
         return;
     }
 
+    validate_bootdevices(boot_order, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
     if (boot_set_handler(boot_set_opaque, boot_order)) {
         error_setg(errp, "setting boot device list failed");
         return;
-- 
1.7.9.5

[Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

[arei.gonglei]

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.
For x86 architecture, we pass &error_abort to set_boot_dev()
when vm startup in pc_coms_init().

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 bootdevice.c            |    5 +----
 hw/i386/pc.c            |   21 +++++++++------------
 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 |    3 +--
 7 files changed, 19 insertions(+), 27 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 f31d55e..99deba6 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 {
@@ -412,9 +411,7 @@ 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)) {
-        exit(1);
-    }
+    set_boot_dev(s, boot_device, &error_abort);
 
     /* floppy type */
     if (floppy) {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 89aee71..ee1ed8a 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -116,10 +116,10 @@ static const MemoryRegionOps unin_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 32c21a4..15109c2 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -49,13 +49,12 @@
 #define CLOCKFREQ 266000000UL
 #define BUSFREQ 66000000UL
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
-
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
 {
     return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 8273199..df259ad 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -121,10 +121,10 @@ void DMA_register_channel (int nchan,
 {
 }
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static void nvram_init(M48t59State *nvram, uint8_t *macaddr,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index f42112c..acac8f9 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -124,10 +124,10 @@ void DMA_register_channel (int nchan,
 {
 }
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static int sun4u_NVRAM_set_params(M48t59State *nvram, uint16_t NVRAM_size,
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ee7fee7..3767073 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -220,8 +220,7 @@ void restore_boot_order(void *opaque);
 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 */
-typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
+typedef void QEMUBootSetHandler(void *opaque, const char *boot_order, Error **errp);
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
 void qemu_boot_set(const char *boot_order, Error **errp);
 
-- 
1.7.9.5

[Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

[arei.gonglei]

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.
For x86 architecture, we pass &error_abort to set_boot_dev()
when vm startup in pc_coms_init().

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 bootdevice.c            |    5 +----
 hw/i386/pc.c            |   21 +++++++++------------
 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 |    3 +--
 7 files changed, 19 insertions(+), 27 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 f31d55e..99deba6 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 {
@@ -412,9 +411,7 @@ 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)) {
-        exit(1);
-    }
+    set_boot_dev(s, boot_device, &error_abort);
 
     /* floppy type */
     if (floppy) {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 89aee71..ee1ed8a 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -116,10 +116,10 @@ static const MemoryRegionOps unin_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 32c21a4..15109c2 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -49,13 +49,12 @@
 #define CLOCKFREQ 266000000UL
 #define BUSFREQ 66000000UL
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
-
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
 {
     return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 8273199..df259ad 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -121,10 +121,10 @@ void DMA_register_channel (int nchan,
 {
 }
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static void nvram_init(M48t59State *nvram, uint8_t *macaddr,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index f42112c..acac8f9 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -124,10 +124,10 @@ void DMA_register_channel (int nchan,
 {
 }
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static int sun4u_NVRAM_set_params(M48t59State *nvram, uint16_t NVRAM_size,
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ee7fee7..3767073 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -220,8 +220,7 @@ void restore_boot_order(void *opaque);
 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 */
-typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
+typedef void QEMUBootSetHandler(void *opaque, const char *boot_order, Error **errp);
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
 void qemu_boot_set(const char *boot_order, Error **errp);
 
-- 
1.7.9.5

Re: [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

[Peter Maydell]

On 16 December 2014 at 09:22,  <arei.gonglei@huawei.com> wrote:
> @@ -412,9 +411,7 @@ 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)) {
> -        exit(1);
> -    }
> +    set_boot_dev(s, boot_device, &error_abort);

This turns a "print error message and exit" path into
an abort(), which doesn't seem right (this can be triggered
by bad user input arguments, yes?). error_abort should
only be used in cases where you would assert() if there
was an error (ie where it would be a QEMU bug if it
happened).

thanks
-- PMM

Re: [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

[Gonglei]

On 2014/12/16 20:42, Peter Maydell wrote:

> On 16 December 2014 at 09:22,  <arei.gonglei@huawei.com> wrote:
>> @@ -412,9 +411,7 @@ 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)) {
>> -        exit(1);
>> -    }
>> +    set_boot_dev(s, boot_device, &error_abort);
> 
> This turns a "print error message and exit" path into
> an abort(), which doesn't seem right (this can be triggered
> by bad user input arguments, yes?). error_abort should
> only be used in cases where you would assert() if there
> was an error (ie where it would be a QEMU bug if it
> happened).
> 

Yes, agree. How does use a incremental patch fix this, Peter?

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 99deba6..d7822b8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -364,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 */

@@ -411,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);

-    set_boot_dev(s, boot_device, &error_abort);
+    set_boot_dev(s, boot_device, &local_err);
+    if (local_err) {
+        exit(1);
+    }

     /* floppy type */
     if (floppy) {

Regards,
-Gonglei

Re: [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

[Peter Maydell]

On 16 December 2014 at 13:04, Gonglei <arei.gonglei@huawei.com> wrote:
> On 2014/12/16 20:42, Peter Maydell wrote:
>
>> On 16 December 2014 at 09:22,  <arei.gonglei@huawei.com> wrote:
>>> @@ -412,9 +411,7 @@ 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)) {
>>> -        exit(1);
>>> -    }
>>> +    set_boot_dev(s, boot_device, &error_abort);
>>
>> This turns a "print error message and exit" path into
>> an abort(), which doesn't seem right (this can be triggered
>> by bad user input arguments, yes?). error_abort should
>> only be used in cases where you would assert() if there
>> was an error (ie where it would be a QEMU bug if it
>> happened).
>>
>
> Yes, agree. How does use a incremental patch fix this, Peter?
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 99deba6..d7822b8 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -364,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 */
>
> @@ -411,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);
>
> -    set_boot_dev(s, boot_device, &error_abort);
> +    set_boot_dev(s, boot_device, &local_err);
> +    if (local_err) {
> +        exit(1);
> +    }

That won't print the error message at all...

-- PMM

Re: [Qemu-devel] [PULL 0/5] bootdevice patches

[Peter Maydell]

On 16 December 2014 at 09:22,  <arei.gonglei@huawei.com> wrote:
> From: root <root@ceth6.(none)>
>
> This is my first pull request as a submaintainer. Those patches just
> move boot order related code to bootdevice.c and add a Error **errp
> argument for corresponding functions so that it can propagate error messages
> to the caller. Please pull.

This also seems to cause 'make check' to fail:

TEST: tests/boot-order-test... (pid=16958)
  /i386/boot-order/pc:
Broken pipe
FAIL
GTester: last random seed: R02S5702c094d31af53e45fffabed844705a
(pid=16973)
FAIL: tests/boot-order-test

thanks
-- PMM

Re: [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

[Gonglei]

On 2014/12/16 21:23, Peter Maydell wrote:

> On 16 December 2014 at 13:04, Gonglei <arei.gonglei@huawei.com> wrote:
>> On 2014/12/16 20:42, Peter Maydell wrote:
>>
>>> On 16 December 2014 at 09:22,  <arei.gonglei@huawei.com> wrote:
>>>> @@ -412,9 +411,7 @@ 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)) {
>>>> -        exit(1);
>>>> -    }
>>>> +    set_boot_dev(s, boot_device, &error_abort);
>>>
>>> This turns a "print error message and exit" path into
>>> an abort(), which doesn't seem right (this can be triggered
>>> by bad user input arguments, yes?). error_abort should
>>> only be used in cases where you would assert() if there
>>> was an error (ie where it would be a QEMU bug if it
>>> happened).
>>>
>>
>> Yes, agree. How does use a incremental patch fix this, Peter?
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 99deba6..d7822b8 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -364,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 */
>>
>> @@ -411,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);
>>
>> -    set_boot_dev(s, boot_device, &error_abort);
>> +    set_boot_dev(s, boot_device, &local_err);
>> +    if (local_err) {
>> +        exit(1);
>> +    }
> 
> That won't print the error message at all...
> 
Yes, I see. Thanks. I will send a new pull request :)


Regards,
-Gonglei

Re: [Qemu-devel] [PULL 0/5] bootdevice patches

[Gonglei]

On 2014/12/16 22:01, Peter Maydell wrote:

> On 16 December 2014 at 09:22,  <arei.gonglei@huawei.com> wrote:
>> From: root <root@ceth6.(none)>
>>
>> This is my first pull request as a submaintainer. Those patches just
>> move boot order related code to bootdevice.c and add a Error **errp
>> argument for corresponding functions so that it can propagate error messages
>> to the caller. Please pull.
> 
> This also seems to cause 'make check' to fail:
> 
> TEST: tests/boot-order-test... (pid=16958)
>   /i386/boot-order/pc:
> Broken pipe
> FAIL
> GTester: last random seed: R02S5702c094d31af53e45fffabed844705a
> (pid=16973)
> FAIL: tests/boot-order-test
> 

Oops, That's because of a typo in patch 2,  'once' -> 'order'.
 'make check' should be executed before posting patches :(
I apologize for any inconvenience caused during this process,
and great thanks for your precise job.

Regards,
-Gonglei

[Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

It will be useful 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 <mst@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 bootdevice.c            |    5 +----
 hw/i386/pc.c            |   22 ++++++++++++----------
 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, 23 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..1ec7290 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,9 @@ 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));
         exit(1);
     }
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 89aee71..ee1ed8a 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -116,10 +116,10 @@ static const MemoryRegionOps unin_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 32c21a4..15109c2 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -49,13 +49,12 @@
 #define CLOCKFREQ 266000000UL
 #define BUSFREQ 66000000UL
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
-
 static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
 {
     return (addr & 0x0fffffff) + KERNEL_LOAD_ADDR;
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 8273199..df259ad 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -121,10 +121,10 @@ void DMA_register_channel (int nchan,
 {
 }
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static void nvram_init(M48t59State *nvram, uint8_t *macaddr,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index f42112c..acac8f9 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -124,10 +124,10 @@ void DMA_register_channel (int nchan,
 {
 }
 
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+                            Error **errp)
 {
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-    return 0;
 }
 
 static int sun4u_NVRAM_set_params(M48t59State *nvram, uint16_t NVRAM_size,
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ee7fee7..503e5a4 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -220,8 +220,8 @@ void restore_boot_order(void *opaque);
 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 */
-typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
+typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
+                                Error **errp);
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
 void qemu_boot_set(const char *boot_order, Error **errp);
 
-- 
1.7.9.5