09.12.2017 19:39, Eric Blake wrote:
On 12/09/2017 06:31 AM, Vladimir Sementsov-Ogievskiy wrote:
07.12.2017 23:30, Eric Blake wrote:
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the parallels driver accordingly. Note that
the internal function block_status() is still sector-based, because
it is still in use by other sector-based functions; but that's okay
because request_alignment is 512 as a result of those functions.
For now, no optimizations are added based on the mapping hint.
Signed-off-by: Eric Blake <ebl...@redhat.com>
{
BDRVParallelsState *s = bs->opaque;
- int64_t offset;
+ int count;
+ assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
qemu_co_mutex_lock(&s->lock);
- offset = block_status(s, sector_num, nb_sectors, pnum);
+ offset = block_status(s, offset >> BDRV_SECTOR_BITS,
+ bytes >> BDRV_SECTOR_BITS, &count);
qemu_co_mutex_unlock(&s->lock);
+ *pnum = count * BDRV_SECTOR_SIZE;
if (offset < 0) {
return 0;
}
+ *map = offset;
oh, didn't notice previous time :(, should be
*map = offset * BDRV_SECTOR_SIZE
Oh, right.
(also, if you already use ">> BDRV_SECTOR_BITS" in this function,
would not it be more consistent to use "<< BDRV_SECTOR_BITS"
for sector-to-byte conversion?)
with that fixed (with "<<" or "*", up to you),
'>> BDRV_SECTOR_BITS' always works. You could also write '/
BDRV_SECTOR_SIZE' (the compiler should figure out that you are dividing
by a power of 2, and use the shift instruction under the hood), but I
find that a bit harder to reason about.
On the other hand, '<< BDRV_SECTOR_BITS' only produces the same size
output as the input; if the left side is 32 bits, it risks overflowing.
But '* BDRV_SECTOR_SIZE' always produces a 64-bit value. So I've
learned (from past mistakes in other byte-conversion series) that the
multiply form is less likely to introduce unintended truncation bugs.
hmm, interesting observation, thank you
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
--
Best regards,
Vladimir