Define MASK, SHIFT to use NVME_GET

[sc108-lee]

I would like to do some modify based on usage of NVME_GET
I Just wrote only for nvme_id_nsfeat currently, Example below!
Any opinion?

From 519337c64316d8805b3d988f1b475591347f22f2 Mon Sep 17 00:00:00 2001
From: Steven Seungcheol Lee [email protected]
Date: Thu, 16 Dec 2021 17:17:24 +0900
Subject: [PATCH] types: Define enum to use NVME_GET define
Signed-off-by: Steven Seungcheol Lee [email protected]
3 files changed, 32 insertions(+), 27 deletions(-)

--- a/src/nvme/types.h
+++ b/src/nvme/types.h
@@ -1668,32 +1668,37 @@ struct nvme_id_ns {

enum nvme_id_nsfeat {
- NVME_NS_FEAT_THIN = 1 << 0,
- NVME_NS_FEAT_NATOMIC = 1 << 1,
- NVME_NS_FEAT_DULBE = 1 << 2,
- NVME_NS_FEAT_ID_REUSE = 1 << 3,
- NVME_NS_FEAT_IO_OPT = 1 << 4,
+ NVME_NS_FEAT_THINP_SHIFT = 0,
+ NVME_NS_FEAT_THINP_MASK = 0x1,
+ NVME_NS_FEAT_NSABP_SHIFT = 1,
+ NVME_NS_FEAT_NSABP_MASK = 0x1,
+ NVME_NS_FEAT_DAE_SHIFT = 2,
+ NVME_NS_FEAT_DAE_MASK = 0x1,
+ NVME_NS_FEAT_UIDREUSE_SHIFT = 3,
+ NVME_NS_FEAT_UIDREUSE_MASK = 0x1,
+ NVME_NS_FEAT_OPTERF_SHIFT = 4,
+ NVME_NS_FEAT_OPTERF_MASK = 0x1,
};

[igaw]

We seem to have both styles of enums in types.h and it seems it is inconsistently applied. I suppose the shift/mask pattern with helpers also makes shifting/masking simpler for users of these APIs. I don't have any strong opinion on this. Something @hreinecke or @keithbusch might have more input on.

[igaw]

@hreinecke Any comments on this? As I said i don't have a strong opinion but we should figure this out now. After the 1.0 release, it's too late.

[hreinecke]

I'd vote for a consistent approach, and use NVME_GET() (ie the MASK and SHIFT definitions) throughout the code.

[igaw]

Sounds good to me. Let's get this sorted out then.

[igaw]

So this is going to be bigger effort with a lot of potential to introduce regressions. As we are in the process of getting 1.0 out the door this is not going to happen. Alongside this effort we should also introduce a test frame work so we can actually test this stuff.

[igaw]

The upcoming new spec will have a JSON file which defines all types and data structs. We will use a code generator for this.