Add support for TP8010 explicit registration #241
Conversation
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
|
Please split this up into smaller patches. |
| return PyUnicode_FromFormat("Result:0x%04x, Status:0x%04x - %s", result, status, nvme_status_to_string(status, false)); | ||
| } | ||
|
|
||
| // On success, return None |
There was a problem hiding this comment.
I see there are both comment styles in this file. Can we agree on one and just use that one?
| @@ -964,3 +965,255 @@ char *nvmf_hostid_from_file() | |||
| { | |||
| return nvmf_read_file(nvmf_hostid_file, NVMF_HOSTID_SIZE); | |||
| } | |||
|
|
|||
| /** | |||
| * nvmf_get_tel() - Calculate the amount of memory needed for a Discovery | |||
There was a problem hiding this comment.
The short description needs to be on one line. So this text stops right at 'Discovery'.
| } | ||
|
|
||
| /** | ||
| * nvmf_fill_die() - Fill the Discovery Information Entry pointed to by |
| size_t symname_len; | ||
| struct nvmf_ext_attr *exat; | ||
|
|
||
| die->tel = htole32(tel); |
There was a problem hiding this comment.
Wouldn't it make sense to stick using the cpu_to_le32() helpers?
|
|
||
| args.data_len = tdl; | ||
| args.data = dim; | ||
| ret = nvme_send_dim_command(&args); |
There was a problem hiding this comment.
returns -1 and sets errno. So your comment above doesn't match which this here.
| { | ||
| FILE *file = fopen(fname, "re"); | ||
| if (file) { | ||
| char *p = fgets(buffer, *bufsz, file); |
There was a problem hiding this comment.
Use early exit pattern and why is it no error when the file can't be opened?
| char* startswith(const char *s, const char *prefix) | ||
| { | ||
| size_t l = strlen(prefix); | ||
| if (strncmp(s, prefix, l) == 0) |
There was a problem hiding this comment.
new line between deceleration and code.
btw, you are not even using the helper (which I don't disagree strongly to introduce it)
| char* kv_keymatch(const char *kv, const char *key) | ||
| { | ||
| char *value = startswith(kv, key); | ||
| if (NULL != value) { |
| return val_len; | ||
| } | ||
|
|
||
| size_t getEntityName(char *buffer, size_t bufsz) |
| * if (streq(string, "")) | ||
| * printf("String is empty!\n"); | ||
| */ | ||
| #define streq(a,b) (strcmp((a),(b)) == 0) |
There was a problem hiding this comment.
First of all this define already exists in ccan, second don't use it. There is no point in papering over the fact C sucks when it comes to to strings. So everyone should know what '!strcmp()' means. streq is something I have to lookup to figure out what it does.
|
Thanks for the review @igaw . I will address all of those and resubmit the pull request. I also just found that there is a conflict with nvme-cli. I defined a method, read_file(), that also exists in nvme-cli and their definitions clash when building nvme-cli. So I need to fix that as well. I will also break this pull request into 3 commits: One for adding the Symbolic Name, one for adding the cntrltype and dctype, and one for the adding the DIM PDU. That should make reviewing a bit easier. |
This adds support for TP8010. More precisely, it adds the ability to send a new PDU, the Discovery Information Management (DIM) PDU, which is used to perform an explicit registration with Direct Discovery Controllers (DDC) and Central Discovery Controllers (CDC).
The following new APIs are added:
Note: The Host Symbolic Name is an optional parameter needed for by DIM PDU. When defined, it will be included in the DIM, otherwise it is omitted.
The kernel exposes a new attribute, the Discovery Controller Type (dctype) through the sysfs (kernel patch pending merge). This is used to determine whether explicit registration using the DIM PDU is supported. When the dctype (a previously reserved field of the Identify command response) is set to 1 (DDC) or 2 (CDC), we know that the remote Discovery Controller supports TP8010 and explicit registration. For pre-TP8010 DCs or any other type of controllers (admin, I/O), the dctype is set to 0 indicating that the dctype is not reported, in which case explicit registration will not be attempted.
Signed-off-by: Martin Belanger martin.belanger@dell.com