[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Use QEMU_IS_ALIGNED instead of reinventi
From: |
Nir Soffer |
Subject: |
Re: [Qemu-devel] [PATCH] block: Use QEMU_IS_ALIGNED instead of reinventing it |
Date: |
Tue, 20 Aug 2019 23:46:49 +0300 |
On Tue, Aug 20, 2019 at 10:51 PM Max Reitz <address@hidden> wrote:
> On 17.08.19 19:53, Nir Soffer wrote:
> > Replace instances of:
> >
> > (n & (BDRV_SECTOR_SIZE - 1)) == 0)
> >
> > With:
> >
> > QEMU_IS_ALIGNED(n, BDRV_SECTOR_SIZE)
> >
> > Which reveals the intent of the code better, and makes it easier to
> > locate the code checking alignment.
> >
> > QEMU_IS_ALIGNED is implemented using %, which may be less efficient but
> > it is used only in assert() except one instance, so it should not
> > matter.
> >
> > Signed-off-by: Nir Soffer <address@hidden>
> > ---
> > block/bochs.c | 4 ++--
> > block/cloop.c | 4 ++--
> > block/dmg.c | 4 ++--
> > block/io.c | 8 ++++----
> > block/qcow2.c | 4 ++--
> > block/vvfat.c | 8 ++++----
> > qemu-img.c | 2 +-
> > 7 files changed, 17 insertions(+), 17 deletions(-)
>
> Because John was speaking about a magical incantation, I found
> BDRV_SECTOR_MASK. There are two places in block/qcow2-cluster.c that
> use that to check alignment, I think those should be amended as well.
>
$ git grep BDRV_SECTOR_MASK
block/qcow2-cluster.c: assert((offset_in_cluster &
~BDRV_SECTOR_MASK) == 0);
block/qcow2-cluster.c: assert((bytes & ~BDRV_SECTOR_MASK) == 0);
Right, should use QEMU_IS_ALIGNED
include/block/block.h:#define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1)
include/block/block.h:#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK
migration/block.c: flags = addr & ~BDRV_SECTOR_MASK;
This could use (BDRV_SECTOR_SIZE - 1).
In Linux SECTOR_MASK is defined as:
drivers/block/null_blk_main.c:#define SECTOR_MASK
(PAGE_SECTORS - 1)
It seems that qemu use use the same.
I will try to handle these in v2.
Nir