Re: [Qemu-devel] [PATCH 3/3] vpc: Do not return RAW from block_status

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1796 bytes --]

On 13.08.19 12:38, Kevin Wolf wrote:
> Am 12.08.2019 um 17:56 hat Max Reitz geschrieben:
>> On 12.08.19 17:33, Vladimir Sementsov-Ogievskiy wrote:
>>> 25.07.2019 18:55, Max Reitz wrote:
>>>> vpc is not really a passthrough driver, even when using the fixed
>>>> subformat (where host and guest offsets are equal).  It should handle
>>>> preallocation like all other drivers do, namely by returning
>>>> DATA | RECURSE instead of RAW.
>>>>
>>>> There is no tangible difference but the fact that bdrv_is_allocated() no
>>>> longer falls through to the protocol layer.
>>>
>>> Hmm. Isn't a real bug (fixed by this patch) ?
>>>
>>> Assume vpc->file is qcow2 with backing, which have "unallocated" region, which is
>>> backed by actual data in backing file.
>>
>> Come on now.
>>
>>> So, this region will be reported as not allocated and will be skipped by any copying
>>> loop using block-status? Is it a bug of BDRV_BLOCK_RAW itself? Or I don't understand
>>> something..
>>
>> I think what you don’t understand is that if you have a vpc file inside
>> of a qcow2 file, you’re doing basically everything wrong. ;-)
>>
>> But maybe we should drop BDRV_BLOCK_RAW...  Does it do anything good for
>> us in the raw driver?  Shouldn’t it too just return DATA | RECURSE?
> 
> DATA | RECURSE is still DATA, i.e. marks the block as allocated. If you
> do that unconditionally, we will never consider a block unallocated.

Which is correct for formats that do not have backing files.

> RECURSE doesn't undo this, the only thing it might do is settting ZERO
> additionally.
> 
> So I would say unconditionally returning DATA | RECURSE is almost always
> wrong.

I would say it’s always right when it is a format driver and there is no
backing file.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]