Alignment checks cleanup

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

Posted by Nir Soffer 6 years, 2 months ago

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.

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) {
-- 
2.20.1

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

Posted by Juan Quintela 6 years, 2 months ago

Nir Soffer <nirsof@gmail.com> 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.
>
> Signed-off-by: Nir Soffer <nsoffer@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

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

Posted by John Snow 6 years, 2 months ago


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) {
>