[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/4] fix & merge block_status_above and is_allocated_above
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH 0/4] fix & merge block_status_above and is_allocated_above |
Date: |
Tue, 19 Nov 2019 12:30:55 +0000 |
19.11.2019 15:20, Max Reitz wrote:
> On 19.11.19 13:02, Denis V. Lunev wrote:
>> On 11/19/19 1:22 PM, Max Reitz wrote:
>>> On 16.11.19 17:34, Vladimir Sementsov-Ogievskiy wrote:
>>>> Hi all!
>>>>
>>>> I wanted to understand, what is the real difference between
>>>> bdrv_block_status_above
>>>> and bdrv_is_allocated_above, IMHO bdrv_is_allocated_above should work
>>>> through
>>>> bdrv_block_status_above..
>>>>
>>>> And I found the problem: bdrv_is_allocated_above considers space after EOF
>>>> as
>>>> UNALLOCATED for intermediate nodes..
>>>>
>>>> UNALLOCATED is not about allocation at fs level, but about should we go to
>>>> backing or
>>>> not.. And it seems incorrect for me, as in case of short backing file,
>>>> we'll read
>>>> zeroes after EOF, instead of going further by backing chain.
>>> Should we, though? It absolutely makes sense to me to consider post-EOF
>>> space as unallocated because, well, it is as unallocated as it gets.
>>>
>>> So from my POV it would make more sense to fall back to the backing file
>>> for post-EOF reads.
>>>
>>> OTOH, I don’t know whether changing that behavior would qualify as a
>>> possible security issue now, because maybe someone has sensitive
>>> information in the tail of some disk and then truncated the overlay so
>>> as to hide it? But honestly, that seems ridiculous and I can’t imagine
>>> people to do that. (It would work only for the tail, and why not just
>>> write zeroes there, which works everywhere?) So in practice I don’t
>>> believe that to be a problem.
>>>
>>> Max
>>
>> That seems to be wrong from my POW. Once we get block device truncated,
>> it exposed that tail to the guest with all zeroes.
>>
>> Let us assume that we have virtual disk of length L. We create new top
>> delta of
>> length X (less then L) and new top delta after with length Y (more than L),
>> like the following:
>>
>> [.........................] Y
>> [........] X
>> [...................] L
>>
>> Once the guest creates FS on state Y it relies on the fact that data from X
>> to Y is all zeroes.
>>
>> Any operations with backing chain must keep guest content to be tha same,
>> i.e. if we commit from Y to L, virtual disk content should be preserved,
>> i.e.
>> read as all zero even if there is some data in L from X to L.
>>
>> If we commit from X to Y, the range from X to L should remain all zeroes.
>>
>> This is especially valid for backups, which can not be changed and are
>> validated by the software from time to time.
>>
>> Does this makes sense?
>
> All right then. But then there’s the case of commit not shrinking the
> backing file, so the guest content won’t be the same if you commit a
> short overlay into a longer backing file.
>
Hmm. Isn't commit target truncated to source before operation?
--
Best regards,
Vladimir
Re: [PATCH 0/4] fix & merge block_status_above and is_allocated_above, Kevin Wolf, 2019/11/19
Re: [PATCH 0/4] fix & merge block_status_above and is_allocated_above, Kevin Wolf, 2019/11/19
Re: [PATCH 0/4] fix & merge block_status_above and is_allocated_above, Kevin Wolf, 2019/11/19
Re: [PATCH 0/4] fix & merge block_status_above and is_allocated_above, Stefan Hajnoczi, 2019/11/19