On 09/26/2017 02:39 PM, John Snow wrote: >> -int64_t bdrv_get_block_status(BlockDriverState *bs, >> - int64_t sector_num, >> - int nb_sectors, int *pnum, >> - BlockDriverState **file) >> +int64_t bdrv_block_status(BlockDriverState *bs, >> + int64_t offset, int64_t bytes, int64_t *pnum, >> + BlockDriverState **file) >> { >> - return bdrv_get_block_status_above(bs, backing_bs(bs), >> - sector_num, nb_sectors, pnum, file); >> + int64_t ret; >> + int n; >> + >> + assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE)); >> + bytes = MIN(bytes, BDRV_REQUEST_MAX_BYTES); >> + ret = bdrv_get_block_status_above(bs, backing_bs(bs), >> + offset >> BDRV_SECTOR_BITS, >> + bytes >> BDRV_SECTOR_BITS, &n, file); >> + if (pnum) { >> + *pnum = n * BDRV_SECTOR_SIZE; >> + } > > Is it safe to truncate the request in the event that the caller did not > provide a pnum target? that is, how will they know for what range we are > answering?
Hmm. I think I have some rebase cruft here. At one point, I was playing with the idea of allowing pnum == NULL for ALL get_status() callers, similar to the existing block/vvfat.c:cluster_was_modified(): block/vvfat.c: res = bdrv_is_allocated(s->qcow->bs, block/vvfat.c- (offset + i) * BDRV_SECTOR_SIZE, block/vvfat.c- BDRV_SECTOR_SIZE, NULL); but looking further, only bdrv_is_allocated() (and NOT bdrv_[get_]block_status) is ever used in that manner. Or, in terms of the 'mapping' variable, a NULL pnum only makes sense when mapping == false. So the conditional on 'if (pnum)' should be dropped here. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature