[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH-for-5.1] hw/nvram/fw_cfg: Let fw_cfg_add_from_generator() ret
From: |
Laszlo Ersek |
Subject: |
Re: [PATCH-for-5.1] hw/nvram/fw_cfg: Let fw_cfg_add_from_generator() return boolean value |
Date: |
Thu, 23 Jul 2020 13:01:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 |
+Igor, and question below
On 07/23/20 09:37, Markus Armbruster wrote:
> You must use ERRP_GUARD() in functions that dereference their @errp
> parameter (so that works even when the argument is null) or pass it to
> error_prepend() or error_append_hint() (so they get reached even when
> the argumentis &error_fatal).
>
> You should use Use ERRP_GUARD() to avoid clumsy error propagation.
>
> You should not use ERRP_GUARD() when propagation is not actually
> needed.
Thank you for the explanation. :)
Two patches from a series (work in progress) that I'd like to raise:
- [PATCH 2/6] x86: cphp: prevent guest crash on CPU hotplug when broadcast SMI
is in use
http://mid.mail-archive.com/20200720141610.574308-3-imammedo@redhat.com
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg05852.html
- [PATCH 3/6] x86: cpuhp: refuse cpu hot-unplug request earlier if not supported
http://mid.mail-archive.com/20200720141610.574308-4-imammedo@redhat.com
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg05851.html
Both of these call error_append_hint(errp, ...). I think these functions
are never called against "error_fatal" (they are reached in "device_add"
and "device_del" monitor commands). But just for consistency with the
new rules, should these functions -- ich9_pm_device_pre_plug_cb() and
ich9_pm_device_unplug_request_cb() -- adopt ERRP_GUARD() in those
patches?
(If the answer is "yes", then could you please state that right under
those patches, so the feedback is easier for Igor to collect?
Plus I think commit e3fe3988d78 should be mentioned frequently, because
it's really helpful, and at least I wouldn't have remembered to check
"include/qapi/error.h" for the new rules; mea culpa :/)
Thanks!
Laszlo