[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 3/9] block: introduce BDRV_REQ_ALLOCATE flag
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v7 3/9] block: introduce BDRV_REQ_ALLOCATE flag |
Date: |
Thu, 1 Feb 2018 19:06:41 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 2018-02-01 14:34, Anton Nefedov wrote:
> On 31/1/2018 8:31 PM, Max Reitz wrote:
>> On 2018-01-30 13:34, Anton Nefedov wrote:
>>> Offtop: does REQ_ZERO_WRITE override REQ_WRITE_COMPRESSED in this
>>> function? at least with !REQ_MAY_UNMAP it looks wrong
>>
>> Looks like zero detection will indeed override compression. I think
>> that was intended, but I don't even have an opinion either way.
>>
>> Of course, it wouldn't be so nice if you tried to compress something and
>> then, because the zero write failed, you actually write uncompressed
>> zeroes... But since zero detection is an optional feature, it might be
>> your own fault if you enable it when you want compression anyway, and if
>> you write to some format/protocol combination that doesn't allow zero
>> writes.
>>
>> Max
>>
>
> Is it detection only? I guess in case the caller sets
> (REQ_ZERO_WRITE | REQ_COMPRESSED) it also fires here.
True, but that's the caller's "fault". One of those things has to take
precedence.
And I've just noticed that when bdrv_co_do_pwrite_zeroes() falls back to
a bounce buffer, it passes the original flags (without
BDRV_REQ_ZERO_WRITE) to bdrv_driver_pwritev(), so compression will take
effect then. So the only result is that we first try a zero write, and
if that fails, we do a compressed write. That sounds reasonable to me.
(I mean, we could do it the other way around, but I firstly I don't
think it matters much and secondly it's probably better this way because
I'd suspect zero writes to be more efficient than compressing the bounce
buffer; at least that's how it is for qcow2.)
Max
> Looks like noone does that though, but for example backup might;
> it supports compression, and it also does a zero detection itself and
> calls write_zeroes() when it can.
> It sets REQ_MAY_UNMAP which should indeed override a compression, but
> unmap might be not supported.
>
> /Anton
signature.asc
Description: OpenPGP digital signature