Hi all- This was github PR #594, but Keith asked me to email it. Please review this carefully. Also, if you merge this, please consider changing the 'luto.us' bit to something a bit more sensible. Maybe com.github.linux-nvme? I'm still dubious about the way that newlines in the ids are handled, but my patches don't change that per se. Andy Lutomirski (4): hostnqn: Fix the systemd-based NQN namespace to comply with the spec fabrics: Rename nvmf_hostnqn_file() to nvmf_hostnqn_load() Use systemd-generated hostid if no hostid is configured Do not install /etc/nvme/hostid or /etc/nvme/hostnqn by default Documentation/nvme-show-hostid.txt | 29 +++++++++++++ Makefile | 4 +- fabrics.c | 68 +++++++++++++++++++++++++----- fabrics.h | 1 + nvme-builtin.h | 1 + nvme.c | 15 +++++++ 6 files changed, 105 insertions(+), 13 deletions(-) create mode 100644 Documentation/nvme-show-hostid.txt -- 2.23.0 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
The NVME Base spec, section 7.9 explicitly states that nqn.2014-08.org.nvmexpress:uuid: uses an RFC 4122 UUID, but we weren't generating one. We could change it by actually generating a compliant UUID, but we can also just invent a new naming authority. Use "nqn.2019-10.us.luto:sd_id128_app:" Signed-off-by: Andy Lutomirski <luto@kernel.org> --- fabrics.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fabrics.c b/fabrics.c index 655bd36c5639..12be2e73986e 100644 --- a/fabrics.c +++ b/fabrics.c @@ -691,7 +691,12 @@ static char *hostnqn_generate_systemd(void) if (sd_id128_get_machine_app_specific(NVME_HOSTNQN_ID, &id) < 0) return NULL; - if (asprintf(&ret, "nqn.2014-08.org.nvmexpress:uuid:" SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)) == -1) + /* + * "nqn.2019-10.us.luto:sd_id128_app:" is hereby defined as an SD_ID128 + * app-specific machine ID as generated here. + */ + + if (asprintf(&ret, "nqn.2019-10.us.luto:sd_id128_app:" SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)) == -1) ret = NULL; return ret; -- 2.23.0 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
The old name was confusing, since the value didn't necessarily come from a file. Signed-off-by: Andy Lutomirski <luto@kernel.org> --- fabrics.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fabrics.c b/fabrics.c index 12be2e73986e..4448416f9855 100644 --- a/fabrics.c +++ b/fabrics.c @@ -721,7 +721,7 @@ char *hostnqn_read(void) return NULL; } -static int nvmf_hostnqn_file(void) +static int nvmf_hostnqn_load(void) { cfg.hostnqn = hostnqn_read(); @@ -827,7 +827,7 @@ static int build_options(char *argstr, int max_len, bool discover) add_argument(&argstr, &max_len, "traddr", cfg.traddr) || add_argument(&argstr, &max_len, "host_traddr", cfg.host_traddr) || add_argument(&argstr, &max_len, "trsvcid", cfg.trsvcid) || - ((cfg.hostnqn || nvmf_hostnqn_file()) && + ((cfg.hostnqn || nvmf_hostnqn_load()) && add_argument(&argstr, &max_len, "hostnqn", cfg.hostnqn)) || ((cfg.hostid || nvmf_hostid_file()) && add_argument(&argstr, &max_len, "hostid", cfg.hostid)) || -- 2.23.0 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
This is just like the hostnqn support. It adds a show-hostid command for introspection. Signed-off-by: Andy Lutomirski <luto@kernel.org> --- Documentation/nvme-show-hostid.txt | 29 +++++++++++++++ fabrics.c | 57 ++++++++++++++++++++++++++---- fabrics.h | 1 + nvme-builtin.h | 1 + nvme.c | 15 ++++++++ 5 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 Documentation/nvme-show-hostid.txt diff --git a/Documentation/nvme-show-hostid.txt b/Documentation/nvme-show-hostid.txt new file mode 100644 index 000000000000..52bdc8a1f480 --- /dev/null +++ b/Documentation/nvme-show-hostid.txt @@ -0,0 +1,29 @@ +nvme-show-hostid(1) +=================== + +NAME +---- +nvme-show-hostid - Generate a host NVMe ID + +SYNOPSIS +-------- +[verse] +'nvme show-hostid' + +DESCRIPTION +----------- +Show the host ID configured for the system. If /etc/nvme/hostid is +not present and systemd application-specific machine IDs are available, +this will show the systemd-generated host ID for the system. + +OPTIONS +------- +No options needed + +EXAMPLES +-------- +nvme show-hostid + +NVME +---- +Part of the nvme-user suite diff --git a/fabrics.c b/fabrics.c index 4448416f9855..7c5b7efd5270 100644 --- a/fabrics.c +++ b/fabrics.c @@ -46,6 +46,7 @@ #ifdef HAVE_SYSTEMD #include <systemd/sd-id128.h> #define NVME_HOSTNQN_ID SD_ID128_MAKE(c7,f4,61,81,12,be,49,32,8c,83,10,6f,9d,dd,d8,6b) +#define NVME_HOSTID_ID SD_ID128_MAKE(df,66,bf,ec,f7,e4,21,0e,46,27,ac,a8,f2,8f,3b,98) #endif #define NVMF_HOSTID_SIZE 36 @@ -728,11 +729,11 @@ static int nvmf_hostnqn_load(void) return cfg.hostnqn != NULL; } -static int nvmf_hostid_file(void) +static char *hostid_read_file(void) { FILE *f; char hostid[NVMF_HOSTID_SIZE + 1]; - int ret = false; + char *ret = NULL; f = fopen(PATH_NVMF_HOSTID, "r"); if (f == NULL) @@ -741,16 +742,58 @@ static int nvmf_hostid_file(void) if (fgets(hostid, sizeof(hostid), f) == NULL) goto out; - cfg.hostid = strdup(hostid); - if (!cfg.hostid) - goto out; + ret = strdup(hostid); + - ret = true; out: fclose(f); return ret; } +static char *hostid_generate_systemd(void) +{ +#if defined(LIBUUID) && defined(HAVE_SYSTEMD) + sd_id128_t id; + char uuidstr[37]; + char *ret; + + if (sd_id128_get_machine_app_specific(NVME_HOSTID_ID, &id) < 0) + return NULL; + + uuid_unparse_lower(id.bytes, uuidstr); + + if (asprintf(&ret, "%s\n", uuidstr) == -1) + ret = NULL; + + return ret; +#else + return NULL; +#endif +} + +/* returns an allocated string or NULL */ +char *hostid_read(void) +{ + char *ret; + + ret = hostid_read_file(); + if (ret) + return ret; + + ret = hostid_generate_systemd(); + if (ret) + return ret; + + return NULL; +} + +static int nvmf_hostid_load(void) +{ + cfg.hostid = hostid_read(); + + return cfg.hostid != NULL; +} + static int add_bool_argument(char **argstr, int *max_len, char *arg_str, bool arg) { @@ -829,7 +872,7 @@ static int build_options(char *argstr, int max_len, bool discover) add_argument(&argstr, &max_len, "trsvcid", cfg.trsvcid) || ((cfg.hostnqn || nvmf_hostnqn_load()) && add_argument(&argstr, &max_len, "hostnqn", cfg.hostnqn)) || - ((cfg.hostid || nvmf_hostid_file()) && + ((cfg.hostid || nvmf_hostid_load()) && add_argument(&argstr, &max_len, "hostid", cfg.hostid)) || (!discover && add_int_argument(&argstr, &max_len, "nr_io_queues", diff --git a/fabrics.h b/fabrics.h index b8e53f492b53..64aede897535 100644 --- a/fabrics.h +++ b/fabrics.h @@ -4,6 +4,7 @@ #define NVMF_DEF_DISC_TMO 30 extern char *hostnqn_read(void); +extern char *hostid_read(void); extern int discover(const char *desc, int argc, char **argv, bool connect); extern int connect(const char *desc, int argc, char **argv); diff --git a/nvme-builtin.h b/nvme-builtin.h index bfb907dff9ef..907d470cbeac 100644 --- a/nvme-builtin.h +++ b/nvme-builtin.h @@ -71,6 +71,7 @@ COMMAND_LIST( ENTRY("disconnect-all", "Disconnect from all connected NVMeoF subsystems", disconnect_all_cmd) ENTRY("gen-hostnqn", "Generate NVMeoF host NQN", gen_hostnqn_cmd) ENTRY("show-hostnqn", "Show NVMeoF host NQN", show_hostnqn_cmd) + ENTRY("show-hostid", "Show NVMeoF host ID", show_hostid_cmd) ENTRY("dir-receive", "Submit a Directive Receive command, return results", dir_receive) ENTRY("dir-send", "Submit a Directive Send command, return results", dir_send) ENTRY("virt-mgmt", "Manage Flexible Resources between Primary and Secondary Controller ", virtual_mgmt) diff --git a/nvme.c b/nvme.c index 0c23eee8a477..d81f8226b84d 100644 --- a/nvme.c +++ b/nvme.c @@ -4771,6 +4771,21 @@ static int show_hostnqn_cmd(int argc, char **argv, struct command *command, stru } } +static int show_hostid_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) +{ + char *hostid; + + hostid = hostid_read(); + if (hostid) { + fputs(hostid, stdout); + free(hostid); + return 0; + } else { + fprintf(stderr, "hostid is not available -- generate /etc/nvme/hostid\n"); + return -ENOENT; + } +} + static int discover_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) { const char *desc = "Send Get Log Page request to Discovery Controller."; -- 2.23.0 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
Pre-generating them is nonsensical for package managers. Moreover, with the new systemd-generated ID support, it's preferable for the files to be absent. Don't generate them unless explicitly requested. Signed-off-by: Andy Lutomirski <luto@kernel.org> --- Makefile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile b/Makefile index d88404dd51f6..acb6983cb5b1 100644 --- a/Makefile +++ b/Makefile @@ -140,14 +140,12 @@ install-hostparams: install-etc install-etc: $(INSTALL) -d $(DESTDIR)$(SYSCONFDIR)/nvme - touch $(DESTDIR)$(SYSCONFDIR)/nvme/hostnqn - touch $(DESTDIR)$(SYSCONFDIR)/nvme/hostid if [ ! -f $(DESTDIR)$(SYSCONFDIR)/nvme/discovery.conf ]; then \ $(INSTALL) -m 644 -T ./etc/discovery.conf.in $(DESTDIR)$(SYSCONFDIR)/nvme/discovery.conf; \ fi install-spec: install-bin install-man install-bash-completion install-zsh-completion install-etc install-systemd install-udev install-dracut -install: install-spec install-hostparams +install: install-spec nvme.spec: nvme.spec.in NVME-VERSION-FILE sed -e 's/@@VERSION@@/$(NVME_VERSION)/g' < $< > $@+ -- 2.23.0 _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
Hello Andy, On 12/19/2019 09:31 PM, Andy Lutomirski wrote: > This is just like the hostnqn support. It adds a show-hostid command > for introspection. > > Signed-off-by: Andy Lutomirski <luto@kernel.org> > --- > Documentation/nvme-show-hostid.txt | 29 +++++++++++++++ > fabrics.c | 57 ++++++++++++++++++++++++++---- > fabrics.h | 1 + > nvme-builtin.h | 1 + > nvme.c | 15 ++++++++ > 5 files changed, 96 insertions(+), 7 deletions(-) > create mode 100644 Documentation/nvme-show-hostid.txt > > diff --git a/Documentation/nvme-show-hostid.txt b/Documentation/nvme-show-hostid.txt > new file mode 100644 > index 000000000000..52bdc8a1f480 > --- /dev/null > +++ b/Documentation/nvme-show-hostid.txt > @@ -0,0 +1,29 @@ > +nvme-show-hostid(1) > +=================== > + > +NAME > +---- > +nvme-show-hostid - Generate a host NVMe ID > + I pulled the latest nvme-cli and applied your patch set https://github.com/linux-nvme/nvme-cli Seems to work on Fedora 31 $ nvme show-hostnqn nqn.2019-10.us.luto:sd_id128_app:a9e4b2c8953340988142e32ca6d67922 $ nvme show-hostid f467779d-1940-4659-b200-74278899b2ca Thanks, David > +SYNOPSIS > +-------- > +[verse] > +'nvme show-hostid' > + > +DESCRIPTION > +----------- > +Show the host ID configured for the system. If /etc/nvme/hostid is > +not present and systemd application-specific machine IDs are available, > +this will show the systemd-generated host ID for the system. > + > +OPTIONS > +------- > +No options needed > + > +EXAMPLES > +-------- > +nvme show-hostid > + > +NVME > +---- > +Part of the nvme-user suite > diff --git a/fabrics.c b/fabrics.c > index 4448416f9855..7c5b7efd5270 100644 > --- a/fabrics.c > +++ b/fabrics.c > @@ -46,6 +46,7 @@ > #ifdef HAVE_SYSTEMD > #include <systemd/sd-id128.h> > #define NVME_HOSTNQN_ID SD_ID128_MAKE(c7,f4,61,81,12,be,49,32,8c,83,10,6f,9d,dd,d8,6b) > +#define NVME_HOSTID_ID SD_ID128_MAKE(df,66,bf,ec,f7,e4,21,0e,46,27,ac,a8,f2,8f,3b,98) > #endif > > #define NVMF_HOSTID_SIZE 36 > @@ -728,11 +729,11 @@ static int nvmf_hostnqn_load(void) > return cfg.hostnqn != NULL; > } > > -static int nvmf_hostid_file(void) > +static char *hostid_read_file(void) > { > FILE *f; > char hostid[NVMF_HOSTID_SIZE + 1]; > - int ret = false; > + char *ret = NULL; > > f = fopen(PATH_NVMF_HOSTID, "r"); > if (f == NULL) > @@ -741,16 +742,58 @@ static int nvmf_hostid_file(void) > if (fgets(hostid, sizeof(hostid), f) == NULL) > goto out; > > - cfg.hostid = strdup(hostid); > - if (!cfg.hostid) > - goto out; > + ret = strdup(hostid); > + > > - ret = true; > out: > fclose(f); > return ret; > } > > +static char *hostid_generate_systemd(void) > +{ > +#if defined(LIBUUID) && defined(HAVE_SYSTEMD) > + sd_id128_t id; > + char uuidstr[37]; > + char *ret; > + > + if (sd_id128_get_machine_app_specific(NVME_HOSTID_ID, &id) < 0) > + return NULL; > + > + uuid_unparse_lower(id.bytes, uuidstr); > + > + if (asprintf(&ret, "%s\n", uuidstr) == -1) > + ret = NULL; > + > + return ret; > +#else > + return NULL; > +#endif > +} > + > +/* returns an allocated string or NULL */ > +char *hostid_read(void) > +{ > + char *ret; > + > + ret = hostid_read_file(); > + if (ret) > + return ret; > + > + ret = hostid_generate_systemd(); > + if (ret) > + return ret; > + > + return NULL; > +} > + > +static int nvmf_hostid_load(void) > +{ > + cfg.hostid = hostid_read(); > + > + return cfg.hostid != NULL; > +} > + > static int > add_bool_argument(char **argstr, int *max_len, char *arg_str, bool arg) > { > @@ -829,7 +872,7 @@ static int build_options(char *argstr, int max_len, bool discover) > add_argument(&argstr, &max_len, "trsvcid", cfg.trsvcid) || > ((cfg.hostnqn || nvmf_hostnqn_load()) && > add_argument(&argstr, &max_len, "hostnqn", cfg.hostnqn)) || > - ((cfg.hostid || nvmf_hostid_file()) && > + ((cfg.hostid || nvmf_hostid_load()) && > add_argument(&argstr, &max_len, "hostid", cfg.hostid)) || > (!discover && > add_int_argument(&argstr, &max_len, "nr_io_queues", > diff --git a/fabrics.h b/fabrics.h > index b8e53f492b53..64aede897535 100644 > --- a/fabrics.h > +++ b/fabrics.h > @@ -4,6 +4,7 @@ > #define NVMF_DEF_DISC_TMO 30 > > extern char *hostnqn_read(void); > +extern char *hostid_read(void); > > extern int discover(const char *desc, int argc, char **argv, bool connect); > extern int connect(const char *desc, int argc, char **argv); > diff --git a/nvme-builtin.h b/nvme-builtin.h > index bfb907dff9ef..907d470cbeac 100644 > --- a/nvme-builtin.h > +++ b/nvme-builtin.h > @@ -71,6 +71,7 @@ COMMAND_LIST( > ENTRY("disconnect-all", "Disconnect from all connected NVMeoF subsystems", disconnect_all_cmd) > ENTRY("gen-hostnqn", "Generate NVMeoF host NQN", gen_hostnqn_cmd) > ENTRY("show-hostnqn", "Show NVMeoF host NQN", show_hostnqn_cmd) > + ENTRY("show-hostid", "Show NVMeoF host ID", show_hostid_cmd) > ENTRY("dir-receive", "Submit a Directive Receive command, return results", dir_receive) > ENTRY("dir-send", "Submit a Directive Send command, return results", dir_send) > ENTRY("virt-mgmt", "Manage Flexible Resources between Primary and Secondary Controller ", virtual_mgmt) > diff --git a/nvme.c b/nvme.c > index 0c23eee8a477..d81f8226b84d 100644 > --- a/nvme.c > +++ b/nvme.c > @@ -4771,6 +4771,21 @@ static int show_hostnqn_cmd(int argc, char **argv, struct command *command, stru > } > } > > +static int show_hostid_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) > +{ > + char *hostid; > + > + hostid = hostid_read(); > + if (hostid) { > + fputs(hostid, stdout); > + free(hostid); > + return 0; > + } else { > + fprintf(stderr, "hostid is not available -- generate /etc/nvme/hostid\n"); > + return -ENOENT; > + } > +} > + > static int discover_cmd(int argc, char **argv, struct command *command, struct plugin *plugin) > { > const char *desc = "Send Get Log Page request to Discovery Controller."; > _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
On Fri, Jan 3, 2020 at 10:14 AM David Milburn <dmilburn@redhat.com> wrote: > > Hello Andy, > > On 12/19/2019 09:31 PM, Andy Lutomirski wrote: > > This is just like the hostnqn support. It adds a show-hostid command > > for introspection. > > > > Signed-off-by: Andy Lutomirski <luto@kernel.org> > > --- > > Documentation/nvme-show-hostid.txt | 29 +++++++++++++++ > > fabrics.c | 57 ++++++++++++++++++++++++++---- > > fabrics.h | 1 + > > nvme-builtin.h | 1 + > > nvme.c | 15 ++++++++ > > 5 files changed, 96 insertions(+), 7 deletions(-) > > create mode 100644 Documentation/nvme-show-hostid.txt > > > > diff --git a/Documentation/nvme-show-hostid.txt b/Documentation/nvme-show-hostid.txt > > new file mode 100644 > > index 000000000000..52bdc8a1f480 > > --- /dev/null > > +++ b/Documentation/nvme-show-hostid.txt > > @@ -0,0 +1,29 @@ > > +nvme-show-hostid(1) > > +=================== > > + > > +NAME > > +---- > > +nvme-show-hostid - Generate a host NVMe ID > > + > > I pulled the latest nvme-cli and applied your patch set > > https://github.com/linux-nvme/nvme-cli > > Seems to work on Fedora 31 > > $ nvme show-hostnqn > nqn.2019-10.us.luto:sd_id128_app:a9e4b2c8953340988142e32ca6d67922 > > $ nvme show-hostid > f467779d-1940-4659-b200-74278899b2ca > Good news! When I run it, I get different output! $ ./nvme show-hostnqn nqn.2019-10.us.luto:sd_id128_app:a3cee2b99e504da68d4b9e7de354bebd $ ./nvme show-hostid c701865f-88c9-43c3-83a0-ead417dea2de _______________________________________________ linux-nvme mailing list linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme