[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 0/3] proper locking on bitmap add/remove paths
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v3 0/3] proper locking on bitmap add/remove paths |
Date: |
Fri, 27 Sep 2019 08:37:30 +0000 |
26.09.2019 22:01, John Snow wrote:
>
>
> On 9/20/19 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> We need to lock qcow2 mutex on accessing in-image metadata, especially
>> on updating this metadata. Let's implement it.
>>
>> v3:
>> 01: add John's r-b
>> 02: - fix bdrv_remove_persistent_dirty_bitmap return value
>> - drop extra zeroing of ret in qcow2_remove_persistent_dirty_bitmap
>> 03: add John's r-b
>>
>> Vladimir Sementsov-Ogievskiy (3):
>> block: move bdrv_can_store_new_dirty_bitmap to block/dirty-bitmap.c
>> block/dirty-bitmap: return int from
>> bdrv_remove_persistent_dirty_bitmap
>> block/qcow2: proper locking on bitmap add/remove paths
>>
>> block/qcow2.h | 14 ++---
>> include/block/block_int.h | 14 ++---
>> include/block/dirty-bitmap.h | 5 +-
>> block.c | 22 -------
>> block/dirty-bitmap.c | 119 +++++++++++++++++++++++++++++++++--
>> block/qcow2-bitmap.c | 36 +++++++----
>> block/qcow2.c | 5 +-
>> blockdev.c | 28 +++------
>> 8 files changed, 163 insertions(+), 80 deletions(-)
>>
>
> I'll take this; I imagine the return signatures are going to change
> again with your error propagation series, though ...?
>
Thanks a lot!
Hmm, I don't think so, as I used to think that returning int for errp-functions
is better anyway..
ret = f(..., errp);
if (ret < 0) {
}
vs
f(..., errp);
if (*errp) {
}
Hmmm... The latter just looks unfamiliar in comparison with "if (ret < 0)".. But
if we anyway going to convert a lot of "if (*local_err)" to "if (*errp)", it
will
become familiar.. And the latter may save 6 characters in a line with function
call,
which may save the whole line in some places.
So I don't know.
returning two errors is not very good, we don't have convention for it actually.
if I have int ret = f(..., errp), what should I report?
error_report_err_errno(ret, errp), or just error_report_err(errp), assuming errp
contains the whole information?
Still, sometimes we need to distinguish one error code from another, and we
can't
check errp for such thing..
--
Best regards,
Vladimir
- [PATCH v3 0/3] proper locking on bitmap add/remove paths, Vladimir Sementsov-Ogievskiy, 2019/09/20
- [PATCH v3 1/3] block: move bdrv_can_store_new_dirty_bitmap to block/dirty-bitmap.c, Vladimir Sementsov-Ogievskiy, 2019/09/20
- [PATCH v3 2/3] block/dirty-bitmap: return int from bdrv_remove_persistent_dirty_bitmap, Vladimir Sementsov-Ogievskiy, 2019/09/20
- [PATCH v3 3/3] block/qcow2: proper locking on bitmap add/remove paths, Vladimir Sementsov-Ogievskiy, 2019/09/20
- Re: [PATCH v3 0/3] proper locking on bitmap add/remove paths, John Snow, 2019/09/26
- Re: [PATCH v3 0/3] proper locking on bitmap add/remove paths,
Vladimir Sementsov-Ogievskiy <=
- Re: [PATCH v3 0/3] proper locking on bitmap add/remove paths, John Snow, 2019/09/26