qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement
@ 2014-12-04 11:19 arei.gonglei
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c arei.gonglei
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: arei.gonglei @ 2014-12-04 11:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Gonglei, weidong.huang, peter.huangpeng, mst

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

Patch 1 just move boot order related code to bootdevice.c.
Patch 2,3,5 add an argument to corresponding functions.
This way, we can propagate the error messages to the caller.
Maybe somebody will say we will remove the legacy boot order
in the future, instead of using bootindex. But at present,
for PPC, the have no way support bootindex, ARM on the flight
(Laszlo Ersek) as far as know.

After this work, we can easily to add QMP command for existing
HMP command 'boot_set' if we have a requirement.

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.12.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c
  2014-12-04 11:19 [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement arei.gonglei
@ 2014-12-04 11:19 ` arei.gonglei
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 2/5] bootdevice: add Error **errp argument for validate_bootdevices() arei.gonglei
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: arei.gonglei @ 2014-12-04 11:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, Jan Kiszka, peter.huangpeng, Gonglei,
	pbonzini

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 eb89d62..cf3b5a2 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);
 
@@ -1182,65 +1179,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.12.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 2/5] bootdevice: add Error **errp argument for validate_bootdevices()
  2014-12-04 11:19 [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement arei.gonglei
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c arei.gonglei
@ 2014-12-04 11:19 ` arei.gonglei
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 3/5] bootdevice: add Error **errp argument for qemu_boot_set() arei.gonglei
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: arei.gonglei @ 2014-12-04 11:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Gonglei, weidong.huang, peter.huangpeng, mst

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 cf3b5a2..fb1c254 100644
--- a/vl.c
+++ b/vl.c
@@ -4034,16 +4034,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.12.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 3/5] bootdevice: add Error **errp argument for qemu_boot_set()
  2014-12-04 11:19 [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement arei.gonglei
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c arei.gonglei
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 2/5] bootdevice: add Error **errp argument for validate_bootdevices() arei.gonglei
@ 2014-12-04 11:19 ` arei.gonglei
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 4/5] bootdevice: add validate check " arei.gonglei
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: arei.gonglei @ 2014-12-04 11:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Gonglei, weidong.huang, peter.huangpeng, mst

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 f1031a1..656359e 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.12.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 4/5] bootdevice: add validate check for qemu_boot_set()
  2014-12-04 11:19 [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement arei.gonglei
                   ` (2 preceding siblings ...)
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 3/5] bootdevice: add Error **errp argument for qemu_boot_set() arei.gonglei
@ 2014-12-04 11:19 ` arei.gonglei
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler arei.gonglei
  2014-12-11  8:29 ` [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement Gonglei
  5 siblings, 0 replies; 7+ messages in thread
From: arei.gonglei @ 2014-12-04 11:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Gonglei, weidong.huang, peter.huangpeng, mst

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.12.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler
  2014-12-04 11:19 [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement arei.gonglei
                   ` (3 preceding siblings ...)
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 4/5] bootdevice: add validate check " arei.gonglei
@ 2014-12-04 11:19 ` arei.gonglei
  2014-12-11  8:29 ` [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement Gonglei
  5 siblings, 0 replies; 7+ messages in thread
From: arei.gonglei @ 2014-12-04 11:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: weidong.huang, mst, Alexander Graf, peter.huangpeng, Blue Swirl,
	Gonglei, qemu-ppc, pbonzini

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.12.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement
  2014-12-04 11:19 [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement arei.gonglei
                   ` (4 preceding siblings ...)
  2014-12-04 11:19 ` [Qemu-devel] [PATCH 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler arei.gonglei
@ 2014-12-11  8:29 ` Gonglei
  5 siblings, 0 replies; 7+ messages in thread
From: Gonglei @ 2014-12-11  8:29 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: pbonzini@redhat.com, mst@redhat.com, Huangweidong (C),
	qemu-devel@nongnu.org, Huangpeng (Peter)

On 2014/12/4 19:19, Gonglei (Arei) wrote:

> From: Gonglei <arei.gonglei@huawei.com>
> 
> Patch 1 just move boot order related code to bootdevice.c.
> Patch 2,3,5 add an argument to corresponding functions.
> This way, we can propagate the error messages to the caller.
> Maybe somebody will say we will remove the legacy boot order
> in the future, instead of using bootindex. But at present,
> for PPC, the have no way support bootindex, ARM on the flight
> (Laszlo Ersek) as far as know.
> 
> After this work, we can easily to add QMP command for existing
> HMP command 'boot_set' if we have a requirement.
> 
> 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(-)
> 

Any comments and/or Acks will be appreciated.
Then I'll send a pull request if no one is against  this :)

Regards,
-Gonglei

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-12-11  8:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-04 11:19 [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement arei.gonglei
2014-12-04 11:19 ` [Qemu-devel] [PATCH 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c arei.gonglei
2014-12-04 11:19 ` [Qemu-devel] [PATCH 2/5] bootdevice: add Error **errp argument for validate_bootdevices() arei.gonglei
2014-12-04 11:19 ` [Qemu-devel] [PATCH 3/5] bootdevice: add Error **errp argument for qemu_boot_set() arei.gonglei
2014-12-04 11:19 ` [Qemu-devel] [PATCH 4/5] bootdevice: add validate check " arei.gonglei
2014-12-04 11:19 ` [Qemu-devel] [PATCH 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler arei.gonglei
2014-12-11  8:29 ` [Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement Gonglei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).