On 5/6/19 12:19 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>> +++ b/block/vvfat.c
>>>> @@ -1494,8 +1494,8 @@ static int vvfat_read(BlockDriverState *bs, int64_t
>>>> sector_num,
>>>> DLOG(fprintf(stderr, "sectors %" PRId64 "+%" PRId64
>>>> " allocated\n", sector_num,
>>>> n >> BDRV_SECTOR_BITS));
>>>> - if (bdrv_read(s->qcow, sector_num, buf + i * 0x200,
>>>> - n >> BDRV_SECTOR_BITS)) {
>>>> + if (bdrv_pread(s->qcow, sector_num * BDRV_SECTOR_SIZE,
>>>> + buf + i * 0x200, n) < 0) {
>>>
>>> Shouldn't we use QEMU_ALIGN_DOWN(n, BDRV_SECTOR_SIZE) ?
>>
>> No, n should already be aligned, which makes align_down a no-op.
>>
>>> Could bdrv_is_allocated give unaligned n?
>>>
>>
>> Yes, bdrv_is_allocated can return unaligned n in some situations; I had
>> a patch that didn't make 4.0 that would add bdrv_block_status_aligned
>> for cases where we need to guarantee that different alignment of a
>> backing chain doesn't bleed through to the specified alignment of the
>> current layer. But those situations are rare, and I need to revisit
>> those and send a v2; so I don't see a problem with this one going in
>> during the meantime as-is.
>>
>
> Than, n is not already aligned, as it comes from bdrv_is_allocated.Note that whether bdrv_is_allocated can return data not aligned to 512 depends on the driver. It is possible when querying file-posix.c, but only for a POSIX file that encounters EOF mid-sector. However, it is not possible for the qcow2 driver. The patches I need to rework are worried more about cases where a block device with request_alignment of 4k can still see 512-alignment leak through from a backing file. But since vvfat is grabbing alignment from a qcow2 image, and not a raw POSIX file, we should never see sub-sector alignment. So my answers above were terse but correct: bdrv_is_allocated can return unaligned data in some cases, but vvfat should not be one of those cases. If you'd like to add an assert instead of a QEMU_ALIGN_DOWN, that should be reasonable. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
