Re: [PATCH v5 1/4] block: support compressed write at generic layer

[Andrey Shinkevich]

On 24/10/2019 16:48, Max Reitz wrote:
> On 24.10.19 14:56, Andrey Shinkevich wrote:
>>
>>
>> On 24/10/2019 12:34, Max Reitz wrote:
>>> On 22.10.19 15:53, Andrey Shinkevich wrote:
>>>
>>> [...]
>>>
>>>> If the support of COW for compressed writes is found feasible, will it
>>>> make a sense to implement? Then this series will follow.
>>>
>>> Hm, what exactly do you mean by support of COW for compressed writes?
>>>
>>
>> I spoke in terms of the commit message with the following ID:
>>
>> b0b6862e5e1a1394e0ab3d5da94ba8b0da8664e2
>>
>> "qcow2: Fail write_compressed when overwriting data"
>>
>> "...qcow2_write_compressed() doesn't perform COW as it would have to do..."
>>
>> So, I suggest that we implement writing compressed data to the allocated
>> clusters rather than qcow2_alloc_compressed_cluster_offset() returns the
>> error. Particularly, when it comes to NBD server connection failure for
>> writhing a compressed cluster, it may not be rewritten after the
>> connection is restored.
>> Are there any issues with that implementation idea?
> 
> Well, the COW in that commit is meant differently, because it refers to
> the COW that’s required when writing to a cluster shared by an internal
> snapshot.
> 
> OTOH, you could say that all compressed writes to a cluster that is
> already allocated would need to do COW because we’d always have to fully
> rewrite that cluster in an RMW cycle.
> 
> I don’t see how letting qcow2_alloc_compressed_cluster_offset() use the
> existing cluster would solve the problem, though.  You’d generally need
> to allocate a new cluster; or attempt to reuse the existing space in a
> compressed cluster.
> 
> Max
> 

Yes, new clusters would be allocated for the compressed RMW and the 
existing ones would be reused if possible. It seams to be ineffective 
but users are supposed to know what they do.
So, does it worth to check a feasibility of the implementation?

Andrey