qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v4 07/23] block: Convert bdrv_get_b


From: Eric Blake
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v4 07/23] block: Convert bdrv_get_block_status() to bytes
Date: Tue, 26 Sep 2017 14:57:21 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]