Re: [PATCH 3/3] hw/block/nvme: add id ns metadata cap (mc) enum

[Klaus Jensen]

On Apr 21 18:32, Gollu Appalanaidu wrote:
> Add Idnetify Namespace Metadata Capablities (MC) enum.
>
> Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> ---
> hw/block/nvme-ns.c   | 2 +-
> include/block/nvme.h | 5 +++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
> index 9065a7ae99..db75302136 100644
> --- a/hw/block/nvme-ns.c
> +++ b/hw/block/nvme-ns.c
> @@ -85,7 +85,7 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
>     ds = 31 - clz32(ns->blkconf.logical_block_size);
>     ms = ns->params.ms;
>
> -    id_ns->mc = 0x3;
> +    id_ns->mc = NVME_ID_NS_MC_EXTENDED | NVME_ID_NS_MC_SEPARATE;
>
>     if (ms && ns->params.mset) {
>         id_ns->flbas |= NVME_ID_NS_FLBAS_EXTENDEND;
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 1d61030756..a3b610ba86 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -1344,6 +1344,11 @@ enum NvmeIdNsFlbas {
>     NVME_ID_NS_FLBAS_EXTENDEND  = 1 << 4,
> };
>
> +enum NvmeIdNsMc {
> +    NVME_ID_NS_MC_EXTENDED      = 1 << 0,
> +    NVME_ID_NS_MC_SEPARATE      = 1 << 1,
> +};
> +
> #define NVME_ID_NS_DPS_TYPE(dps) (dps & NVME_ID_NS_DPS_TYPE_MASK)
>
> typedef struct NvmeDifTuple {
> --
> 2.17.1

So, initially I wondered why the compiler didnt complain about the 
`NVME_ID_NS_MC_EXTENDED` and `NVME_ID_NS_MC_SEPARATE` "names" being 
defined twice. A smart colleague was quick to point out that because the
`NVME_ID_NS_MC_EXTENDED(mc)` function-like macro is expanded in the 
preprocessing step, it doesn't exist when we compile so there really is 
no potential clash.

I wonder now if it is bad form to keep the function-like macro 
"accessor" there as well as the enum for the value. Does anyone have any 
opinion on this? In other words, would it be bad or confusing to do 
something like this?

   enum NvmeIdNsMc {
     NVME_ID_NS_MC_EXTENDED = 1 << 0,
   };

   #define NVME_ID_NS_MC_EXTENDED(mc) ((mc) & NVME_ID_NS_MC_EXTENDED)

signature.asc
Description: PGP signature