mbox series

[v3,0/2] Skip copy-on-write when allocating a zero cluster

Message ID cover.1599759873.git.berto@igalia.com
Headers show
Series Skip copy-on-write when allocating a zero cluster | expand

Message

Alberto Garcia Sept. 10, 2020, 5:46 p.m. UTC
Here's the new version, there are two patches this time.

Berto

v3:
- Add a new patch to improve the reporting of BDRV_BLOCK_ZERO [Vladimir]
- Rename function to bdrv_co_is_zero_fast() [Vladimir, Kevin]
- Don't call bdrv_common_block_status_above() if bytes == 0

v2: https://lists.gnu.org/archive/html/qemu-block/2020-08/msg01165.html
- Add new, simpler API: bdrv_is_unallocated_or_zero_above()

v1: https://lists.gnu.org/archive/html/qemu-block/2020-08/msg00403.html

Alberto Garcia (2):
  qcow2: Report BDRV_BLOCK_ZERO more accurately in
    bdrv_co_block_status()
  qcow2: Skip copy-on-write when allocating a zero cluster

 include/block/block.h |  2 ++
 block/io.c            | 35 +++++++++++++++++++++++++++++++----
 block/qcow2.c         | 35 +++++++++++++++++++----------------
 3 files changed, 52 insertions(+), 20 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy Sept. 11, 2020, 11:06 a.m. UTC | #1
11.09.2020 13:04, Alberto Garcia wrote:
> On Fri 11 Sep 2020 11:34:37 AM CEST, Vladimir Sementsov-Ogievskiy wrote:
>>> -        if (!is_zero_cow(bs, m)) {
>>> +        ret = is_zero_cow(bs, m);
>>> +        if (ret < 0) {
>>> +            return ret;
>>
>> It's a common practice to treat block-status errors as "unknown"
>> status and not error-out immediately:
>>
>>    - really, it's not critical, we can continue assuming non-zero
>>    - if there are real problems with IO, we'll most probably fail on
>>    real read or write operation, and report its status, which seems
>>    better for user than block-status error
> 
> But what's the problem exactly, does this complicate the code too much?
> :-?

Of course not :) Hmm. OK, I don't know, I'm just used to this practice in block jobs. Patch is correct as is:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> 
>> So, I'd keep existing logic in handle_alloc_space(). And, if you agree
>> and resend, probably good to split this patch into two, one for
>> block.h/io.c and one for qcow2.c (still, I'm OK with it as one patch).
> 
> Sure, I can split the patch if I have to resend it.
> 
> Berto
>