On 5/6/19 11:47 AM, Vladimir Sementsov-Ogievskiy wrote: > 01.05.2019 21:13, Alberto Garcia wrote: >> There's only a couple of bdrv_read() and bdrv_write() calls left in >> the vvfat code, and they can be trivially replaced with the byte-based >> bdrv_pread() and bdrv_pwrite(). >> >> Signed-off-by: Alberto Garcia <be...@igalia.com> >> --- >> block/vvfat.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/block/vvfat.c b/block/vvfat.c >> index 5f66787890..253cc716dd 100644 >> --- a/block/vvfat.c >> +++ 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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature