[PATCH] nvme-cli: Fix to the logic of the nvme list-ns <device> -a command.

[PATCH] nvme-cli: Fix to the logic of the nvme list-ns <device> -a command.

[Chaitanya Kulkarni]

This fix will reverse the logic of the -a parm in the nvme list-ns
command. Currently, the -a option only shows the attached namespaces
and not all the namespaces. By removing the extra ! it will show attached
inactive namespaces with -a option as mentioned in the help text.

Example : Controller has 3 namespaces, two of them are not attached.

Default exection with ?!!? :-
$ nvme list-ns  /dev/nvme0 -a
[   0]:0x1

Execution with ?!? :-
 nvme list-ns  /dev/nvme0 -a
[   0]:0x1
[   1]:0x2
[   2]:0x3

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at hgst.com>
---
 nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nvme.c b/nvme.c
index d64d042..a3abff0 100644
--- a/nvme.c
+++ b/nvme.c
@@ -560,7 +560,7 @@ 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);
+	err = nvme_identify_ns_list(fd, cfg.namespace_id, !cfg.all, ns_list);
 	if (!err) {
 		for (i = 0; i < 1024; i++)
 			if (ns_list[i])
-- 
2.7.4

[PATCH] nvme-cli: Fix to the logic of the nvme list-ns <device> -a command.

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi at grimberg.me>

[PATCH] nvme-cli: Fix to the logic of the nvme list-ns <device> -a command.

[Guan Junxiong]

Hi,Chaitanya
Thanks for your effort.

On 2017/7/9 8:36, Chaitanya Kulkarni wrote:
> This fix will reverse the logic of the -a parm in the nvme list-ns
> command. Currently, the -a option only shows the attached namespaces
> and not all the namespaces. By removing the extra ! it will show attached
> inactive namespaces with -a option as mentioned in the help text.
> 
Actually, the approach of reversing the logic of the -a option doesn't
keep the consistence of literal input parameter __bool__ __all__ of the
nvme_identify_ns_list function.

By the way ,I had sent a patch to fix this two month ago but Keith missed it.
A new updated patch of this was sent a minute ago. Hope Keith would catch it.
(The origin link is: http://lists.infradead.org/pipermail/linux-nvme/2017-April/009805.html
)

Thanks
.

[PATCH] nvme-cli: Fix to the logic of the nvme list-ns <device> -a command.

[royy walls]

With this approach
(http://lists.infradead.org/pipermail/linux-nvme/2017-April/009805.html)
all the future caller's including any plug-ins will require writing a
code with "!!".

I wanted to get rid of "!!" approach which makes the code simpler for
everyone at first glance, only if everyone agrees with that change.

On Mon, Jul 10, 2017@1:07 AM, Guan Junxiong <guanjunxiong@huawei.com> wrote:
> Hi,Chaitanya
> Thanks for your effort.
>
> On 2017/7/9 8:36, Chaitanya Kulkarni wrote:
>> This fix will reverse the logic of the -a parm in the nvme list-ns
>> command. Currently, the -a option only shows the attached namespaces
>> and not all the namespaces. By removing the extra ! it will show attached
>> inactive namespaces with -a option as mentioned in the help text.
>>
> Actually, the approach of reversing the logic of the -a option doesn't
> keep the consistence of literal input parameter __bool__ __all__ of the
> nvme_identify_ns_list function.
>
> By the way ,I had sent a patch to fix this two month ago but Keith missed it.
> A new updated patch of this was sent a minute ago. Hope Keith would catch it.
> (The origin link is: http://lists.infradead.org/pipermail/linux-nvme/2017-April/009805.html
> )
>
> Thanks
> .
>
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH] nvme-cli: Fix to the logic of the nvme list-ns <device> -a command.

[Keith Busch]

On Mon, Jul 10, 2017@01:40:02AM -0700, royy walls wrote:
> With this approach
> (http://lists.infradead.org/pipermail/linux-nvme/2017-April/009805.html)
> all the future caller's including any plug-ins will require writing a
> code with "!!".

The use of '!!' was to explicitly coerce the 'int' type required for the
arg parsing to a 'true/false' value for the bool type. It's not really
necessary, though.

> I wanted to get rid of "!!" approach which makes the code simpler for
> everyone at first glance, only if everyone agrees with that change.
> 
> On Mon, Jul 10, 2017@1:07 AM, Guan Junxiong <guanjunxiong@huawei.com> wrote:
> > Hi,Chaitanya
> > Thanks for your effort.
> >
> > On 2017/7/9 8:36, Chaitanya Kulkarni wrote:
> >> This fix will reverse the logic of the -a parm in the nvme list-ns
> >> command. Currently, the -a option only shows the attached namespaces
> >> and not all the namespaces. By removing the extra ! it will show attached
> >> inactive namespaces with -a option as mentioned in the help text.
> >>
> > Actually, the approach of reversing the logic of the -a option doesn't
> > keep the consistence of literal input parameter __bool__ __all__ of the
> > nvme_identify_ns_list function.
> >
> > By the way ,I had sent a patch to fix this two month ago but Keith missed it.
> > A new updated patch of this was sent a minute ago. Hope Keith would catch it.
> > (The origin link is: http://lists.infradead.org/pipermail/linux-nvme/2017-April/009805.html
> > )

Sorry I missed that patch. That one looks correct to me, and thank you
for reposting the v2. I think we'll use that one.