All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gonglei <arei.gonglei@huawei.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Blue Swirl <blauwirbel@gmail.com>, Alexander Graf <agraf@suse.de>,
	"qemu-ppc@nongnu.org" <qemu-ppc@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler
Date: Tue, 16 Dec 2014 21:04:10 +0800	[thread overview]
Message-ID: <54902DCA.7040201@huawei.com> (raw)
In-Reply-To: <CAFEAcA-17Upc1BQXqNVPToxaJyYS4H5krQE73xD+Moig9xU=XQ@mail.gmail.com>

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

  reply	other threads:[~2014-12-16 13:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-16  9:22 [Qemu-devel] [PULL 0/5] bootdevice patches arei.gonglei
2014-12-16  9:22 ` [Qemu-devel] [PULL 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c arei.gonglei
2014-12-16  9:22 ` [Qemu-devel] [PULL 2/5] bootdevice: add Error **errp argument for validate_bootdevices() arei.gonglei
2014-12-16  9:22 ` [Qemu-devel] [PULL 3/5] bootdevice: add Error **errp argument for qemu_boot_set() arei.gonglei
2014-12-16  9:22 ` [Qemu-devel] [PULL 4/5] bootdevice: add validate check " arei.gonglei
2014-12-16  9:22 ` [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler arei.gonglei
2014-12-16 12:42   ` Peter Maydell
2014-12-16 13:04     ` Gonglei [this message]
2014-12-16 13:23       ` Peter Maydell
2014-12-17  3:16         ` Gonglei
2014-12-16 14:01 ` [Qemu-devel] [PULL 0/5] bootdevice patches Peter Maydell
2014-12-17  3:27   ` Gonglei
  -- strict thread matches above, loose matches on Subject: below --
2014-12-16 10:12 arei.gonglei
2014-12-16 10:12 ` [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler arei.gonglei
2014-12-22  8:56 [Qemu-devel] [PULL 0/5] bootdevice: Refactor and improvement arei.gonglei
2014-12-22  8:56 ` [Qemu-devel] [PULL 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler arei.gonglei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54902DCA.7040201@huawei.com \
    --to=arei.gonglei@huawei.com \
    --cc=agraf@suse.de \
    --cc=blauwirbel@gmail.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.