On Tue, Aug 20, 2019 at 10:51 PM Max Reitz <mre...@redhat.com> 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 <nsof...@redhat.com> > > --- > > 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
