* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
This patchset implemets NVMe Namespace Descriptor Identification as of
NVMe 1.3. The Namespace Descriptor Identification allows a NVMe host
to query several Namespace Identification mechanisms, such as EUI-64,
NGUID and UUID from the target. If more than one value is set by the
target, it can transmit all set values to the host.
The Namespace Identification Descriptor list is the only way a target
can identify itself via the newly introduced UUID to the host (in
addition to the EUI-64 or NGUID).
Both the Host and Target side are implemented. In order to get the
Linux Host to send the Linux target implementation a Namespace
Descriptor Identification command, you have to change the target's
announced version code to at least 1.3.
Unfortunately the host side already did have a sysfs attribute called
'uuid' which represented the NGUID, so precautions have been taken to
not break any existing userspace.
The code is tested using the nvme-loop loopback target and cut against
Christoph's uuid/for-next branch.
A patch for nvmetcli and nvme-cli will follow shortly.
Changes to v5:
* Change '\0' to 0 in sg_zerroout_area()s memset (Sagi)
* Fix uinitialized variable in version setting (Guan)
* Renamed struct nvme_ns_identifier_hdr to nvme_ns_id_desc (Sagi)
* Move all users of dev_(warn|err)\(ctrl->dev to ctrl->device (Sagi)
* Add Max's R-b
Changes to v4:
* Add nvmet_copy_ns_identifier() helper (Christoph)
* Move 'nvmet: use NVME_IDENTIFY_DATA_SIZE' to beginning of series (Christoph)
* Protect subsystem version with nvmet_config_sem (Christoph)
* Get rid of local variables in nvmet_subsys_version_show (Christoph)
* Included dots in subsystem version (Sagi)
* Merge nvme_parse_ns_descs() into nvme_identify_ns_descs() (Christoph)
* Skip unknown NS descriptors in nvme_identify_ns_descs() (Christoph)
* Add sg_zeroout_area() helper to lib/scatterlist.c (Christoph)
* I'm not sure if I shall use nvme_ns_identifier_hdr or nvme_ns_id_desc
* Rebased on Christoph's uuid/for-next branch
* Collected Acks
Changes to v3:
* Autogenerate UUID on target NS allocation (Sagi)
Changes to v2:
* Added Max's Reviewed-by
* Make series bisectable
Changes to v1:
* Added Reviewed-by tags from Christoph and Hannes for unchanged patches
* Added patch introducing new structs at the beginning (Christoph)
* Dropped SZ_4K patch
* Got rid of dynamic memory allocation on the target side (Christoph)
* Reworked host side parser (Christoph)
* Check length inside type check in the host parser (Max)
Changes to v0:
* Fixed wrong size of 4069 and replaced it with SZ_4K (Max)
* Add constants for UUID, NGUID and EUI-64 length (Max)
* Drop EUI-64 Support on target side (Christoph)
* Use uuid_be instead of u8[] (Christoph)
* Add ability to override target's version (Hannes)
* Change hard coded magic 4096 and 0x1000 to SZ_4k in drivers/nvme/*
Johannes Thumshirn (10):
scatterlist: add sg_zeroout_area() helper
nvmet: use NVME_IDENTIFY_DATA_SIZE
nvme: introduce NVMe Namespace Identification Descriptor structures
nvme: rename uuid to nguid in nvme_ns
nvmet: implement namespace identify descriptor list
nvmet: add uuid field to nvme_ns and populate via configfs
nvme: get list of namespace descriptors
nvme: provide UUID value to userspace
nvmet: allow overriding the NVMe VS via configfs
nvme: use ctrl->device consistently for logging
drivers/nvme/host/core.c | 120 +++++++++++++++++++++++++++++++++++++---
drivers/nvme/host/fc.c | 12 ++--
drivers/nvme/host/nvme.h | 1 +
drivers/nvme/target/admin-cmd.c | 65 +++++++++++++++++++++-
drivers/nvme/target/configfs.c | 68 +++++++++++++++++++++++
drivers/nvme/target/core.c | 3 +-
drivers/nvme/target/discovery.c | 2 +-
drivers/nvme/target/nvmet.h | 1 +
include/linux/nvme.h | 23 ++++++++
include/linux/scatterlist.h | 3 +
lib/scatterlist.c | 37 +++++++++++++
11 files changed, 316 insertions(+), 19 deletions(-)
--
2.12.3
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 01/10] scatterlist: add sg_zeroout_area() helper
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
The sg_zeroout_area() helper is used to zero fill an area in a SG
list.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
include/linux/scatterlist.h | 3 +++
lib/scatterlist.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index cb3c8fe6acd7..a38ce9f4e78a 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -279,6 +279,9 @@ size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents,
size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
void *buf, size_t buflen, off_t skip);
+size_t sg_zeroout_area(struct scatterlist *sgl, unsigned int nents,
+ size_t buflen, off_t skip);
+
/*
* Maximum number of entries that will be allocated in one piece, if
* a list larger than this is required then chaining will be utilized.
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index c6cf82242d65..584f50c94fce 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -751,3 +751,40 @@ size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
return sg_copy_buffer(sgl, nents, buf, buflen, skip, true);
}
EXPORT_SYMBOL(sg_pcopy_to_buffer);
+
+/**
+ * sg_zeroout_area - Zero-out a part of a SG list
+ * @sgl: The SG list
+ * @nents: Number of SG entries
+ * @buflen: The number of bytes to zero out
+ * @skip: Number of bytes to skip before zeroing
+ *
+ * Returns the number of bytes zeroed.
+ *
+ **/
+size_t sg_zeroout_area(struct scatterlist *sgl, unsigned int nents,
+ size_t buflen, off_t skip)
+{
+ unsigned int offset = 0;
+ struct sg_mapping_iter miter;
+ unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_TO_SG;
+
+ sg_miter_start(&miter, sgl, nents, sg_flags);
+
+ if (!sg_miter_skip(&miter, skip))
+ return false;
+
+ while ((offset < buflen) && sg_miter_next(&miter)) {
+ unsigned int len;
+
+ len = min(miter.length, buflen - offset);
+ memset(miter.addr, 0, len);
+
+ offset += len;
+ }
+
+ sg_miter_stop(&miter);
+
+ return offset;
+}
+EXPORT_SYMBOL(sg_zeroout_area);
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 01/10] scatterlist: add sg_zeroout_area() helper
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
The sg_zeroout_area() helper is used to zero fill an area in a SG
list.
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
---
include/linux/scatterlist.h | 3 +++
lib/scatterlist.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index cb3c8fe6acd7..a38ce9f4e78a 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -279,6 +279,9 @@ size_t sg_pcopy_from_buffer(struct scatterlist *sgl, unsigned int nents,
size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
void *buf, size_t buflen, off_t skip);
+size_t sg_zeroout_area(struct scatterlist *sgl, unsigned int nents,
+ size_t buflen, off_t skip);
+
/*
* Maximum number of entries that will be allocated in one piece, if
* a list larger than this is required then chaining will be utilized.
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index c6cf82242d65..584f50c94fce 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -751,3 +751,40 @@ size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
return sg_copy_buffer(sgl, nents, buf, buflen, skip, true);
}
EXPORT_SYMBOL(sg_pcopy_to_buffer);
+
+/**
+ * sg_zeroout_area - Zero-out a part of a SG list
+ * @sgl: The SG list
+ * @nents: Number of SG entries
+ * @buflen: The number of bytes to zero out
+ * @skip: Number of bytes to skip before zeroing
+ *
+ * Returns the number of bytes zeroed.
+ *
+ **/
+size_t sg_zeroout_area(struct scatterlist *sgl, unsigned int nents,
+ size_t buflen, off_t skip)
+{
+ unsigned int offset = 0;
+ struct sg_mapping_iter miter;
+ unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_TO_SG;
+
+ sg_miter_start(&miter, sgl, nents, sg_flags);
+
+ if (!sg_miter_skip(&miter, skip))
+ return false;
+
+ while ((offset < buflen) && sg_miter_next(&miter)) {
+ unsigned int len;
+
+ len = min(miter.length, buflen - offset);
+ memset(miter.addr, 0, len);
+
+ offset += len;
+ }
+
+ sg_miter_stop(&miter);
+
+ return offset;
+}
+EXPORT_SYMBOL(sg_zeroout_area);
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* Re: [PATCH v6 01/10] scatterlist: add sg_zeroout_area() helper
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:49 ` Sagi Grimberg
-1 siblings, 0 replies; 76+ messages in thread
From: Sagi Grimberg @ 2017-06-07 9:49 UTC (permalink / raw)
To: Johannes Thumshirn, Christoph Hellwig, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 01/10] scatterlist: add sg_zeroout_area() helper
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-08 7:41 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:41 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
> size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
> void *buf, size_t buflen, off_t skip);
>
> +size_t sg_zeroout_area(struct scatterlist *sgl, unsigned int nents,
> + size_t buflen, off_t skip);
Maybe sg_zero_buffer to fit with the other functions in the family?
(I'd be happy to fix this up if you're ok and we don't need another
respin otherwise).
> + while ((offset < buflen) && sg_miter_next(&miter)) {
Nit: no need for the inner braces.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 01/10] scatterlist: add sg_zeroout_area() helper
@ 2017-06-08 7:41 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:41 UTC (permalink / raw)
> size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
> void *buf, size_t buflen, off_t skip);
>
> +size_t sg_zeroout_area(struct scatterlist *sgl, unsigned int nents,
> + size_t buflen, off_t skip);
Maybe sg_zero_buffer to fit with the other functions in the family?
(I'd be happy to fix this up if you're ok and we don't need another
respin otherwise).
> + while ((offset < buflen) && sg_miter_next(&miter)) {
Nit: no need for the inner braces.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch at lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 01/10] scatterlist: add sg_zeroout_area() helper
2017-06-08 7:41 ` Christoph Hellwig
@ 2017-06-08 7:46 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-08 7:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Keith Busch, Hannes Reinecke, Max Gurtovoy,
Linux NVMe Mailinglist, Linux Kernel Mailinglist
On 06/08/2017 09:41 AM, Christoph Hellwig wrote:
>> size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
>> void *buf, size_t buflen, off_t skip);
>>
>> +size_t sg_zeroout_area(struct scatterlist *sgl, unsigned int nents,
>> + size_t buflen, off_t skip);
>
> Maybe sg_zero_buffer to fit with the other functions in the family?
>
> (I'd be happy to fix this up if you're ok and we don't need another
> respin otherwise).
Sounds good.
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 01/10] scatterlist: add sg_zeroout_area() helper
@ 2017-06-08 7:46 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-08 7:46 UTC (permalink / raw)
On 06/08/2017 09:41 AM, Christoph Hellwig wrote:
>> size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
>> void *buf, size_t buflen, off_t skip);
>>
>> +size_t sg_zeroout_area(struct scatterlist *sgl, unsigned int nents,
>> + size_t buflen, off_t skip);
>
> Maybe sg_zero_buffer to fit with the other functions in the family?
>
> (I'd be happy to fix this up if you're ok and we don't need another
> respin otherwise).
Sounds good.
--
Johannes Thumshirn Storage
jthumshirn at suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 02/10] nvmet: use NVME_IDENTIFY_DATA_SIZE
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
Use NVME_IDENTIFY_DATA_SIZE define instead of hard coding the magic
4096 value.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
drivers/nvme/target/admin-cmd.c | 4 ++--
drivers/nvme/target/discovery.c | 2 +-
include/linux/nvme.h | 2 ++
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index ff1f97006322..96c144325443 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -336,7 +336,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
static void nvmet_execute_identify_nslist(struct nvmet_req *req)
{
- static const int buf_size = 4096;
+ static const int buf_size = NVME_IDENTIFY_DATA_SIZE;
struct nvmet_ctrl *ctrl = req->sq->ctrl;
struct nvmet_ns *ns;
u32 min_nsid = le32_to_cpu(req->cmd->identify.nsid);
@@ -504,7 +504,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
}
break;
case nvme_admin_identify:
- req->data_len = 4096;
+ req->data_len = NVME_IDENTIFY_DATA_SIZE;
switch (cmd->identify.cns) {
case NVME_ID_CNS_NS:
req->execute = nvmet_execute_identify_ns;
diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
index 1aaf597e81fc..c7a90384dd75 100644
--- a/drivers/nvme/target/discovery.c
+++ b/drivers/nvme/target/discovery.c
@@ -185,7 +185,7 @@ u16 nvmet_parse_discovery_cmd(struct nvmet_req *req)
return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
}
case nvme_admin_identify:
- req->data_len = 4096;
+ req->data_len = NVME_IDENTIFY_DATA_SIZE;
switch (cmd->identify.cns) {
case NVME_ID_CNS_CTRL:
req->execute =
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index e400a69fa1d3..c004af5faa48 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -659,6 +659,8 @@ struct nvme_identify {
__u32 rsvd11[5];
};
+#define NVME_IDENTIFY_DATA_SIZE 4096
+
struct nvme_features {
__u8 opcode;
__u8 flags;
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 02/10] nvmet: use NVME_IDENTIFY_DATA_SIZE
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
Use NVME_IDENTIFY_DATA_SIZE define instead of hard coding the magic
4096 value.
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: Hannes Reinecke <hare at suse.com>
---
drivers/nvme/target/admin-cmd.c | 4 ++--
drivers/nvme/target/discovery.c | 2 +-
include/linux/nvme.h | 2 ++
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index ff1f97006322..96c144325443 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -336,7 +336,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
static void nvmet_execute_identify_nslist(struct nvmet_req *req)
{
- static const int buf_size = 4096;
+ static const int buf_size = NVME_IDENTIFY_DATA_SIZE;
struct nvmet_ctrl *ctrl = req->sq->ctrl;
struct nvmet_ns *ns;
u32 min_nsid = le32_to_cpu(req->cmd->identify.nsid);
@@ -504,7 +504,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
}
break;
case nvme_admin_identify:
- req->data_len = 4096;
+ req->data_len = NVME_IDENTIFY_DATA_SIZE;
switch (cmd->identify.cns) {
case NVME_ID_CNS_NS:
req->execute = nvmet_execute_identify_ns;
diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
index 1aaf597e81fc..c7a90384dd75 100644
--- a/drivers/nvme/target/discovery.c
+++ b/drivers/nvme/target/discovery.c
@@ -185,7 +185,7 @@ u16 nvmet_parse_discovery_cmd(struct nvmet_req *req)
return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
}
case nvme_admin_identify:
- req->data_len = 4096;
+ req->data_len = NVME_IDENTIFY_DATA_SIZE;
switch (cmd->identify.cns) {
case NVME_ID_CNS_CTRL:
req->execute =
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index e400a69fa1d3..c004af5faa48 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -659,6 +659,8 @@ struct nvme_identify {
__u32 rsvd11[5];
};
+#define NVME_IDENTIFY_DATA_SIZE 4096
+
struct nvme_features {
__u8 opcode;
__u8 flags;
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* Re: [PATCH v6 02/10] nvmet: use NVME_IDENTIFY_DATA_SIZE
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-08 7:42 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:42 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
Looks good, but we should also use this for BUILD_BUG_ON
checks on the identify payloads, bug I can fix that up.
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 03/10] nvme: introduce NVMe Namespace Identification Descriptor structures
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
include/linux/nvme.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index c004af5faa48..01d75c2b32a6 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -289,6 +289,7 @@ enum {
NVME_ID_CNS_NS = 0x00,
NVME_ID_CNS_CTRL = 0x01,
NVME_ID_CNS_NS_ACTIVE_LIST = 0x02,
+ NVME_ID_CNS_NS_DESC_LIST = 0x03,
NVME_ID_CNS_NS_PRESENT_LIST = 0x10,
NVME_ID_CNS_NS_PRESENT = 0x11,
NVME_ID_CNS_CTRL_NS_LIST = 0x12,
@@ -315,6 +316,22 @@ enum {
NVME_NS_DPS_PI_TYPE3 = 3,
};
+struct nvme_ns_id_desc {
+ __u8 nidt;
+ __u8 nidl;
+ __le16 reserved;
+};
+
+#define NVME_NIDT_EUI64_LEN 8
+#define NVME_NIDT_NGUID_LEN 16
+#define NVME_NIDT_UUID_LEN 16
+
+enum {
+ NVME_NIDT_EUI64 = 0x01,
+ NVME_NIDT_NGUID = 0x02,
+ NVME_NIDT_UUID = 0x03,
+};
+
struct nvme_smart_log {
__u8 critical_warning;
__u8 temperature[2];
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 03/10] nvme: introduce NVMe Namespace Identification Descriptor structures
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: Hannes Reinecke <hare at suse.com>
---
include/linux/nvme.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index c004af5faa48..01d75c2b32a6 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -289,6 +289,7 @@ enum {
NVME_ID_CNS_NS = 0x00,
NVME_ID_CNS_CTRL = 0x01,
NVME_ID_CNS_NS_ACTIVE_LIST = 0x02,
+ NVME_ID_CNS_NS_DESC_LIST = 0x03,
NVME_ID_CNS_NS_PRESENT_LIST = 0x10,
NVME_ID_CNS_NS_PRESENT = 0x11,
NVME_ID_CNS_CTRL_NS_LIST = 0x12,
@@ -315,6 +316,22 @@ enum {
NVME_NS_DPS_PI_TYPE3 = 3,
};
+struct nvme_ns_id_desc {
+ __u8 nidt;
+ __u8 nidl;
+ __le16 reserved;
+};
+
+#define NVME_NIDT_EUI64_LEN 8
+#define NVME_NIDT_NGUID_LEN 16
+#define NVME_NIDT_UUID_LEN 16
+
+enum {
+ NVME_NIDT_EUI64 = 0x01,
+ NVME_NIDT_NGUID = 0x02,
+ NVME_NIDT_UUID = 0x03,
+};
+
struct nvme_smart_log {
__u8 critical_warning;
__u8 temperature[2];
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* Re: [PATCH v6 03/10] nvme: introduce NVMe Namespace Identification Descriptor structures
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-08 7:42 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:42 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 04/10] nvme: rename uuid to nguid in nvme_ns
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
The uuid field in the nvme_ns structure represents the nguid field
from the identify namespace command. And as NVMe 1.3 introduced an
UUID in the NVMe Namespace Identification Descriptor this will
collide.
So rename the uuid to nguid to prevent any further
confusion. Unfortunately we export the nguid to sysfs in the uuid
sysfs attribute, but this can't be changed anymore without possibly
breaking existing userspace.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
drivers/nvme/host/core.c | 10 +++++-----
drivers/nvme/host/nvme.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a60926410438..7b254be16887 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1016,7 +1016,7 @@ static int nvme_revalidate_ns(struct nvme_ns *ns, struct nvme_id_ns **id)
if (ns->ctrl->vs >= NVME_VS(1, 1, 0))
memcpy(ns->eui, (*id)->eui64, sizeof(ns->eui));
if (ns->ctrl->vs >= NVME_VS(1, 2, 0))
- memcpy(ns->uuid, (*id)->nguid, sizeof(ns->uuid));
+ memcpy(ns->nguid, (*id)->nguid, sizeof(ns->nguid));
return 0;
}
@@ -1784,8 +1784,8 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
int serial_len = sizeof(ctrl->serial);
int model_len = sizeof(ctrl->model);
- if (memchr_inv(ns->uuid, 0, sizeof(ns->uuid)))
- return sprintf(buf, "eui.%16phN\n", ns->uuid);
+ if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
+ return sprintf(buf, "eui.%16phN\n", ns->nguid);
if (memchr_inv(ns->eui, 0, sizeof(ns->eui)))
return sprintf(buf, "eui.%8phN\n", ns->eui);
@@ -1804,7 +1804,7 @@ static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
- return sprintf(buf, "%pU\n", ns->uuid);
+ return sprintf(buf, "%pU\n", ns->nguid);
}
static DEVICE_ATTR(uuid, S_IRUGO, uuid_show, NULL);
@@ -1839,7 +1839,7 @@ static umode_t nvme_ns_attrs_are_visible(struct kobject *kobj,
struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
if (a == &dev_attr_uuid.attr) {
- if (!memchr_inv(ns->uuid, 0, sizeof(ns->uuid)))
+ if (!memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
return 0;
}
if (a == &dev_attr_eui.attr) {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9d6a070d4391..5004f0c41397 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -189,7 +189,7 @@ struct nvme_ns {
int instance;
u8 eui[8];
- u8 uuid[16];
+ u8 nguid[16];
unsigned ns_id;
int lba_shift;
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 04/10] nvme: rename uuid to nguid in nvme_ns
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
The uuid field in the nvme_ns structure represents the nguid field
from the identify namespace command. And as NVMe 1.3 introduced an
UUID in the NVMe Namespace Identification Descriptor this will
collide.
So rename the uuid to nguid to prevent any further
confusion. Unfortunately we export the nguid to sysfs in the uuid
sysfs attribute, but this can't be changed anymore without possibly
breaking existing userspace.
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
---
drivers/nvme/host/core.c | 10 +++++-----
drivers/nvme/host/nvme.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a60926410438..7b254be16887 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1016,7 +1016,7 @@ static int nvme_revalidate_ns(struct nvme_ns *ns, struct nvme_id_ns **id)
if (ns->ctrl->vs >= NVME_VS(1, 1, 0))
memcpy(ns->eui, (*id)->eui64, sizeof(ns->eui));
if (ns->ctrl->vs >= NVME_VS(1, 2, 0))
- memcpy(ns->uuid, (*id)->nguid, sizeof(ns->uuid));
+ memcpy(ns->nguid, (*id)->nguid, sizeof(ns->nguid));
return 0;
}
@@ -1784,8 +1784,8 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
int serial_len = sizeof(ctrl->serial);
int model_len = sizeof(ctrl->model);
- if (memchr_inv(ns->uuid, 0, sizeof(ns->uuid)))
- return sprintf(buf, "eui.%16phN\n", ns->uuid);
+ if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
+ return sprintf(buf, "eui.%16phN\n", ns->nguid);
if (memchr_inv(ns->eui, 0, sizeof(ns->eui)))
return sprintf(buf, "eui.%8phN\n", ns->eui);
@@ -1804,7 +1804,7 @@ static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
- return sprintf(buf, "%pU\n", ns->uuid);
+ return sprintf(buf, "%pU\n", ns->nguid);
}
static DEVICE_ATTR(uuid, S_IRUGO, uuid_show, NULL);
@@ -1839,7 +1839,7 @@ static umode_t nvme_ns_attrs_are_visible(struct kobject *kobj,
struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
if (a == &dev_attr_uuid.attr) {
- if (!memchr_inv(ns->uuid, 0, sizeof(ns->uuid)))
+ if (!memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
return 0;
}
if (a == &dev_attr_eui.attr) {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9d6a070d4391..5004f0c41397 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -189,7 +189,7 @@ struct nvme_ns {
int instance;
u8 eui[8];
- u8 uuid[16];
+ u8 nguid[16];
unsigned ns_id;
int lba_shift;
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
A NVMe Identify NS command with a CNS value of '3' is expecting a list
of Namespace Identification Descriptor structures to be returned to
the host for the namespace requested in the namespace identify
command.
This Namespace Identification Descriptor structure consists of the
type of the namespace identifier, the length of the identifier and the
actual identifier.
Valid types are NGUID and UUID which we have saved in our nvme_ns
structure if they have been configured via configfs. If no value has
been assigened to one of these we return an "invalid opcode" back to
the host to maintain backward compatibiliy with older implementations
without Namespace Identify Descriptor list support.
Also as the Namespace Identify Descriptor list is the only mandatory
feature change between 1.2.1 and 1.3 we can bump the advertised
version as well.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
---
drivers/nvme/target/admin-cmd.c | 61 +++++++++++++++++++++++++++++++++++++++++
drivers/nvme/target/core.c | 3 +-
drivers/nvme/target/nvmet.h | 1 +
3 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 96c144325443..6f9f0881aa2b 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -367,6 +367,64 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req)
nvmet_req_complete(req, status);
}
+static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
+ void *id, off_t *off)
+{
+ struct nvme_ns_id_desc desc = {
+ .nidt = type,
+ .nidl = len,
+ };
+ u16 status;
+
+ status = nvmet_copy_to_sgl(req, *off, &desc, sizeof(desc));
+ if (status)
+ return status;
+ *off += sizeof(desc);
+
+ status = nvmet_copy_to_sgl(req, *off, id, len);
+ if (status)
+ return status;
+ *off += len;
+
+ return 0;
+}
+
+static void nvmet_execute_identify_desclist(struct nvmet_req *req)
+{
+ struct nvmet_ns *ns;
+ u16 status = 0;
+ off_t off = 0;
+
+ ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
+ if (!ns) {
+ status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+ goto out;
+ }
+
+ if (memchr_inv(&ns->uuid, 0, sizeof(ns->uuid))) {
+ status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID,
+ NVME_NIDT_UUID_LEN,
+ &ns->uuid, &off);
+ if (status)
+ goto out_put_ns;
+ }
+ if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid))) {
+ status = nvmet_copy_ns_identifier(req, NVME_NIDT_NGUID,
+ NVME_NIDT_NGUID_LEN,
+ &ns->nguid, &off);
+ if (status)
+ goto out_put_ns;
+ }
+
+ if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
+ != NVME_IDENTIFY_DATA_SIZE - off)
+ status = NVME_SC_INTERNAL | NVME_SC_DNR;
+out_put_ns:
+ nvmet_put_namespace(ns);
+out:
+ nvmet_req_complete(req, status);
+}
+
/*
* A "mimimum viable" abort implementation: the command is mandatory in the
* spec, but we are not required to do any useful work. We couldn't really
@@ -515,6 +573,9 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
case NVME_ID_CNS_NS_ACTIVE_LIST:
req->execute = nvmet_execute_identify_nslist;
return 0;
+ case NVME_ID_CNS_NS_DESC_LIST:
+ req->execute = nvmet_execute_identify_desclist;
+ return 0;
}
break;
case nvme_admin_abort_cmd:
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index eb9399ac97cf..b5b4ac103748 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -380,6 +380,7 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
ns->nsid = nsid;
ns->subsys = subsys;
+ uuid_gen(&ns->uuid);
return ns;
}
@@ -926,7 +927,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
if (!subsys)
return NULL;
- subsys->ver = NVME_VS(1, 2, 1); /* NVMe 1.2.1 */
+ subsys->ver = NVME_VS(1, 3, 0); /* NVMe 1.3.0 */
switch (type) {
case NVME_NQN_NVME:
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 8ff6e430b30a..747bbdb4f9c6 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -47,6 +47,7 @@ struct nvmet_ns {
u32 blksize_shift;
loff_t size;
u8 nguid[16];
+ uuid_t uuid;
bool enabled;
struct nvmet_subsys *subsys;
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
A NVMe Identify NS command with a CNS value of '3' is expecting a list
of Namespace Identification Descriptor structures to be returned to
the host for the namespace requested in the namespace identify
command.
This Namespace Identification Descriptor structure consists of the
type of the namespace identifier, the length of the identifier and the
actual identifier.
Valid types are NGUID and UUID which we have saved in our nvme_ns
structure if they have been configured via configfs. If no value has
been assigened to one of these we return an "invalid opcode" back to
the host to maintain backward compatibiliy with older implementations
without Namespace Identify Descriptor list support.
Also as the Namespace Identify Descriptor list is the only mandatory
feature change between 1.2.1 and 1.3 we can bump the advertised
version as well.
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
---
drivers/nvme/target/admin-cmd.c | 61 +++++++++++++++++++++++++++++++++++++++++
drivers/nvme/target/core.c | 3 +-
drivers/nvme/target/nvmet.h | 1 +
3 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 96c144325443..6f9f0881aa2b 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -367,6 +367,64 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req)
nvmet_req_complete(req, status);
}
+static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
+ void *id, off_t *off)
+{
+ struct nvme_ns_id_desc desc = {
+ .nidt = type,
+ .nidl = len,
+ };
+ u16 status;
+
+ status = nvmet_copy_to_sgl(req, *off, &desc, sizeof(desc));
+ if (status)
+ return status;
+ *off += sizeof(desc);
+
+ status = nvmet_copy_to_sgl(req, *off, id, len);
+ if (status)
+ return status;
+ *off += len;
+
+ return 0;
+}
+
+static void nvmet_execute_identify_desclist(struct nvmet_req *req)
+{
+ struct nvmet_ns *ns;
+ u16 status = 0;
+ off_t off = 0;
+
+ ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
+ if (!ns) {
+ status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+ goto out;
+ }
+
+ if (memchr_inv(&ns->uuid, 0, sizeof(ns->uuid))) {
+ status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID,
+ NVME_NIDT_UUID_LEN,
+ &ns->uuid, &off);
+ if (status)
+ goto out_put_ns;
+ }
+ if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid))) {
+ status = nvmet_copy_ns_identifier(req, NVME_NIDT_NGUID,
+ NVME_NIDT_NGUID_LEN,
+ &ns->nguid, &off);
+ if (status)
+ goto out_put_ns;
+ }
+
+ if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
+ != NVME_IDENTIFY_DATA_SIZE - off)
+ status = NVME_SC_INTERNAL | NVME_SC_DNR;
+out_put_ns:
+ nvmet_put_namespace(ns);
+out:
+ nvmet_req_complete(req, status);
+}
+
/*
* A "mimimum viable" abort implementation: the command is mandatory in the
* spec, but we are not required to do any useful work. We couldn't really
@@ -515,6 +573,9 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
case NVME_ID_CNS_NS_ACTIVE_LIST:
req->execute = nvmet_execute_identify_nslist;
return 0;
+ case NVME_ID_CNS_NS_DESC_LIST:
+ req->execute = nvmet_execute_identify_desclist;
+ return 0;
}
break;
case nvme_admin_abort_cmd:
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index eb9399ac97cf..b5b4ac103748 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -380,6 +380,7 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
ns->nsid = nsid;
ns->subsys = subsys;
+ uuid_gen(&ns->uuid);
return ns;
}
@@ -926,7 +927,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
if (!subsys)
return NULL;
- subsys->ver = NVME_VS(1, 2, 1); /* NVMe 1.2.1 */
+ subsys->ver = NVME_VS(1, 3, 0); /* NVMe 1.3.0 */
switch (type) {
case NVME_NQN_NVME:
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 8ff6e430b30a..747bbdb4f9c6 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -47,6 +47,7 @@ struct nvmet_ns {
u32 blksize_shift;
loff_t size;
u8 nguid[16];
+ uuid_t uuid;
bool enabled;
struct nvmet_subsys *subsys;
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* Re: [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:50 ` Sagi Grimberg
-1 siblings, 0 replies; 76+ messages in thread
From: Sagi Grimberg @ 2017-06-07 9:50 UTC (permalink / raw)
To: Johannes Thumshirn, Christoph Hellwig, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist
Last one :)
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Thanks Johannes
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-08 7:44 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:44 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
On Wed, Jun 07, 2017 at 11:45:32AM +0200, Johannes Thumshirn wrote:
> A NVMe Identify NS command with a CNS value of '3' is expecting a list
> of Namespace Identification Descriptor structures to be returned to
> the host for the namespace requested in the namespace identify
> command.
>
> This Namespace Identification Descriptor structure consists of the
> type of the namespace identifier, the length of the identifier and the
> actual identifier.
>
> Valid types are NGUID and UUID which we have saved in our nvme_ns
> structure if they have been configured via configfs. If no value has
> been assigened to one of these we return an "invalid opcode" back to
> the host to maintain backward compatibiliy with older implementations
> without Namespace Identify Descriptor list support.
>
> Also as the Namespace Identify Descriptor list is the only mandatory
> feature change between 1.2.1 and 1.3 we can bump the advertised
> version as well.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
> ---
> drivers/nvme/target/admin-cmd.c | 61 +++++++++++++++++++++++++++++++++++++++++
> drivers/nvme/target/core.c | 3 +-
> drivers/nvme/target/nvmet.h | 1 +
> 3 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index 96c144325443..6f9f0881aa2b 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -367,6 +367,64 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req)
> nvmet_req_complete(req, status);
> }
>
> +static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
> + void *id, off_t *off)
> +{
> + struct nvme_ns_id_desc desc = {
> + .nidt = type,
> + .nidl = len,
> + };
> + u16 status;
> +
> + status = nvmet_copy_to_sgl(req, *off, &desc, sizeof(desc));
> + if (status)
> + return status;
> + *off += sizeof(desc);
> +
> + status = nvmet_copy_to_sgl(req, *off, id, len);
> + if (status)
> + return status;
> + *off += len;
> +
> + return 0;
> +}
> +
> +static void nvmet_execute_identify_desclist(struct nvmet_req *req)
> +{
> + struct nvmet_ns *ns;
> + u16 status = 0;
> + off_t off = 0;
> +
> + ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
> + if (!ns) {
> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
> + goto out;
> + }
> +
> + if (memchr_inv(&ns->uuid, 0, sizeof(ns->uuid))) {
> + status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID,
> + NVME_NIDT_UUID_LEN,
> + &ns->uuid, &off);
> + if (status)
> + goto out_put_ns;
> + }
> + if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid))) {
> + status = nvmet_copy_ns_identifier(req, NVME_NIDT_NGUID,
> + NVME_NIDT_NGUID_LEN,
> + &ns->nguid, &off);
> + if (status)
> + goto out_put_ns;
> + }
> +
> + if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
Shouldn;t the third argument be NVME_IDENTIFY_DATA_SIZE - off in theory?
It's probably fine as is as the S/G helpers deal with overflows
gracefully, but still..
Otherwise looks fine:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
@ 2017-06-08 7:44 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:44 UTC (permalink / raw)
On Wed, Jun 07, 2017@11:45:32AM +0200, Johannes Thumshirn wrote:
> A NVMe Identify NS command with a CNS value of '3' is expecting a list
> of Namespace Identification Descriptor structures to be returned to
> the host for the namespace requested in the namespace identify
> command.
>
> This Namespace Identification Descriptor structure consists of the
> type of the namespace identifier, the length of the identifier and the
> actual identifier.
>
> Valid types are NGUID and UUID which we have saved in our nvme_ns
> structure if they have been configured via configfs. If no value has
> been assigened to one of these we return an "invalid opcode" back to
> the host to maintain backward compatibiliy with older implementations
> without Namespace Identify Descriptor list support.
>
> Also as the Namespace Identify Descriptor list is the only mandatory
> feature change between 1.2.1 and 1.3 we can bump the advertised
> version as well.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
> Reviewed-by: Hannes Reinecke <hare at suse.com>
> Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
> ---
> drivers/nvme/target/admin-cmd.c | 61 +++++++++++++++++++++++++++++++++++++++++
> drivers/nvme/target/core.c | 3 +-
> drivers/nvme/target/nvmet.h | 1 +
> 3 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index 96c144325443..6f9f0881aa2b 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -367,6 +367,64 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req)
> nvmet_req_complete(req, status);
> }
>
> +static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
> + void *id, off_t *off)
> +{
> + struct nvme_ns_id_desc desc = {
> + .nidt = type,
> + .nidl = len,
> + };
> + u16 status;
> +
> + status = nvmet_copy_to_sgl(req, *off, &desc, sizeof(desc));
> + if (status)
> + return status;
> + *off += sizeof(desc);
> +
> + status = nvmet_copy_to_sgl(req, *off, id, len);
> + if (status)
> + return status;
> + *off += len;
> +
> + return 0;
> +}
> +
> +static void nvmet_execute_identify_desclist(struct nvmet_req *req)
> +{
> + struct nvmet_ns *ns;
> + u16 status = 0;
> + off_t off = 0;
> +
> + ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
> + if (!ns) {
> + status = NVME_SC_INVALID_NS | NVME_SC_DNR;
> + goto out;
> + }
> +
> + if (memchr_inv(&ns->uuid, 0, sizeof(ns->uuid))) {
> + status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID,
> + NVME_NIDT_UUID_LEN,
> + &ns->uuid, &off);
> + if (status)
> + goto out_put_ns;
> + }
> + if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid))) {
> + status = nvmet_copy_ns_identifier(req, NVME_NIDT_NGUID,
> + NVME_NIDT_NGUID_LEN,
> + &ns->nguid, &off);
> + if (status)
> + goto out_put_ns;
> + }
> +
> + if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
Shouldn;t the third argument be NVME_IDENTIFY_DATA_SIZE - off in theory?
It's probably fine as is as the S/G helpers deal with overflows
gracefully, but still..
Otherwise looks fine:
Reviewed-by: Christoph Hellwig <hch at lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
2017-06-08 7:44 ` Christoph Hellwig
@ 2017-06-08 7:49 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-08 7:49 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Keith Busch, Hannes Reinecke, Max Gurtovoy,
Linux NVMe Mailinglist, Linux Kernel Mailinglist
On 06/08/2017 09:44 AM, Christoph Hellwig wrote:
>> + if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
>
> Shouldn;t the third argument be NVME_IDENTIFY_DATA_SIZE - off in theory?
> It's probably fine as is as the S/G helpers deal with overflows
> gracefully, but still..
Ahm yes. *doh*
Can you fix it up when applying?
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
@ 2017-06-08 7:49 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-08 7:49 UTC (permalink / raw)
On 06/08/2017 09:44 AM, Christoph Hellwig wrote:
>> + if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
>
> Shouldn;t the third argument be NVME_IDENTIFY_DATA_SIZE - off in theory?
> It's probably fine as is as the S/G helpers deal with overflows
> gracefully, but still..
Ahm yes. *doh*
Can you fix it up when applying?
--
Johannes Thumshirn Storage
jthumshirn at suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
2017-06-08 7:49 ` Johannes Thumshirn
@ 2017-06-08 7:52 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:52 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
On Thu, Jun 08, 2017 at 09:49:35AM +0200, Johannes Thumshirn wrote:
> On 06/08/2017 09:44 AM, Christoph Hellwig wrote:
> >> + if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
> >
> > Shouldn;t the third argument be NVME_IDENTIFY_DATA_SIZE - off in theory?
> > It's probably fine as is as the S/G helpers deal with overflows
> > gracefully, but still..
>
> Ahm yes. *doh*
>
> Can you fix it up when applying?
Sure.
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
@ 2017-06-08 7:52 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:52 UTC (permalink / raw)
On Thu, Jun 08, 2017@09:49:35AM +0200, Johannes Thumshirn wrote:
> On 06/08/2017 09:44 AM, Christoph Hellwig wrote:
> >> + if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
> >
> > Shouldn;t the third argument be NVME_IDENTIFY_DATA_SIZE - off in theory?
> > It's probably fine as is as the S/G helpers deal with overflows
> > gracefully, but still..
>
> Ahm yes. *doh*
>
> Can you fix it up when applying?
Sure.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
2017-06-08 7:52 ` Christoph Hellwig
@ 2017-06-08 7:55 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-08 7:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Keith Busch, Hannes Reinecke, Max Gurtovoy,
Linux NVMe Mailinglist, Linux Kernel Mailinglist
On 06/08/2017 09:52 AM, Christoph Hellwig wrote:
> On Thu, Jun 08, 2017 at 09:49:35AM +0200, Johannes Thumshirn wrote:
>> On 06/08/2017 09:44 AM, Christoph Hellwig wrote:
>>>> + if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
>>>
>>> Shouldn;t the third argument be NVME_IDENTIFY_DATA_SIZE - off in theory?
>>> It's probably fine as is as the S/G helpers deal with overflows
>>> gracefully, but still..
>>
>> Ahm yes. *doh*
>>
>> Can you fix it up when applying?
>
> Sure.
>
Btw, is the NVMf sleftests still avtively developed? If yes I have a
testcase for this series (not sure this qualifies for blktests)
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
@ 2017-06-08 7:55 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-08 7:55 UTC (permalink / raw)
On 06/08/2017 09:52 AM, Christoph Hellwig wrote:
> On Thu, Jun 08, 2017@09:49:35AM +0200, Johannes Thumshirn wrote:
>> On 06/08/2017 09:44 AM, Christoph Hellwig wrote:
>>>> + if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
>>>
>>> Shouldn;t the third argument be NVME_IDENTIFY_DATA_SIZE - off in theory?
>>> It's probably fine as is as the S/G helpers deal with overflows
>>> gracefully, but still..
>>
>> Ahm yes. *doh*
>>
>> Can you fix it up when applying?
>
> Sure.
>
Btw, is the NVMf sleftests still avtively developed? If yes I have a
testcase for this series (not sure this qualifies for blktests)
--
Johannes Thumshirn Storage
jthumshirn at suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
2017-06-08 7:55 ` Johannes Thumshirn
@ 2017-06-08 7:57 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:57 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist,
Chaitanya Kulkarni
On Thu, Jun 08, 2017 at 09:55:37AM +0200, Johannes Thumshirn wrote:
> Btw, is the NVMf sleftests still avtively developed? If yes I have a
> testcase for this series (not sure this qualifies for blktests)
As-is it's pretty dead, but Chaitanya has been looking into resurrecting
it in a modern form. Which reminds me that I owe him an offlist review
on what he has so far, let me get to that ASAP..
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
@ 2017-06-08 7:57 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:57 UTC (permalink / raw)
On Thu, Jun 08, 2017@09:55:37AM +0200, Johannes Thumshirn wrote:
> Btw, is the NVMf sleftests still avtively developed? If yes I have a
> testcase for this series (not sure this qualifies for blktests)
As-is it's pretty dead, but Chaitanya has been looking into resurrecting
it in a modern form. Which reminds me that I owe him an offlist review
on what he has so far, let me get to that ASAP..
^ permalink raw reply [flat|nested] 76+ messages in thread
[parent not found: <MWHPR04MB10088151BCB7D7A209A739E686CD0@MWHPR04MB1008.namprd04.prod.outlook.com>]
* Re: [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
[not found] ` <MWHPR04MB10088151BCB7D7A209A739E686CD0@MWHPR04MB1008.namprd04.prod.outlook.com>
@ 2017-06-12 16:19 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-12 16:19 UTC (permalink / raw)
To: Chaitanya Kulkarni
Cc: Johannes Thumshirn, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
On Mon, Jun 12, 2017 at 03:33:11AM +0000, Chaitanya Kulkarni wrote:
> Right now I'm keeping the framework under "${KDIR}/tools/testing/selftests/", please let me know if that is not a desirable directory.
Heh, Johannes and I just had a discussion on the test framework
locations last weekend over a beer.
Both of us agree that a separate repository would be much preferable,
mostly due to two reasons:
- it allows people that backport changes (e.g. distros) to always
run the latests test suite without having to backport it as well,
and have one test suite config for all branches
- ease of running it from a VM or initramfs instead of having to
download the whole kernel tree or copying things out
Last but not least it also seems a little easier to get started that
way.
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 05/10] nvmet: implement namespace identify descriptor list
@ 2017-06-12 16:19 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-12 16:19 UTC (permalink / raw)
On Mon, Jun 12, 2017@03:33:11AM +0000, Chaitanya Kulkarni wrote:
> Right now I'm keeping the framework under "${KDIR}/tools/testing/selftests/", please let me know if that is not a desirable directory.
Heh, Johannes and I just had a discussion on the test framework
locations last weekend over a beer.
Both of us agree that a separate repository would be much preferable,
mostly due to two reasons:
- it allows people that backport changes (e.g. distros) to always
run the latests test suite without having to backport it as well,
and have one test suite config for all branches
- ease of running it from a VM or initramfs instead of having to
download the whole kernel tree or copying things out
Last but not least it also seems a little easier to get started that
way.
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 06/10] nvmet: add uuid field to nvme_ns and populate via configfs
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
Add the UUID field from the NVMe Namespace Identification Descriptor
to the nvmet_ns structure and allow it's population via configfs.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
drivers/nvme/target/configfs.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index be8c800078e2..83bfe28fe7da 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -305,11 +305,41 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item,
CONFIGFS_ATTR(nvmet_ns_, device_path);
+static ssize_t nvmet_ns_device_uuid_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%pUb\n", &to_nvmet_ns(item)->uuid);
+}
+
+static ssize_t nvmet_ns_device_uuid_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct nvmet_ns *ns = to_nvmet_ns(item);
+ struct nvmet_subsys *subsys = ns->subsys;
+ int ret = 0;
+
+
+ mutex_lock(&subsys->lock);
+ if (ns->enabled) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+
+
+ if (uuid_parse(page, &ns->uuid))
+ ret = -EINVAL;
+
+out_unlock:
+ mutex_unlock(&subsys->lock);
+ return ret ? ret : count;
+}
+
static ssize_t nvmet_ns_device_nguid_show(struct config_item *item, char *page)
{
return sprintf(page, "%pUb\n", &to_nvmet_ns(item)->nguid);
}
+CONFIGFS_ATTR(nvmet_ns_, device_uuid);
+
static ssize_t nvmet_ns_device_nguid_store(struct config_item *item,
const char *page, size_t count)
{
@@ -379,6 +409,7 @@ CONFIGFS_ATTR(nvmet_ns_, enable);
static struct configfs_attribute *nvmet_ns_attrs[] = {
&nvmet_ns_attr_device_path,
&nvmet_ns_attr_device_nguid,
+ &nvmet_ns_attr_device_uuid,
&nvmet_ns_attr_enable,
NULL,
};
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 06/10] nvmet: add uuid field to nvme_ns and populate via configfs
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
Add the UUID field from the NVMe Namespace Identification Descriptor
to the nvmet_ns structure and allow it's population via configfs.
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
Reviewed-by: Hannes Reinecke <hare at suse.com>
---
drivers/nvme/target/configfs.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index be8c800078e2..83bfe28fe7da 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -305,11 +305,41 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item,
CONFIGFS_ATTR(nvmet_ns_, device_path);
+static ssize_t nvmet_ns_device_uuid_show(struct config_item *item, char *page)
+{
+ return sprintf(page, "%pUb\n", &to_nvmet_ns(item)->uuid);
+}
+
+static ssize_t nvmet_ns_device_uuid_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct nvmet_ns *ns = to_nvmet_ns(item);
+ struct nvmet_subsys *subsys = ns->subsys;
+ int ret = 0;
+
+
+ mutex_lock(&subsys->lock);
+ if (ns->enabled) {
+ ret = -EBUSY;
+ goto out_unlock;
+ }
+
+
+ if (uuid_parse(page, &ns->uuid))
+ ret = -EINVAL;
+
+out_unlock:
+ mutex_unlock(&subsys->lock);
+ return ret ? ret : count;
+}
+
static ssize_t nvmet_ns_device_nguid_show(struct config_item *item, char *page)
{
return sprintf(page, "%pUb\n", &to_nvmet_ns(item)->nguid);
}
+CONFIGFS_ATTR(nvmet_ns_, device_uuid);
+
static ssize_t nvmet_ns_device_nguid_store(struct config_item *item,
const char *page, size_t count)
{
@@ -379,6 +409,7 @@ CONFIGFS_ATTR(nvmet_ns_, enable);
static struct configfs_attribute *nvmet_ns_attrs[] = {
&nvmet_ns_attr_device_path,
&nvmet_ns_attr_device_nguid,
+ &nvmet_ns_attr_device_uuid,
&nvmet_ns_attr_enable,
NULL,
};
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* Re: [PATCH v6 06/10] nvmet: add uuid field to nvme_ns and populate via configfs
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-08 7:44 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:44 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
On Wed, Jun 07, 2017 at 11:45:33AM +0200, Johannes Thumshirn wrote:
> Add the UUID field from the NVMe Namespace Identification Descriptor
> to the nvmet_ns structure and allow it's population via configfs.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 06/10] nvmet: add uuid field to nvme_ns and populate via configfs
@ 2017-06-08 7:44 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:44 UTC (permalink / raw)
On Wed, Jun 07, 2017@11:45:33AM +0200, Johannes Thumshirn wrote:
> Add the UUID field from the NVMe Namespace Identification Descriptor
> to the nvmet_ns structure and allow it's population via configfs.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
> Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
> Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
> Reviewed-by: Hannes Reinecke <hare at suse.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch at lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 07/10] nvme: get list of namespace descriptors
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
If a target identifies itself as NVMe 1.3 compliant, try to get the
list of Namespace Identification Descriptors and populate the UUID,
NGUID and EUI64 fileds in the NVMe namespace structure with these
values.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
drivers/nvme/host/core.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/nvme/host/nvme.h | 1 +
2 files changed, 80 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 7b254be16887..34857191c8bd 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -643,6 +643,77 @@ int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
return error;
}
+static int nvme_identify_ns_descs(struct nvme_ns *ns, unsigned nsid)
+{
+ struct nvme_command c = { };
+ int status;
+ void *data;
+ int pos;
+ int len;
+
+ c.identify.opcode = nvme_admin_identify;
+ c.identify.nsid = cpu_to_le32(nsid);
+ c.identify.cns = NVME_ID_CNS_NS_DESC_LIST;
+
+ data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ status = nvme_submit_sync_cmd(ns->ctrl->admin_q, &c, data,
+ NVME_IDENTIFY_DATA_SIZE);
+ if (status)
+ goto free_data;
+
+ for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
+ struct nvme_ns_id_desc *cur = data + pos;
+
+ if (cur->nidl == 0)
+ break;
+
+ switch (cur->nidt) {
+ case NVME_NIDT_EUI64:
+ if (cur->nidl != NVME_NIDT_EUI64_LEN) {
+ dev_warn(ns->ctrl->device,
+ "ctrl returned bogus length: %d for NVME_NIDT_EUI64\n",
+ cur->nidl);
+ goto free_data;
+ }
+ len = NVME_NIDT_EUI64_LEN;
+ memcpy(ns->eui, data + pos + sizeof(*cur), len);
+ break;
+ case NVME_NIDT_NGUID:
+ if (cur->nidl != NVME_NIDT_NGUID_LEN) {
+ dev_warn(ns->ctrl->device,
+ "ctrl returned bogus length: %d for NVME_NIDT_NGUID\n",
+ cur->nidl);
+ goto free_data;
+ }
+ len = NVME_NIDT_NGUID_LEN;
+ memcpy(ns->nguid, data + pos + sizeof(*cur), len);
+ break;
+ case NVME_NIDT_UUID:
+ if (cur->nidl != NVME_NIDT_UUID_LEN) {
+ dev_warn(ns->ctrl->device,
+ "ctrl returned bogus length: %d for NVME_NIDT_UUID\n",
+ cur->nidl);
+ goto free_data;
+ }
+ len = NVME_NIDT_UUID_LEN;
+ memcpy(ns->uuid, data + pos + sizeof(*cur), len);
+ break;
+ default:
+ /* Skip unnkown types */
+ len = cur->nidl;
+ break;
+ }
+
+ len += sizeof(*cur);
+ }
+free_data:
+ kfree(data);
+ return status;
+}
+
static int nvme_identify_ns_list(struct nvme_ctrl *dev, unsigned nsid, __le32 *ns_list)
{
struct nvme_command c = { };
@@ -1017,6 +1088,14 @@ static int nvme_revalidate_ns(struct nvme_ns *ns, struct nvme_id_ns **id)
memcpy(ns->eui, (*id)->eui64, sizeof(ns->eui));
if (ns->ctrl->vs >= NVME_VS(1, 2, 0))
memcpy(ns->nguid, (*id)->nguid, sizeof(ns->nguid));
+ if (ns->ctrl->vs >= NVME_VS(1, 3, 0)) {
+ /* Don't treat error as fatal we potentially
+ * already have a NGUID or EUI-64
+ */
+ if (nvme_identify_ns_descs(ns, ns->ns_id))
+ dev_warn(ns->ctrl->device,
+ "%s: Identify Descriptors failed\n", __func__);
+ }
return 0;
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 5004f0c41397..7007521e8194 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -190,6 +190,7 @@ struct nvme_ns {
u8 eui[8];
u8 nguid[16];
+ u8 uuid[16];
unsigned ns_id;
int lba_shift;
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 07/10] nvme: get list of namespace descriptors
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
If a target identifies itself as NVMe 1.3 compliant, try to get the
list of Namespace Identification Descriptors and populate the UUID,
NGUID and EUI64 fileds in the NVMe namespace structure with these
values.
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Hannes Reinecke <hare at suse.com>
---
drivers/nvme/host/core.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/nvme/host/nvme.h | 1 +
2 files changed, 80 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 7b254be16887..34857191c8bd 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -643,6 +643,77 @@ int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
return error;
}
+static int nvme_identify_ns_descs(struct nvme_ns *ns, unsigned nsid)
+{
+ struct nvme_command c = { };
+ int status;
+ void *data;
+ int pos;
+ int len;
+
+ c.identify.opcode = nvme_admin_identify;
+ c.identify.nsid = cpu_to_le32(nsid);
+ c.identify.cns = NVME_ID_CNS_NS_DESC_LIST;
+
+ data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ status = nvme_submit_sync_cmd(ns->ctrl->admin_q, &c, data,
+ NVME_IDENTIFY_DATA_SIZE);
+ if (status)
+ goto free_data;
+
+ for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
+ struct nvme_ns_id_desc *cur = data + pos;
+
+ if (cur->nidl == 0)
+ break;
+
+ switch (cur->nidt) {
+ case NVME_NIDT_EUI64:
+ if (cur->nidl != NVME_NIDT_EUI64_LEN) {
+ dev_warn(ns->ctrl->device,
+ "ctrl returned bogus length: %d for NVME_NIDT_EUI64\n",
+ cur->nidl);
+ goto free_data;
+ }
+ len = NVME_NIDT_EUI64_LEN;
+ memcpy(ns->eui, data + pos + sizeof(*cur), len);
+ break;
+ case NVME_NIDT_NGUID:
+ if (cur->nidl != NVME_NIDT_NGUID_LEN) {
+ dev_warn(ns->ctrl->device,
+ "ctrl returned bogus length: %d for NVME_NIDT_NGUID\n",
+ cur->nidl);
+ goto free_data;
+ }
+ len = NVME_NIDT_NGUID_LEN;
+ memcpy(ns->nguid, data + pos + sizeof(*cur), len);
+ break;
+ case NVME_NIDT_UUID:
+ if (cur->nidl != NVME_NIDT_UUID_LEN) {
+ dev_warn(ns->ctrl->device,
+ "ctrl returned bogus length: %d for NVME_NIDT_UUID\n",
+ cur->nidl);
+ goto free_data;
+ }
+ len = NVME_NIDT_UUID_LEN;
+ memcpy(ns->uuid, data + pos + sizeof(*cur), len);
+ break;
+ default:
+ /* Skip unnkown types */
+ len = cur->nidl;
+ break;
+ }
+
+ len += sizeof(*cur);
+ }
+free_data:
+ kfree(data);
+ return status;
+}
+
static int nvme_identify_ns_list(struct nvme_ctrl *dev, unsigned nsid, __le32 *ns_list)
{
struct nvme_command c = { };
@@ -1017,6 +1088,14 @@ static int nvme_revalidate_ns(struct nvme_ns *ns, struct nvme_id_ns **id)
memcpy(ns->eui, (*id)->eui64, sizeof(ns->eui));
if (ns->ctrl->vs >= NVME_VS(1, 2, 0))
memcpy(ns->nguid, (*id)->nguid, sizeof(ns->nguid));
+ if (ns->ctrl->vs >= NVME_VS(1, 3, 0)) {
+ /* Don't treat error as fatal we potentially
+ * already have a NGUID or EUI-64
+ */
+ if (nvme_identify_ns_descs(ns, ns->ns_id))
+ dev_warn(ns->ctrl->device,
+ "%s: Identify Descriptors failed\n", __func__);
+ }
return 0;
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 5004f0c41397..7007521e8194 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -190,6 +190,7 @@ struct nvme_ns {
u8 eui[8];
u8 nguid[16];
+ u8 uuid[16];
unsigned ns_id;
int lba_shift;
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* Re: [PATCH v6 07/10] nvme: get list of namespace descriptors
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-08 7:46 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:46 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
On Wed, Jun 07, 2017 at 11:45:34AM +0200, Johannes Thumshirn wrote:
> If a target identifies itself as NVMe 1.3 compliant, try to get the
> list of Namespace Identification Descriptors and populate the UUID,
> NGUID and EUI64 fileds in the NVMe namespace structure with these
> values.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 07/10] nvme: get list of namespace descriptors
@ 2017-06-08 7:46 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:46 UTC (permalink / raw)
On Wed, Jun 07, 2017@11:45:34AM +0200, Johannes Thumshirn wrote:
> If a target identifies itself as NVMe 1.3 compliant, try to get the
> list of Namespace Identification Descriptors and populate the UUID,
> NGUID and EUI64 fileds in the NVMe namespace structure with these
> values.
>
> Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
> Reviewed-by: Hannes Reinecke <hare at suse.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch at lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 08/10] nvme: provide UUID value to userspace
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
Now that we have a way for getting the UUID from a target, provide it
to userspace as well.
Unfortunately there is already a sysfs attribute called UUID which is
a misnomer as it holds the NGUID value. So instead of creating yet
another wrong name, create a new 'nguid' sysfs attribute for the
NGUID. For the UUID attribute add a check wheter the namespace has a
UUID assigned to it and return this or return the NGUID to maintain
backwards compatibility. This should give userspace a chance to catch
up.
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Sagi Grimberg <sagi@rimberg.me>
---
drivers/nvme/host/core.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 34857191c8bd..462187a4047b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1879,11 +1879,28 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR(wwid, S_IRUGO, wwid_show, NULL);
+static ssize_t nguid_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
+ return sprintf(buf, "%pU\n", ns->nguid);
+}
+static DEVICE_ATTR(nguid, S_IRUGO, nguid_show, NULL);
+
static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
- return sprintf(buf, "%pU\n", ns->nguid);
+
+ /* For backward compatibility expose the NGUID to userspace if
+ * we have no UUID set
+ */
+ if (!memchr_inv(ns->uuid, 0, sizeof(ns->uuid))) {
+ printk_ratelimited(KERN_WARNING
+ "No UUID available providing old NGUID\n");
+ return sprintf(buf, "%pU\n", ns->nguid);
+ }
+ return sprintf(buf, "%pU\n", ns->uuid);
}
static DEVICE_ATTR(uuid, S_IRUGO, uuid_show, NULL);
@@ -1906,6 +1923,7 @@ static DEVICE_ATTR(nsid, S_IRUGO, nsid_show, NULL);
static struct attribute *nvme_ns_attrs[] = {
&dev_attr_wwid.attr,
&dev_attr_uuid.attr,
+ &dev_attr_nguid.attr,
&dev_attr_eui.attr,
&dev_attr_nsid.attr,
NULL,
@@ -1918,6 +1936,11 @@ static umode_t nvme_ns_attrs_are_visible(struct kobject *kobj,
struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
if (a == &dev_attr_uuid.attr) {
+ if (!memchr_inv(ns->uuid, 0, sizeof(ns->uuid)) ||
+ !memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
+ return 0;
+ }
+ if (a == &dev_attr_nguid.attr) {
if (!memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
return 0;
}
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 08/10] nvme: provide UUID value to userspace
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
Now that we have a way for getting the UUID from a target, provide it
to userspace as well.
Unfortunately there is already a sysfs attribute called UUID which is
a misnomer as it holds the NGUID value. So instead of creating yet
another wrong name, create a new 'nguid' sysfs attribute for the
NGUID. For the UUID attribute add a check wheter the namespace has a
UUID assigned to it and return this or return the NGUID to maintain
backwards compatibility. This should give userspace a chance to catch
up.
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Sagi Grimberg <sagi at rimberg.me>
---
drivers/nvme/host/core.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 34857191c8bd..462187a4047b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1879,11 +1879,28 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR(wwid, S_IRUGO, wwid_show, NULL);
+static ssize_t nguid_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
+ return sprintf(buf, "%pU\n", ns->nguid);
+}
+static DEVICE_ATTR(nguid, S_IRUGO, nguid_show, NULL);
+
static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
- return sprintf(buf, "%pU\n", ns->nguid);
+
+ /* For backward compatibility expose the NGUID to userspace if
+ * we have no UUID set
+ */
+ if (!memchr_inv(ns->uuid, 0, sizeof(ns->uuid))) {
+ printk_ratelimited(KERN_WARNING
+ "No UUID available providing old NGUID\n");
+ return sprintf(buf, "%pU\n", ns->nguid);
+ }
+ return sprintf(buf, "%pU\n", ns->uuid);
}
static DEVICE_ATTR(uuid, S_IRUGO, uuid_show, NULL);
@@ -1906,6 +1923,7 @@ static DEVICE_ATTR(nsid, S_IRUGO, nsid_show, NULL);
static struct attribute *nvme_ns_attrs[] = {
&dev_attr_wwid.attr,
&dev_attr_uuid.attr,
+ &dev_attr_nguid.attr,
&dev_attr_eui.attr,
&dev_attr_nsid.attr,
NULL,
@@ -1918,6 +1936,11 @@ static umode_t nvme_ns_attrs_are_visible(struct kobject *kobj,
struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
if (a == &dev_attr_uuid.attr) {
+ if (!memchr_inv(ns->uuid, 0, sizeof(ns->uuid)) ||
+ !memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
+ return 0;
+ }
+ if (a == &dev_attr_nguid.attr) {
if (!memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
return 0;
}
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 09/10] nvmet: allow overriding the NVMe VS via configfs
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-07 9:45 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch
Cc: Hannes Reinecke, Max Gurtovoy, Linux NVMe Mailinglist,
Linux Kernel Mailinglist, Johannes Thumshirn
Allow overriding the announced NVMe Version of a via configfs.
This is particularly helpful when debugging new features for the host
or target side without bumping the hard coded version (as the target
might not be fully compliant to the announced version yet).
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Guan Junxiong <guanjunxiong@huawei.com>
---
drivers/nvme/target/configfs.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/nvme.h | 4 ++++
2 files changed, 41 insertions(+)
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 83bfe28fe7da..a358ecd93e11 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -650,8 +650,45 @@ static ssize_t nvmet_subsys_attr_allow_any_host_store(struct config_item *item,
CONFIGFS_ATTR(nvmet_subsys_, attr_allow_any_host);
+static ssize_t nvmet_subsys_version_show(struct config_item *item,
+ char *page)
+{
+ struct nvmet_subsys *subsys = to_subsys(item);
+
+ if (NVME_TERTIARY(subsys->ver))
+ return snprintf(page, PAGE_SIZE, "%d.%d.%d\n",
+ (int)NVME_MAJOR(subsys->ver),
+ (int)NVME_MINOR(subsys->ver),
+ (int)NVME_TERTIARY(subsys->ver));
+ else
+ return snprintf(page, PAGE_SIZE, "%d.%d\n",
+ (int)NVME_MAJOR(subsys->ver),
+ (int)NVME_MINOR(subsys->ver));
+}
+
+static ssize_t nvmet_subsys_version_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct nvmet_subsys *subsys = to_subsys(item);
+ int major, minor, tertiary = 0;
+ int ret;
+
+
+ ret = sscanf(page, "%d.%d.%d\n", &major, &minor, &tertiary);
+ if (ret != 2 && ret != 3)
+ return -EINVAL;
+
+ down_write(&nvmet_config_sem);
+ subsys->ver = NVME_VS(major, minor, tertiary);
+ up_write(&nvmet_config_sem);
+
+ return count;
+}
+CONFIGFS_ATTR(nvmet_subsys_, version);
+
static struct configfs_attribute *nvmet_subsys_attrs[] = {
&nvmet_subsys_attr_attr_allow_any_host,
+ &nvmet_subsys_attr_version,
NULL,
};
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 01d75c2b32a6..350c32fca1f5 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -1070,4 +1070,8 @@ struct nvme_completion {
#define NVME_VS(major, minor, tertiary) \
(((major) << 16) | ((minor) << 8) | (tertiary))
+#define NVME_MAJOR(ver) ((ver) >> 16)
+#define NVME_MINOR(ver) (((ver) >> 8) & 0xff)
+#define NVME_TERTIARY(ver) ((ver) & 0xff)
+
#endif /* _LINUX_NVME_H */
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH v6 09/10] nvmet: allow overriding the NVMe VS via configfs
@ 2017-06-07 9:45 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-07 9:45 UTC (permalink / raw)
Allow overriding the announced NVMe Version of a via configfs.
This is particularly helpful when debugging new features for the host
or target side without bumping the hard coded version (as the target
might not be fully compliant to the announced version yet).
Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Hannes Reinecke <hare at suse.com>
Reviewed-by: Guan Junxiong <guanjunxiong at huawei.com>
---
drivers/nvme/target/configfs.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/nvme.h | 4 ++++
2 files changed, 41 insertions(+)
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 83bfe28fe7da..a358ecd93e11 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -650,8 +650,45 @@ static ssize_t nvmet_subsys_attr_allow_any_host_store(struct config_item *item,
CONFIGFS_ATTR(nvmet_subsys_, attr_allow_any_host);
+static ssize_t nvmet_subsys_version_show(struct config_item *item,
+ char *page)
+{
+ struct nvmet_subsys *subsys = to_subsys(item);
+
+ if (NVME_TERTIARY(subsys->ver))
+ return snprintf(page, PAGE_SIZE, "%d.%d.%d\n",
+ (int)NVME_MAJOR(subsys->ver),
+ (int)NVME_MINOR(subsys->ver),
+ (int)NVME_TERTIARY(subsys->ver));
+ else
+ return snprintf(page, PAGE_SIZE, "%d.%d\n",
+ (int)NVME_MAJOR(subsys->ver),
+ (int)NVME_MINOR(subsys->ver));
+}
+
+static ssize_t nvmet_subsys_version_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct nvmet_subsys *subsys = to_subsys(item);
+ int major, minor, tertiary = 0;
+ int ret;
+
+
+ ret = sscanf(page, "%d.%d.%d\n", &major, &minor, &tertiary);
+ if (ret != 2 && ret != 3)
+ return -EINVAL;
+
+ down_write(&nvmet_config_sem);
+ subsys->ver = NVME_VS(major, minor, tertiary);
+ up_write(&nvmet_config_sem);
+
+ return count;
+}
+CONFIGFS_ATTR(nvmet_subsys_, version);
+
static struct configfs_attribute *nvmet_subsys_attrs[] = {
&nvmet_subsys_attr_attr_allow_any_host,
+ &nvmet_subsys_attr_version,
NULL,
};
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 01d75c2b32a6..350c32fca1f5 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -1070,4 +1070,8 @@ struct nvme_completion {
#define NVME_VS(major, minor, tertiary) \
(((major) << 16) | ((minor) << 8) | (tertiary))
+#define NVME_MAJOR(ver) ((ver) >> 16)
+#define NVME_MINOR(ver) (((ver) >> 8) & 0xff)
+#define NVME_TERTIARY(ver) ((ver) & 0xff)
+
#endif /* _LINUX_NVME_H */
--
2.12.3
^ permalink raw reply related [flat|nested] 76+ messages in thread
* Re: [PATCH v6 09/10] nvmet: allow overriding the NVMe VS via configfs
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-08 7:47 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-08 7:47 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-07 9:45 ` Johannes Thumshirn
@ 2017-06-15 16:31 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-15 16:31 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
Max Gurtovoy, Linux NVMe Mailinglist, Linux Kernel Mailinglist
On Wed, Jun 07, 2017 at 11:45:27AM +0200, Johannes Thumshirn wrote:
> A patch for nvmetcli and nvme-cli will follow shortly.
Can you send them out? Especially as I want to debug why I don't seem
to get a uuid attribute with nvme-loop despite the fact that we should
always have one.
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-15 16:31 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-15 16:31 UTC (permalink / raw)
On Wed, Jun 07, 2017@11:45:27AM +0200, Johannes Thumshirn wrote:
> A patch for nvmetcli and nvme-cli will follow shortly.
Can you send them out? Especially as I want to debug why I don't seem
to get a uuid attribute with nvme-loop despite the fact that we should
always have one.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-15 16:31 ` Christoph Hellwig
@ 2017-06-16 8:20 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-16 8:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Linux Kernel Mailinglist, Linux NVMe Mailinglist,
Keith Busch, Hannes Reinecke, Max Gurtovoy
On 06/15/2017 06:31 PM, Christoph Hellwig wrote:
> On Wed, Jun 07, 2017 at 11:45:27AM +0200, Johannes Thumshirn wrote:
>> A patch for nvmetcli and nvme-cli will follow shortly.
>
> Can you send them out? Especially as I want to debug why I don't seem
> to get a uuid attribute with nvme-loop despite the fact that we should
> always have one.
Strange, as I tested with nvme-loop all the time...
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-16 8:20 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-16 8:20 UTC (permalink / raw)
On 06/15/2017 06:31 PM, Christoph Hellwig wrote:
> On Wed, Jun 07, 2017@11:45:27AM +0200, Johannes Thumshirn wrote:
>> A patch for nvmetcli and nvme-cli will follow shortly.
>
> Can you send them out? Especially as I want to debug why I don't seem
> to get a uuid attribute with nvme-loop despite the fact that we should
> always have one.
Strange, as I tested with nvme-loop all the time...
--
Johannes Thumshirn Storage
jthumshirn at suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-16 8:20 ` Johannes Thumshirn
@ 2017-06-16 9:40 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-16 9:40 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Linux Kernel Mailinglist,
Linux NVMe Mailinglist, Keith Busch, Hannes Reinecke,
Max Gurtovoy
On Fri, Jun 16, 2017 at 10:20:04AM +0200, Johannes Thumshirn wrote:
> Strange, as I tested with nvme-loop all the time...
Yeah, it's actually there, but for some reason find on sysfs
behaves strange:
root@testvm:~# find /sys -name uuid
root@testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
6665a65b-f42f-469b-800e-a047238649eb
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-16 9:40 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-16 9:40 UTC (permalink / raw)
On Fri, Jun 16, 2017@10:20:04AM +0200, Johannes Thumshirn wrote:
> Strange, as I tested with nvme-loop all the time...
Yeah, it's actually there, but for some reason find on sysfs
behaves strange:
root at testvm:~# find /sys -name uuid
root at testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
6665a65b-f42f-469b-800e-a047238649eb
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-16 9:40 ` Christoph Hellwig
@ 2017-06-16 9:48 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-16 9:48 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Linux Kernel Mailinglist, Linux NVMe Mailinglist,
Keith Busch, Hannes Reinecke, Max Gurtovoy
On 06/16/2017 11:40 AM, Christoph Hellwig wrote:
> On Fri, Jun 16, 2017 at 10:20:04AM +0200, Johannes Thumshirn wrote:
>> Strange, as I tested with nvme-loop all the time...
>
> Yeah, it's actually there, but for some reason find on sysfs
> behaves strange:
>
> root@testvm:~# find /sys -name uuid
> root@testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
> 6665a65b-f42f-469b-800e-a047238649eb
Wasn't there something that find on sysfs isn't reliable?
Anyways, my shock's gone now.
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-16 9:48 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-16 9:48 UTC (permalink / raw)
On 06/16/2017 11:40 AM, Christoph Hellwig wrote:
> On Fri, Jun 16, 2017@10:20:04AM +0200, Johannes Thumshirn wrote:
>> Strange, as I tested with nvme-loop all the time...
>
> Yeah, it's actually there, but for some reason find on sysfs
> behaves strange:
>
> root at testvm:~# find /sys -name uuid
> root at testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
> 6665a65b-f42f-469b-800e-a047238649eb
Wasn't there something that find on sysfs isn't reliable?
Anyways, my shock's gone now.
--
Johannes Thumshirn Storage
jthumshirn at suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-16 9:48 ` Johannes Thumshirn
@ 2017-06-16 9:58 ` Christoph Hellwig
-1 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-16 9:58 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Christoph Hellwig, Sagi Grimberg, Linux Kernel Mailinglist,
Linux NVMe Mailinglist, Keith Busch, Hannes Reinecke,
Max Gurtovoy
On Fri, Jun 16, 2017 at 11:48:32AM +0200, Johannes Thumshirn wrote:
> On 06/16/2017 11:40 AM, Christoph Hellwig wrote:
> > On Fri, Jun 16, 2017 at 10:20:04AM +0200, Johannes Thumshirn wrote:
> >> Strange, as I tested with nvme-loop all the time...
> >
> > Yeah, it's actually there, but for some reason find on sysfs
> > behaves strange:
> >
> > root@testvm:~# find /sys -name uuid
> > root@testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
> > 6665a65b-f42f-469b-800e-a047238649eb
>
> Wasn't there something that find on sysfs isn't reliable?
Looks like it. Which is a pitty.
> Anyways, my shock's gone now.
Heh. Now we just need the nvme-cli patches to verify it independently :)
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-16 9:58 ` Christoph Hellwig
0 siblings, 0 replies; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-16 9:58 UTC (permalink / raw)
On Fri, Jun 16, 2017@11:48:32AM +0200, Johannes Thumshirn wrote:
> On 06/16/2017 11:40 AM, Christoph Hellwig wrote:
> > On Fri, Jun 16, 2017@10:20:04AM +0200, Johannes Thumshirn wrote:
> >> Strange, as I tested with nvme-loop all the time...
> >
> > Yeah, it's actually there, but for some reason find on sysfs
> > behaves strange:
> >
> > root at testvm:~# find /sys -name uuid
> > root at testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
> > 6665a65b-f42f-469b-800e-a047238649eb
>
> Wasn't there something that find on sysfs isn't reliable?
Looks like it. Which is a pitty.
> Anyways, my shock's gone now.
Heh. Now we just need the nvme-cli patches to verify it independently :)
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-16 9:58 ` Christoph Hellwig
@ 2017-06-16 9:59 ` Johannes Thumshirn
-1 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-16 9:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sagi Grimberg, Linux Kernel Mailinglist, Linux NVMe Mailinglist,
Keith Busch, Hannes Reinecke, Max Gurtovoy
On 06/16/2017 11:58 AM, Christoph Hellwig wrote:
> Heh. Now we just need the nvme-cli patches to verify it independently :)
I'm on it ;-)
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-16 9:59 ` Johannes Thumshirn
0 siblings, 0 replies; 76+ messages in thread
From: Johannes Thumshirn @ 2017-06-16 9:59 UTC (permalink / raw)
On 06/16/2017 11:58 AM, Christoph Hellwig wrote:
> Heh. Now we just need the nvme-cli patches to verify it independently :)
I'm on it ;-)
--
Johannes Thumshirn Storage
jthumshirn at suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-16 9:58 ` Christoph Hellwig
@ 2017-06-16 13:28 ` Linus Torvalds
-1 siblings, 0 replies; 76+ messages in thread
From: Linus Torvalds @ 2017-06-16 13:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Johannes Thumshirn, Sagi Grimberg, Linux Kernel Mailinglist,
Linux NVMe Mailinglist, Keith Busch, Hannes Reinecke,
Max Gurtovoy
On Fri, Jun 16, 2017 at 6:58 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Fri, Jun 16, 2017 at 11:48:32AM +0200, Johannes Thumshirn wrote:
>> >
>> > Yeah, it's actually there, but for some reason find on sysfs
>> > behaves strange:
>> >
>> > root@testvm:~# find /sys -name uuid
>> > root@testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
>> > 6665a65b-f42f-469b-800e-a047238649eb
>>
>> Wasn't there something that find on sysfs isn't reliable?
>
> Looks like it. Which is a pitty.
Hmm. The *traditional* reason for this particular 'find' oddity is
that find has an optimization which will look at the nlink count of a
directory to decide how many subdirectories it can have.
So when 'find' then traverses the directory tree, once it has found
all the subdirectories it expects, it will stop traversing any further
subdirectories.
The reason for this is that it can then avoid doing the 'lstat()' on
each directory entry to even figure out what kind of file it is (ie
directory vs regular file etc). I forget the exact rules, but it
basically depends on nlink being "2+umber of subdirectories". I wonder
if the sysfs code gets this wrong for some cases.
All the directories I have on the laptop I'm on right now get it
right, but maybe nvme triggers something.
You can check with some silly shell scripts, and do things like
stat -c %h /sys/class/nvme
and then compare that to the number of subdirectories (the link count
should be 2 higher - the parent entry and the '.' entry).
The traditional *fix* for this is to just set "nlink" to 1 for a
directory, which tells 'find' to not use this optimization. That's
what filesystems like VFAT do, that don't count subdirectories. But
sysfs should get the directory count right.
I can't imagine any other reason why 'find' would screw up.
Linus
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-16 13:28 ` Linus Torvalds
0 siblings, 0 replies; 76+ messages in thread
From: Linus Torvalds @ 2017-06-16 13:28 UTC (permalink / raw)
On Fri, Jun 16, 2017@6:58 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Fri, Jun 16, 2017@11:48:32AM +0200, Johannes Thumshirn wrote:
>> >
>> > Yeah, it's actually there, but for some reason find on sysfs
>> > behaves strange:
>> >
>> > root at testvm:~# find /sys -name uuid
>> > root at testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
>> > 6665a65b-f42f-469b-800e-a047238649eb
>>
>> Wasn't there something that find on sysfs isn't reliable?
>
> Looks like it. Which is a pitty.
Hmm. The *traditional* reason for this particular 'find' oddity is
that find has an optimization which will look at the nlink count of a
directory to decide how many subdirectories it can have.
So when 'find' then traverses the directory tree, once it has found
all the subdirectories it expects, it will stop traversing any further
subdirectories.
The reason for this is that it can then avoid doing the 'lstat()' on
each directory entry to even figure out what kind of file it is (ie
directory vs regular file etc). I forget the exact rules, but it
basically depends on nlink being "2+umber of subdirectories". I wonder
if the sysfs code gets this wrong for some cases.
All the directories I have on the laptop I'm on right now get it
right, but maybe nvme triggers something.
You can check with some silly shell scripts, and do things like
stat -c %h /sys/class/nvme
and then compare that to the number of subdirectories (the link count
should be 2 higher - the parent entry and the '.' entry).
The traditional *fix* for this is to just set "nlink" to 1 for a
directory, which tells 'find' to not use this optimization. That's
what filesystems like VFAT do, that don't count subdirectories. But
sysfs should get the directory count right.
I can't imagine any other reason why 'find' would screw up.
Linus
^ permalink raw reply [flat|nested] 76+ messages in thread
* odd sysfs find behavior, was: Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-16 13:28 ` Linus Torvalds
(?)
@ 2017-06-17 12:08 ` Christoph Hellwig
2017-06-18 1:12 ` Greg Kroah-Hartman
-1 siblings, 1 reply; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-17 12:08 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Tejun Heo, Greg Kroah-Hartman, linux-fsdevel
On Fri, Jun 16, 2017 at 10:28:28PM +0900, Linus Torvalds wrote:
> Hmm. The *traditional* reason for this particular 'find' oddity is
> that find has an optimization which will look at the nlink count of a
> directory to decide how many subdirectories it can have.
It looks like sysfs maintains i_nlink properly as far as I can tell.
But I noticed another things that's even more weird:
root@testvm:~/nvmetcli# find /sys -name uuid
root@testvm:~/nvmetcli# find /sys -name uuid
/sys/devices/virtual/nvme-fabrics/ctl/nvme2/nvme2n1/uuid
so just repeating the find a second time makes it work! Looks like
there is some sort of lazy population scheme in sysfs..
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: odd sysfs find behavior, was: Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-17 12:08 ` odd sysfs find behavior, was: " Christoph Hellwig
@ 2017-06-18 1:12 ` Greg Kroah-Hartman
2017-06-18 7:32 ` Christoph Hellwig
0 siblings, 1 reply; 76+ messages in thread
From: Greg Kroah-Hartman @ 2017-06-18 1:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Linus Torvalds, Tejun Heo, linux-fsdevel
On Sat, Jun 17, 2017 at 02:08:14PM +0200, Christoph Hellwig wrote:
> On Fri, Jun 16, 2017 at 10:28:28PM +0900, Linus Torvalds wrote:
> > Hmm. The *traditional* reason for this particular 'find' oddity is
> > that find has an optimization which will look at the nlink count of a
> > directory to decide how many subdirectories it can have.
>
> It looks like sysfs maintains i_nlink properly as far as I can tell.
> But I noticed another things that's even more weird:
>
> root@testvm:~/nvmetcli# find /sys -name uuid
> root@testvm:~/nvmetcli# find /sys -name uuid
> /sys/devices/virtual/nvme-fabrics/ctl/nvme2/nvme2n1/uuid
>
> so just repeating the find a second time makes it work! Looks like
> there is some sort of lazy population scheme in sysfs..
There is a lazy population scheme in sysfs, but that means that things
are only populated in memory when we actually look for them, not the
second time around only :)
I just tried this on my laptop which does have a nvme block device in
it, and it worked just fine both times.
As Linus said, a strace would be great.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: odd sysfs find behavior, was: Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-18 1:12 ` Greg Kroah-Hartman
@ 2017-06-18 7:32 ` Christoph Hellwig
[not found] ` <20170618073305.GB25797@lst.de>
0 siblings, 1 reply; 76+ messages in thread
From: Christoph Hellwig @ 2017-06-18 7:32 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Christoph Hellwig, Linus Torvalds, Tejun Heo, linux-fsdevel
On Sun, Jun 18, 2017 at 09:12:50AM +0800, Greg Kroah-Hartman wrote:
> >
> > root@testvm:~/nvmetcli# find /sys -name uuid
> > root@testvm:~/nvmetcli# find /sys -name uuid
> > /sys/devices/virtual/nvme-fabrics/ctl/nvme2/nvme2n1/uuid
> >
> > so just repeating the find a second time makes it work! Looks like
> > there is some sort of lazy population scheme in sysfs..
>
> There is a lazy population scheme in sysfs, but that means that things
> are only populated in memory when we actually look for them, not the
> second time around only :)
>
> I just tried this on my laptop which does have a nvme block device in
> it, and it worked just fine both times.
Note that the search is for an attribute in the sysfs dir, but trying
to search for the device it doesn't make any difference.
> As Linus said, a strace would be great.
Both straces attached (gzipped due to size). Note that this on a system
with four NVMe devices, two of which are nvme-loop devices that have the
uuid attribute.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
2017-06-16 13:28 ` Linus Torvalds
@ 2017-06-22 0:06 ` Eric W. Biederman
-1 siblings, 0 replies; 76+ messages in thread
From: Eric W. Biederman @ 2017-06-22 0:06 UTC (permalink / raw)
To: Linus Torvalds
Cc: Christoph Hellwig, Johannes Thumshirn, Sagi Grimberg,
Linux Kernel Mailinglist, Linux NVMe Mailinglist, Keith Busch,
Hannes Reinecke, Max Gurtovoy
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Fri, Jun 16, 2017 at 6:58 PM, Christoph Hellwig <hch@lst.de> wrote:
>> On Fri, Jun 16, 2017 at 11:48:32AM +0200, Johannes Thumshirn wrote:
>>> >
>>> > Yeah, it's actually there, but for some reason find on sysfs
>>> > behaves strange:
>>> >
>>> > root@testvm:~# find /sys -name uuid
>>> > root@testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
>>> > 6665a65b-f42f-469b-800e-a047238649eb
>>>
>>> Wasn't there something that find on sysfs isn't reliable?
>>
>> Looks like it. Which is a pitty.
>
> Hmm. The *traditional* reason for this particular 'find' oddity is
> that find has an optimization which will look at the nlink count of a
> directory to decide how many subdirectories it can have.
>
> So when 'find' then traverses the directory tree, once it has found
> all the subdirectories it expects, it will stop traversing any further
> subdirectories.
>
> The reason for this is that it can then avoid doing the 'lstat()' on
> each directory entry to even figure out what kind of file it is (ie
> directory vs regular file etc). I forget the exact rules, but it
> basically depends on nlink being "2+umber of subdirectories". I wonder
> if the sysfs code gets this wrong for some cases.
>
> All the directories I have on the laptop I'm on right now get it
> right, but maybe nvme triggers something.
>
> You can check with some silly shell scripts, and do things like
>
> stat -c %h /sys/class/nvme
>
> and then compare that to the number of subdirectories (the link count
> should be 2 higher - the parent entry and the '.' entry).
>
> The traditional *fix* for this is to just set "nlink" to 1 for a
> directory, which tells 'find' to not use this optimization. That's
> what filesystems like VFAT do, that don't count subdirectories. But
> sysfs should get the directory count right.
>
> I can't imagine any other reason why 'find' would screw up.
Definitely worth looking at.
There is only one code path for that in kernfs/sysfs so I don't think
you can mess that up.
I suspect find is getting confused by something more subtle. Perhaps a
symlink. /sys/class/nvme/ ought to be be filled with symlinks to other
locations in sysfs. Which could be part of the challenge.
Perhaps find /sys/ behaves differently than find /sys..
It might be worth running "find /sys | grep uuid" and see if your file
turns up.
After the work I did cleaning up sysfs to support network namespaces
find really ought to work properly on sysfs. inotify is likely to have
problems but find should work.
Eric
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH v6 00/10] Implement NVMe Namespace Descriptor Identification
@ 2017-06-22 0:06 ` Eric W. Biederman
0 siblings, 0 replies; 76+ messages in thread
From: Eric W. Biederman @ 2017-06-22 0:06 UTC (permalink / raw)
Linus Torvalds <torvalds at linux-foundation.org> writes:
> On Fri, Jun 16, 2017@6:58 PM, Christoph Hellwig <hch@lst.de> wrote:
>> On Fri, Jun 16, 2017@11:48:32AM +0200, Johannes Thumshirn wrote:
>>> >
>>> > Yeah, it's actually there, but for some reason find on sysfs
>>> > behaves strange:
>>> >
>>> > root at testvm:~# find /sys -name uuid
>>> > root at testvm:~# cat /sys/class/nvme/nvme2/nvme2n1/uuid
>>> > 6665a65b-f42f-469b-800e-a047238649eb
>>>
>>> Wasn't there something that find on sysfs isn't reliable?
>>
>> Looks like it. Which is a pitty.
>
> Hmm. The *traditional* reason for this particular 'find' oddity is
> that find has an optimization which will look at the nlink count of a
> directory to decide how many subdirectories it can have.
>
> So when 'find' then traverses the directory tree, once it has found
> all the subdirectories it expects, it will stop traversing any further
> subdirectories.
>
> The reason for this is that it can then avoid doing the 'lstat()' on
> each directory entry to even figure out what kind of file it is (ie
> directory vs regular file etc). I forget the exact rules, but it
> basically depends on nlink being "2+umber of subdirectories". I wonder
> if the sysfs code gets this wrong for some cases.
>
> All the directories I have on the laptop I'm on right now get it
> right, but maybe nvme triggers something.
>
> You can check with some silly shell scripts, and do things like
>
> stat -c %h /sys/class/nvme
>
> and then compare that to the number of subdirectories (the link count
> should be 2 higher - the parent entry and the '.' entry).
>
> The traditional *fix* for this is to just set "nlink" to 1 for a
> directory, which tells 'find' to not use this optimization. That's
> what filesystems like VFAT do, that don't count subdirectories. But
> sysfs should get the directory count right.
>
> I can't imagine any other reason why 'find' would screw up.
Definitely worth looking at.
There is only one code path for that in kernfs/sysfs so I don't think
you can mess that up.
I suspect find is getting confused by something more subtle. Perhaps a
symlink. /sys/class/nvme/ ought to be be filled with symlinks to other
locations in sysfs. Which could be part of the challenge.
Perhaps find /sys/ behaves differently than find /sys..
It might be worth running "find /sys | grep uuid" and see if your file
turns up.
After the work I did cleaning up sysfs to support network namespaces
find really ought to work properly on sysfs. inotify is likely to have
problems but find should work.
Eric
^ permalink raw reply [flat|nested] 76+ messages in thread