Re: [PATCH for-7.0 2/4] hw/nvme: add zone attribute get/set helpers

[Keith Busch]

On Thu, Nov 25, 2021 at 08:37:33AM +0100, Klaus Jensen wrote:
> @@ -295,7 +295,7 @@ static void nvme_assign_zone_state(NvmeNamespace *ns, 
> NvmeZone *zone,
>      case NVME_ZONE_STATE_READ_ONLY:
>          break;
>      default:
> -        zone->d.za = 0;
> +        NVME_ZA_CLEAR_ALL(zone->d.za);
>      }
>  }
>  
> @@ -3356,7 +3356,7 @@ static uint16_t nvme_set_zd_ext(NvmeNamespace *ns, 
> NvmeZone *zone)
>              return status;
>          }
>          nvme_aor_inc_active(ns);
> -        zone->d.za |= NVME_ZA_ZD_EXT_VALID;
> +        NVME_ZA_SET(zone->d.za, NVME_ZA_ZD_EXT_VALID);
>          nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_CLOSED);
>          return NVME_SUCCESS;
>      }
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 2ee227760265..2b8b906466ab 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -1407,6 +1407,10 @@ enum NvmeZoneAttr {
>      NVME_ZA_ZD_EXT_VALID             = 1 << 7,
>  };
>  
> +#define NVME_ZA_SET(za, attrs)   ((za) |= (attrs))
> +#define NVME_ZA_CLEAR(za, attrs) ((za) &= ~(attrs))
> +#define NVME_ZA_CLEAR_ALL(za)    ((za) = 0x0)

This doesn't really look any more helpful than open coding it. I think
it would appear better to take a "struct NvmeZone" type parameter
instead, and use inline functions instead of macro.