[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 12/13] block/qcow2: simplify qcow2_co_invalidate_cache()
From: |
Alberto Garcia |
Subject: |
Re: [PATCH v2 12/13] block/qcow2: simplify qcow2_co_invalidate_cache() |
Date: |
Fri, 18 Sep 2020 17:51:30 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Fri 18 Sep 2020 05:30:06 PM CEST, Greg Kurz wrote:
>> qcow2_do_open correctly sets errp on each failure path. So, we can
>> simplify code in qcow2_co_invalidate_cache() and drop explicit error
>> propagation. We should use ERRP_GUARD() (accordingly to comment in
>> include/qapi/error.h) together with error_append() call which we add to
>> avoid problems with error_fatal.
>>
>
> The wording gives the impression that we add error_append() to avoid problems
> with error_fatal which is certainly not true. Also it isn't _append() but
> _prepend() :)
>
> What about ?
>
> "Add ERRP_GUARD() as mandated by the documentation in include/qapi/error.h
> to avoid problems with the error_prepend() call if errp is
> &error_fatal."
I had to go to the individual error functions to see what "it doesn't
work with &error_fatal" actually means.
So in a case like qcow2_do_open() which has:
error_setg(errp, ...)
error_append_hint(errp, ...)
As far as I can see this works just fine without ERRP_GUARD() and with
error_fatal, the difference is that if we don't use the guard then the
process exists during error_setg(), and if we use the guard it exists
during the implicit error_propagate() call triggered by its destruction
at the end of the function. In this latter case the printed error
message would include the hint.
Berto
- [PATCH v2 05/13] block: drop extra error propagation for bdrv_set_backing_hd, (continued)
- [PATCH v2 05/13] block: drop extra error propagation for bdrv_set_backing_hd, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 03/13] block: check return value of bdrv_open_child and drop error propagation, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 10/13] block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 06/13] block/mirror: drop extra error propagation in commit_active_start(), Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 07/13] blockjob: return status from block_job_set_speed(), Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 11/13] block/qcow2: read_cache_sizes: return status value, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 12/13] block/qcow2: simplify qcow2_co_invalidate_cache(), Vladimir Sementsov-Ogievskiy, 2020/09/17
[PATCH v2 08/13] block/qcow2: qcow2_get_specific_info(): drop error propagation, Vladimir Sementsov-Ogievskiy, 2020/09/17
[PATCH v2 09/13] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface, Vladimir Sementsov-Ogievskiy, 2020/09/17
[PATCH v2 13/13] block/qed: bdrv_qed_do_open: deal with errp, Vladimir Sementsov-Ogievskiy, 2020/09/17