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

[Max Reitz]

On 24.10.19 16:07, Andrey Shinkevich wrote:
> 
> 
> 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?

I don’t know, Vladimir said that use case wouldn’t be needed.

I think if the option was there, people would actually use it.  But I
doubt that anyone misses it sufficiently to warrant the effort.

In addition, there’s still VMDK to consider, too.

Max

signature.asc
Description: OpenPGP digital signature