Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/2] block: Remove unused masks

[John Snow <jsnow@redhat.com>]

On 8/27/19 2:59 PM, Nir Soffer wrote:
> Replace confusing usage:
> 
>     ~BDRV_SECTOR_MASK
> 
> With more clear:
> 
>     (BDRV_SECTOR_SIZE - 1)
> 
> Remove BDRV_SECTOR_MASK and the unused BDRV_BLOCK_OFFSET_MASK which was
> it's last user.
> 

Kind of lateral in my opinion, but if there was only ONE user across TWO
definitions, then for sure it can go, especially because using the
ALIGNED macros is indeed nicer and should be encouraged.

Reviewed-by: John Snow <jsnow@redhat.com>

> Signed-off-by: Nir Soffer <nsoffer@redhat.com>
> ---
>  include/block/block.h | 2 --
>  migration/block.c     | 2 +-
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/block/block.h b/include/block/block.h
> index 124ad40809..37c9de7446 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -143,7 +143,6 @@ typedef struct HDGeometry {
>  
>  #define BDRV_SECTOR_BITS   9
>  #define BDRV_SECTOR_SIZE   (1ULL << BDRV_SECTOR_BITS)
> -#define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
>  
>  #define BDRV_REQUEST_MAX_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, \
>                                       INT_MAX >> BDRV_SECTOR_BITS)
> @@ -195,7 +194,6 @@ typedef struct HDGeometry {
>  #define BDRV_BLOCK_ALLOCATED    0x10
>  #define BDRV_BLOCK_EOF          0x20
>  #define BDRV_BLOCK_RECURSE      0x40
> -#define BDRV_BLOCK_OFFSET_MASK  BDRV_SECTOR_MASK
>  
>  typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
>  
> diff --git a/migration/block.c b/migration/block.c
> index aa747b55fa..92c36b68ec 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -906,7 +906,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>      do {
>          addr = qemu_get_be64(f);
>  
> -        flags = addr & ~BDRV_SECTOR_MASK;
> +        flags = addr & (BDRV_SECTOR_SIZE - 1);
>          addr >>= BDRV_SECTOR_BITS;
>  
>          if (flags & BLK_MIG_FLAG_DEVICE_BLOCK) {
>