* [PATCH 1/5] nvme: update list-ns nsid option
@ 2019-05-30 9:43 Max Gurtovoy
2019-05-30 9:43 ` [PATCH 2/5] nvme: update description for "nvme list" command Max Gurtovoy
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Max Gurtovoy @ 2019-05-30 9:43 UTC (permalink / raw)
This commit updates the optional nsid argument to define the wanted
nsid for start, instead of starting from nsid + 1. E.g. in case we've
wanted to get the list of namespaces starting from 1, before this
commit, we used the "--namespace-id=0" option. Nsid 0 is not valid in
NVMe spec, thus change it to start counting from the given nsid.
Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
nvme.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/nvme.c b/nvme.c
index 9819fcb..9d763f5 100644
--- a/nvme.c
+++ b/nvme.c
@@ -950,9 +950,9 @@ close_fd:
static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
{
- const char *desc = "For the specified device, show the "\
- "namespace list in a NVMe subsystem, optionally starting with a given namespace";
- const char *namespace_id = "namespace number returned list should to start after";
+ const char *desc = "For the specified controller handle, show the "\
+ "namespace list in the associated NVMe subsystem, optionally starting with a given nsid.";
+ const char *namespace_id = "first nsid returned list should start from";
const char *all = "show all namespaces in the subsystem, whether attached or inactive";
int err, i, fd;
__u32 ns_list[1024];
@@ -963,7 +963,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
};
struct config cfg = {
- .namespace_id = 0,
+ .namespace_id = 1,
};
const struct argconfig_commandline_options command_line_options[] = {
@@ -976,7 +976,14 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
if (fd < 0)
return fd;
- err = nvme_identify_ns_list(fd, cfg.namespace_id, !!cfg.all, ns_list);
+ if (!cfg.namespace_id) {
+ err = -EINVAL;
+ fprintf(stderr, "invalid nsid parameter\n");
+ goto close_fd;
+ }
+
+ err = nvme_identify_ns_list(fd, cfg.namespace_id - 1, !!cfg.all,
+ ns_list);
if (!err) {
for (i = 0; i < 1024; i++)
if (ns_list[i])
@@ -987,6 +994,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
else
perror("id namespace list");
+close_fd:
close(fd);
return err;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] nvme: update description for "nvme list" command
2019-05-30 9:43 [PATCH 1/5] nvme: update list-ns nsid option Max Gurtovoy
@ 2019-05-30 9:43 ` Max Gurtovoy
2019-06-03 16:26 ` Chaitanya Kulkarni
2019-05-30 9:43 ` [PATCH 3/5] fabrics: Fix memory leak of subsys list Max Gurtovoy
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Max Gurtovoy @ 2019-05-30 9:43 UTC (permalink / raw)
The "nvme list" command doesn't get any device handle as an input
argument. This operation prints out a basic information for all NVMe
namespaces in the system.
Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
nvme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nvme.c b/nvme.c
index 9d763f5..3310abd 100644
--- a/nvme.c
+++ b/nvme.c
@@ -1801,7 +1801,7 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
struct list_item *list_items;
unsigned int list_cnt = 0;
int fmt, ret, fd, i, n;
- const char *desc = "Retrieve basic information for the given device";
+ const char *desc = "Retrieve basic information for all NVMe namespaces";
struct config {
char *output_format;
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] fabrics: Fix memory leak of subsys list
2019-05-30 9:43 [PATCH 1/5] nvme: update list-ns nsid option Max Gurtovoy
2019-05-30 9:43 ` [PATCH 2/5] nvme: update description for "nvme list" command Max Gurtovoy
@ 2019-05-30 9:43 ` Max Gurtovoy
2019-06-03 16:31 ` Chaitanya Kulkarni
2019-05-30 9:43 ` [PATCH 4/5] nvme-print: fix json object memory leak Max Gurtovoy
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Max Gurtovoy @ 2019-05-30 9:43 UTC (permalink / raw)
Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
fabrics.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fabrics.c b/fabrics.c
index 573a6ef..9ed4a56 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -1205,5 +1205,6 @@ int disconnect_all(const char *desc, int argc, char **argv)
}
}
out:
+ free_subsys_list(slist, subcnt);
return ret;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] nvme-print: fix json object memory leak
2019-05-30 9:43 [PATCH 1/5] nvme: update list-ns nsid option Max Gurtovoy
2019-05-30 9:43 ` [PATCH 2/5] nvme: update description for "nvme list" command Max Gurtovoy
2019-05-30 9:43 ` [PATCH 3/5] fabrics: Fix memory leak of subsys list Max Gurtovoy
@ 2019-05-30 9:43 ` Max Gurtovoy
2019-06-03 16:32 ` Chaitanya Kulkarni
2019-05-30 9:43 ` [PATCH 5/5] nvme: fix coding style issue Max Gurtovoy
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Max Gurtovoy @ 2019-05-30 9:43 UTC (permalink / raw)
Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
nvme-print.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/nvme-print.c b/nvme-print.c
index 2c4822e..6f85e73 100644
--- a/nvme-print.c
+++ b/nvme-print.c
@@ -2918,6 +2918,7 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
json_object_add_value_array(root, "Subsystems", subsystems);
json_print_object(root, NULL);
printf("\n");
+ json_free_object(root);
}
static void show_registers_cap(struct nvme_bar_cap *cap)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] nvme: fix coding style issue
2019-05-30 9:43 [PATCH 1/5] nvme: update list-ns nsid option Max Gurtovoy
` (2 preceding siblings ...)
2019-05-30 9:43 ` [PATCH 4/5] nvme-print: fix json object memory leak Max Gurtovoy
@ 2019-05-30 9:43 ` Max Gurtovoy
2019-06-03 16:33 ` Chaitanya Kulkarni
2019-05-30 17:51 ` [PATCH 1/5] nvme: update list-ns nsid option Sagi Grimberg
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Max Gurtovoy @ 2019-05-30 9:43 UTC (permalink / raw)
It's more common to use the following coding style:
if (condition) {
do_that;
do_this;
} else if (condition2) {
do_this;
} else {
do_that;
}
Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
nvme.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/nvme.c b/nvme.c
index 3310abd..2fd4362 100644
--- a/nvme.c
+++ b/nvme.c
@@ -988,11 +988,11 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
for (i = 0; i < 1024; i++)
if (ns_list[i])
printf("[%4u]:%#x\n", i, le32_to_cpu(ns_list[i]));
- }
- else if (err > 0)
+ } else if (err > 0) {
show_nvme_status(err);
- else
+ } else {
perror("id namespace list");
+ }
close_fd:
close(fd);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 1/5] nvme: update list-ns nsid option
2019-05-30 9:43 [PATCH 1/5] nvme: update list-ns nsid option Max Gurtovoy
` (3 preceding siblings ...)
2019-05-30 9:43 ` [PATCH 5/5] nvme: fix coding style issue Max Gurtovoy
@ 2019-05-30 17:51 ` Sagi Grimberg
2019-06-03 16:25 ` Chaitanya Kulkarni
2019-06-07 16:19 ` Keith Busch
6 siblings, 0 replies; 15+ messages in thread
From: Sagi Grimberg @ 2019-05-30 17:51 UTC (permalink / raw)
Next time please add nvme-cli prefix to the patch set
For the series:
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] nvme: update list-ns nsid option
2019-05-30 9:43 [PATCH 1/5] nvme: update list-ns nsid option Max Gurtovoy
` (4 preceding siblings ...)
2019-05-30 17:51 ` [PATCH 1/5] nvme: update list-ns nsid option Sagi Grimberg
@ 2019-06-03 16:25 ` Chaitanya Kulkarni
2019-06-04 8:56 ` Max Gurtovoy
2019-06-07 16:19 ` Keith Busch
6 siblings, 1 reply; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-03 16:25 UTC (permalink / raw)
This patch overall looks good go me, except one nit.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 5/30/19 2:43 AM, Max Gurtovoy wrote:
> This commit updates the optional nsid argument to define the wanted
> nsid for start, instead of starting from nsid + 1. E.g. in case we've
> wanted to get the list of namespaces starting from 1, before this
> commit, we used the "--namespace-id=0" option. Nsid 0 is not valid in
> NVMe spec, thus change it to start counting from the given nsid.
>
> Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> ---
> nvme.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/nvme.c b/nvme.c
> index 9819fcb..9d763f5 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -950,9 +950,9 @@ close_fd:
>
> static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
> {
> - const char *desc = "For the specified device, show the "\
> - "namespace list in a NVMe subsystem, optionally starting with a given namespace";
> - const char *namespace_id = "namespace number returned list should to start after";
> + const char *desc = "For the specified controller handle, show the "\
> + "namespace list in the associated NVMe subsystem, optionally starting with a given nsid.";
> + const char *namespace_id = "first nsid returned list should start from";
> const char *all = "show all namespaces in the subsystem, whether attached or inactive";
> int err, i, fd;
> __u32 ns_list[1024];
> @@ -963,7 +963,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
> };
>
> struct config cfg = {
> - .namespace_id = 0,
> + .namespace_id = 1,
> };
>
> const struct argconfig_commandline_options command_line_options[] = {
> @@ -976,7 +976,14 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
> if (fd < 0)
> return fd;
>
> - err = nvme_identify_ns_list(fd, cfg.namespace_id, !!cfg.all, ns_list);
> + if (!cfg.namespace_id) {
> + err = -EINVAL;
> + fprintf(stderr, "invalid nsid parameter\n");
> + goto close_fd;
> + }
> +
> + err = nvme_identify_ns_list(fd, cfg.namespace_id - 1, !!cfg.all,
> + ns_list);
nit:- Can we get rid of the new line for the last arg in above call ?
Can be fixed at the commit time.
> if (!err) {
> for (i = 0; i < 1024; i++)
> if (ns_list[i])
> @@ -987,6 +994,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
> else
> perror("id namespace list");
>
> +close_fd:
> close(fd);
>
> return err;
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/5] nvme: update description for "nvme list" command
2019-05-30 9:43 ` [PATCH 2/5] nvme: update description for "nvme list" command Max Gurtovoy
@ 2019-06-03 16:26 ` Chaitanya Kulkarni
0 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-03 16:26 UTC (permalink / raw)
Looks good.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 5/30/19 2:44 AM, Max Gurtovoy wrote:
> The "nvme list" command doesn't get any device handle as an input
> argument. This operation prints out a basic information for all NVMe
> namespaces in the system.
>
> Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> ---
> nvme.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/nvme.c b/nvme.c
> index 9d763f5..3310abd 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -1801,7 +1801,7 @@ static int list(int argc, char **argv, struct command *cmd, struct plugin *plugi
> struct list_item *list_items;
> unsigned int list_cnt = 0;
> int fmt, ret, fd, i, n;
> - const char *desc = "Retrieve basic information for the given device";
> + const char *desc = "Retrieve basic information for all NVMe namespaces";
> struct config {
> char *output_format;
> };
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/5] fabrics: Fix memory leak of subsys list
2019-05-30 9:43 ` [PATCH 3/5] fabrics: Fix memory leak of subsys list Max Gurtovoy
@ 2019-06-03 16:31 ` Chaitanya Kulkarni
0 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-03 16:31 UTC (permalink / raw)
Looks good.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 5/30/19 2:44 AM, Max Gurtovoy wrote:
> Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> ---
> fabrics.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fabrics.c b/fabrics.c
> index 573a6ef..9ed4a56 100644
> --- a/fabrics.c
> +++ b/fabrics.c
> @@ -1205,5 +1205,6 @@ int disconnect_all(const char *desc, int argc, char **argv)
> }
> }
> out:
> + free_subsys_list(slist, subcnt);
> return ret;
> }
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/5] nvme-print: fix json object memory leak
2019-05-30 9:43 ` [PATCH 4/5] nvme-print: fix json object memory leak Max Gurtovoy
@ 2019-06-03 16:32 ` Chaitanya Kulkarni
0 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-03 16:32 UTC (permalink / raw)
Looks good.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 5/30/19 2:44 AM, Max Gurtovoy wrote:
> Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> ---
> nvme-print.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/nvme-print.c b/nvme-print.c
> index 2c4822e..6f85e73 100644
> --- a/nvme-print.c
> +++ b/nvme-print.c
> @@ -2918,6 +2918,7 @@ void json_print_nvme_subsystem_list(struct subsys_list_item *slist, int n)
> json_object_add_value_array(root, "Subsystems", subsystems);
> json_print_object(root, NULL);
> printf("\n");
> + json_free_object(root);
> }
>
> static void show_registers_cap(struct nvme_bar_cap *cap)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/5] nvme: fix coding style issue
2019-05-30 9:43 ` [PATCH 5/5] nvme: fix coding style issue Max Gurtovoy
@ 2019-06-03 16:33 ` Chaitanya Kulkarni
0 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-03 16:33 UTC (permalink / raw)
Looks good.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
On 5/30/19 2:44 AM, Max Gurtovoy wrote:
> It's more common to use the following coding style:
> if (condition) {
> do_that;
> do_this;
> } else if (condition2) {
> do_this;
> } else {
> do_that;
> }
>
> Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> ---
> nvme.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/nvme.c b/nvme.c
> index 3310abd..2fd4362 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -988,11 +988,11 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
> for (i = 0; i < 1024; i++)
> if (ns_list[i])
> printf("[%4u]:%#x\n", i, le32_to_cpu(ns_list[i]));
> - }
> - else if (err > 0)
> + } else if (err > 0) {
> show_nvme_status(err);
> - else
> + } else {
> perror("id namespace list");
> + }
>
> close_fd:
> close(fd);
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] nvme: update list-ns nsid option
2019-06-03 16:25 ` Chaitanya Kulkarni
@ 2019-06-04 8:56 ` Max Gurtovoy
2019-06-04 16:45 ` Chaitanya Kulkarni
0 siblings, 1 reply; 15+ messages in thread
From: Max Gurtovoy @ 2019-06-04 8:56 UTC (permalink / raw)
On 6/3/2019 7:25 PM, Chaitanya Kulkarni wrote:
> This patch overall looks good go me, except one nit.
>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
>
> On 5/30/19 2:43 AM, Max Gurtovoy wrote:
>> This commit updates the optional nsid argument to define the wanted
>> nsid for start, instead of starting from nsid + 1. E.g. in case we've
>> wanted to get the list of namespaces starting from 1, before this
>> commit, we used the "--namespace-id=0" option. Nsid 0 is not valid in
>> NVMe spec, thus change it to start counting from the given nsid.
>>
>> Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
>> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
>> ---
>> nvme.c | 18 +++++++++++++-----
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/nvme.c b/nvme.c
>> index 9819fcb..9d763f5 100644
>> --- a/nvme.c
>> +++ b/nvme.c
>> @@ -950,9 +950,9 @@ close_fd:
>>
>> static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
>> {
>> - const char *desc = "For the specified device, show the "\
>> - "namespace list in a NVMe subsystem, optionally starting with a given namespace";
>> - const char *namespace_id = "namespace number returned list should to start after";
>> + const char *desc = "For the specified controller handle, show the "\
>> + "namespace list in the associated NVMe subsystem, optionally starting with a given nsid.";
>> + const char *namespace_id = "first nsid returned list should start from";
>> const char *all = "show all namespaces in the subsystem, whether attached or inactive";
>> int err, i, fd;
>> __u32 ns_list[1024];
>> @@ -963,7 +963,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>> };
>>
>> struct config cfg = {
>> - .namespace_id = 0,
>> + .namespace_id = 1,
>> };
>>
>> const struct argconfig_commandline_options command_line_options[] = {
>> @@ -976,7 +976,14 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>> if (fd < 0)
>> return fd;
>>
>> - err = nvme_identify_ns_list(fd, cfg.namespace_id, !!cfg.all, ns_list);
>> + if (!cfg.namespace_id) {
>> + err = -EINVAL;
>> + fprintf(stderr, "invalid nsid parameter\n");
>> + goto close_fd;
>> + }
>> +
>> + err = nvme_identify_ns_list(fd, cfg.namespace_id - 1, !!cfg.all,
>> + ns_list);
> nit:- Can we get rid of the new line for the last arg in above call ?
> Can be fixed at the commit time.
I did it to avoid getting more that 80 char line.
I think the maintainer can do it if it's necessary during commit time..
(or I can do it also. both work for me)
>> if (!err) {
>> for (i = 0; i < 1024; i++)
>> if (ns_list[i])
>> @@ -987,6 +994,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>> else
>> perror("id namespace list");
>>
>> +close_fd:
>> close(fd);
>>
>> return err;
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] nvme: update list-ns nsid option
2019-06-04 8:56 ` Max Gurtovoy
@ 2019-06-04 16:45 ` Chaitanya Kulkarni
0 siblings, 0 replies; 15+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-04 16:45 UTC (permalink / raw)
On 6/4/19 1:57 AM, Max Gurtovoy wrote:
> On 6/3/2019 7:25 PM, Chaitanya Kulkarni wrote:
>> This patch overall looks good go me, except one nit.
>>
>> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
>>
>> On 5/30/19 2:43 AM, Max Gurtovoy wrote:
>>> This commit updates the optional nsid argument to define the wanted
>>> nsid for start, instead of starting from nsid + 1. E.g. in case we've
>>> wanted to get the list of namespaces starting from 1, before this
>>> commit, we used the "--namespace-id=0" option. Nsid 0 is not valid in
>>> NVMe spec, thus change it to start counting from the given nsid.
>>>
>>> Reviewed-by: Minwoo Im <minwoo.im at samsung.com>
>>> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
>>> ---
>>> nvme.c | 18 +++++++++++++-----
>>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/nvme.c b/nvme.c
>>> index 9819fcb..9d763f5 100644
>>> --- a/nvme.c
>>> +++ b/nvme.c
>>> @@ -950,9 +950,9 @@ close_fd:
>>>
>>> static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *plugin)
>>> {
>>> - const char *desc = "For the specified device, show the "\
>>> - "namespace list in a NVMe subsystem, optionally starting with a given namespace";
>>> - const char *namespace_id = "namespace number returned list should to start after";
>>> + const char *desc = "For the specified controller handle, show the "\
>>> + "namespace list in the associated NVMe subsystem, optionally starting with a given nsid.";
>>> + const char *namespace_id = "first nsid returned list should start from";
>>> const char *all = "show all namespaces in the subsystem, whether attached or inactive";
>>> int err, i, fd;
>>> __u32 ns_list[1024];
>>> @@ -963,7 +963,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>>> };
>>>
>>> struct config cfg = {
>>> - .namespace_id = 0,
>>> + .namespace_id = 1,
>>> };
>>>
>>> const struct argconfig_commandline_options command_line_options[] = {
>>> @@ -976,7 +976,14 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>>> if (fd < 0)
>>> return fd;
>>>
>>> - err = nvme_identify_ns_list(fd, cfg.namespace_id, !!cfg.all, ns_list);
>>> + if (!cfg.namespace_id) {
>>> + err = -EINVAL;
>>> + fprintf(stderr, "invalid nsid parameter\n");
>>> + goto close_fd;
>>> + }
>>> +
>>> + err = nvme_identify_ns_list(fd, cfg.namespace_id - 1, !!cfg.all,
>>> + ns_list);
>> nit:- Can we get rid of the new line for the last arg in above call ?
>> Can be fixed at the commit time.
> I did it to avoid getting more that 80 char line.
>
> I think the maintainer can do it if it's necessary during commit time..
> (or I can do it also. both work for me)
Let's leave it to Keith.
>
>>> if (!err) {
>>> for (i = 0; i < 1024; i++)
>>> if (ns_list[i])
>>> @@ -987,6 +994,7 @@ static int list_ns(int argc, char **argv, struct command *cmd, struct plugin *pl
>>> else
>>> perror("id namespace list");
>>>
>>> +close_fd:
>>> close(fd);
>>>
>>> return err;
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] nvme: update list-ns nsid option
2019-05-30 9:43 [PATCH 1/5] nvme: update list-ns nsid option Max Gurtovoy
` (5 preceding siblings ...)
2019-06-03 16:25 ` Chaitanya Kulkarni
@ 2019-06-07 16:19 ` Keith Busch
2019-06-10 8:20 ` Max Gurtovoy
6 siblings, 1 reply; 15+ messages in thread
From: Keith Busch @ 2019-06-07 16:19 UTC (permalink / raw)
I've applied the entire series. Thanks, Max!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] nvme: update list-ns nsid option
2019-06-07 16:19 ` Keith Busch
@ 2019-06-10 8:20 ` Max Gurtovoy
0 siblings, 0 replies; 15+ messages in thread
From: Max Gurtovoy @ 2019-06-10 8:20 UTC (permalink / raw)
On 6/7/2019 7:19 PM, Keith Busch wrote:
> I've applied the entire series. Thanks, Max!
Great.
Now we need to decide how to proceed with the original purpose of
improving the "nvme list-subsys" cmd..
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-06-10 8:20 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-30 9:43 [PATCH 1/5] nvme: update list-ns nsid option Max Gurtovoy
2019-05-30 9:43 ` [PATCH 2/5] nvme: update description for "nvme list" command Max Gurtovoy
2019-06-03 16:26 ` Chaitanya Kulkarni
2019-05-30 9:43 ` [PATCH 3/5] fabrics: Fix memory leak of subsys list Max Gurtovoy
2019-06-03 16:31 ` Chaitanya Kulkarni
2019-05-30 9:43 ` [PATCH 4/5] nvme-print: fix json object memory leak Max Gurtovoy
2019-06-03 16:32 ` Chaitanya Kulkarni
2019-05-30 9:43 ` [PATCH 5/5] nvme: fix coding style issue Max Gurtovoy
2019-06-03 16:33 ` Chaitanya Kulkarni
2019-05-30 17:51 ` [PATCH 1/5] nvme: update list-ns nsid option Sagi Grimberg
2019-06-03 16:25 ` Chaitanya Kulkarni
2019-06-04 8:56 ` Max Gurtovoy
2019-06-04 16:45 ` Chaitanya Kulkarni
2019-06-07 16:19 ` Keith Busch
2019-06-10 8:20 ` Max Gurtovoy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.