* Re: [PATCH 3/3] hw/block/nvme: add id ns metadata cap (mc) enum
2021-04-21 13:02 ` [PATCH 3/3] hw/block/nvme: add id ns metadata cap (mc) enum Gollu Appalanaidu
@ 2021-04-22 6:50 ` Klaus Jensen
2021-05-19 7:05 ` Klaus Jensen
1 sibling, 0 replies; 3+ messages in thread
From: Klaus Jensen @ 2021-04-22 6:50 UTC (permalink / raw)
To: Gollu Appalanaidu
Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, stefanha, kbusch
[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]
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
>
>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 3/3] hw/block/nvme: add id ns metadata cap (mc) enum
2021-04-21 13:02 ` [PATCH 3/3] hw/block/nvme: add id ns metadata cap (mc) enum Gollu Appalanaidu
2021-04-22 6:50 ` Klaus Jensen
@ 2021-05-19 7:05 ` Klaus Jensen
1 sibling, 0 replies; 3+ messages in thread
From: Klaus Jensen @ 2021-05-19 7:05 UTC (permalink / raw)
To: Gollu Appalanaidu
Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, stefanha, kbusch
[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]
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)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread