Issue With NVME CLI list Command

Issue With NVME CLI list Command

[Jeffrey Lien]

Keith, 
I'm getting a "Failed to scan namespaces" error message back from the "nvme list" command after updating my fork and picking up your changes from Aug 28th.  The cause of the failure is the following check in the function verify_legacy_ns (in nvme-topology.c):

	if (memcmp(id.mn, c->id.mn, sizeof(id.mn)) ||
	    memcmp(id.sn, c->id.mn, sizeof(id.sn)))
		return -ENODEV;

The problem I see is that ctrl info is never filled in (at least in my case).  Both c->id.mn and c->id.sn are blank.  Looking back in the legacy_list function, I don't see how the ctrl info is ever filled in.  Maybe it's just something strange in my case; I would have thought someone would have stumbled across already but maybe not.  If I remove the above check; everything seems to work so I think the code is basically working as expected.    Could you review the changes and either confirm what I'm seeing or let me know if I'm missing something.  

Thanks
  

Jeff Lien
Linux Device Driver Development
Device Host Apps and Drivers
jeff.lien@wdc.com
o: 507-322-2416 (ext. 23-2416)
m: 507-273-9124







_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: Issue With NVME CLI list Command

[Keith Busch]

On Thu, Oct 10, 2019 at 07:11:10PM +0000, Jeffrey Lien wrote:
> Keith, 
> I'm getting a "Failed to scan namespaces" error message back from the
> "nvme list" command after updating my fork and picking up your changes
> from Aug 28th.  The cause of the failure is the following check in the
> function verify_legacy_ns (in nvme-topology.c):
> 
> 	if (memcmp(id.mn, c->id.mn, sizeof(id.mn)) ||
> 	    memcmp(id.sn, c->id.mn, sizeof(id.sn)))
> 		return -ENODEV;
> 
> The problem I see is that ctrl info is never filled in (at least in my
> case).  Both c->id.mn and c->id.sn are blank.  Looking back in the
> legacy_list function, I don't see how the ctrl info is ever filled in.
> Maybe it's just something strange in my case; I would have thought
> someone would have stumbled across already but maybe not.  If I remove
> the above check; everything seems to work so I think the code is
> basically working as expected.    Could you review the changes and
> either confirm what I'm seeing or let me know if I'm missing
> something.  

Hi Jeff,

What kernel version are you using?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: Issue With NVME CLI list Command

[Keith Busch]

On Fri, Oct 11, 2019 at 04:23:18AM +0900, Keith Busch wrote:
> On Thu, Oct 10, 2019 at 07:11:10PM +0000, Jeffrey Lien wrote:
> > Keith, 
> > I'm getting a "Failed to scan namespaces" error message back from the
> > "nvme list" command after updating my fork and picking up your changes
> > from Aug 28th.  The cause of the failure is the following check in the
> > function verify_legacy_ns (in nvme-topology.c):
> > 
> > 	if (memcmp(id.mn, c->id.mn, sizeof(id.mn)) ||
> > 	    memcmp(id.sn, c->id.mn, sizeof(id.sn)))
> > 		return -ENODEV;
> > 
> > The problem I see is that ctrl info is never filled in (at least in my
> > case).  Both c->id.mn and c->id.sn are blank.  Looking back in the
> > legacy_list function, I don't see how the ctrl info is ever filled in.
> > Maybe it's just something strange in my case; I would have thought
> > someone would have stumbled across already but maybe not.  If I remove
> > the above check; everything seems to work so I think the code is
> > basically working as expected.    Could you review the changes and
> > either confirm what I'm seeing or let me know if I'm missing
> > something.  
> 
> Hi Jeff,
> 
> What kernel version are you using?

Oops, nevermind, there was a couple bugs in that path. The controller
needed to be initialized before scanning the namespaces, and then we
need to compare the correct fields. Fixed now:

  https://github.com/linux-nvme/nvme-cli/commit/1e858be6c25172f9946e4979ee724244bdd722b2

Thanks for the notice.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: Issue With NVME CLI list Command

[Jeffrey Lien]

Keith,
Redhat 7.5, version 3.10.0-862.3.3.el7


Jeff Lien

-----Original Message-----
From: Keith Busch <kbusch@kernel.org> 
Sent: Thursday, October 10, 2019 2:23 PM
To: Jeffrey Lien <Jeff.Lien@wdc.com>
Cc: linux-nvme <linux-nvme@lists.infradead.org>
Subject: Re: Issue With NVME CLI list Command

CAUTION: This email originated from outside of Western Digital. Do not click on links or open attachments unless you recognize the sender and know that the content is safe.


On Thu, Oct 10, 2019 at 07:11:10PM +0000, Jeffrey Lien wrote:
> Keith,
> I'm getting a "Failed to scan namespaces" error message back from the 
> "nvme list" command after updating my fork and picking up your changes 
> from Aug 28th.  The cause of the failure is the following check in the 
> function verify_legacy_ns (in nvme-topology.c):
>
>       if (memcmp(id.mn, c->id.mn, sizeof(id.mn)) ||
>           memcmp(id.sn, c->id.mn, sizeof(id.sn)))
>               return -ENODEV;
>
> The problem I see is that ctrl info is never filled in (at least in my 
> case).  Both c->id.mn and c->id.sn are blank.  Looking back in the 
> legacy_list function, I don't see how the ctrl info is ever filled in.
> Maybe it's just something strange in my case; I would have thought 
> someone would have stumbled across already but maybe not.  If I remove 
> the above check; everything seems to work so I think the code is
> basically working as expected.    Could you review the changes and
> either confirm what I'm seeing or let me know if I'm missing 
> something.

Hi Jeff,

What kernel version are you using?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: Issue With NVME CLI list Command

[Jeffrey Lien]

Thanks for the quick fix Keith.  


Jeff Lien


-----Original Message-----
From: Keith Busch <kbusch@kernel.org> 
Sent: Thursday, October 10, 2019 2:39 PM
To: Jeffrey Lien <Jeff.Lien@wdc.com>
Cc: linux-nvme <linux-nvme@lists.infradead.org>
Subject: Re: Issue With NVME CLI list Command

CAUTION: This email originated from outside of Western Digital. Do not click on links or open attachments unless you recognize the sender and know that the content is safe.


On Fri, Oct 11, 2019 at 04:23:18AM +0900, Keith Busch wrote:
> On Thu, Oct 10, 2019 at 07:11:10PM +0000, Jeffrey Lien wrote:
> > Keith,
> > I'm getting a "Failed to scan namespaces" error message back from 
> > the "nvme list" command after updating my fork and picking up your 
> > changes from Aug 28th.  The cause of the failure is the following 
> > check in the function verify_legacy_ns (in nvme-topology.c):
> >
> >     if (memcmp(id.mn, c->id.mn, sizeof(id.mn)) ||
> >         memcmp(id.sn, c->id.mn, sizeof(id.sn)))
> >             return -ENODEV;
> >
> > The problem I see is that ctrl info is never filled in (at least in 
> > my case).  Both c->id.mn and c->id.sn are blank.  Looking back in 
> > the legacy_list function, I don't see how the ctrl info is ever filled in.
> > Maybe it's just something strange in my case; I would have thought 
> > someone would have stumbled across already but maybe not.  If I 
> > remove the above check; everything seems to work so I think the code is
> > basically working as expected.    Could you review the changes and
> > either confirm what I'm seeing or let me know if I'm missing 
> > something.
>
> Hi Jeff,
>
> What kernel version are you using?

Oops, nevermind, there was a couple bugs in that path. The controller needed to be initialized before scanning the namespaces, and then we need to compare the correct fields. Fixed now:

  https://github.com/linux-nvme/nvme-cli/commit/1e858be6c25172f9946e4979ee724244bdd722b2

Thanks for the notice.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme