[PATCH RFC] nvme: Common subsys and controller instances IDA

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Keith Busch]

The controller char device name used to be a subset of all its
namespace block device names, but that's not true when we name block
devices for their parent subsystem. It's long been the case that the
numerals appended to our device handles are simply the first unique
number available. They do not have any more significance in relations
to other devices with simpler numeric prefixes.

However, this disconnect has led to some very unfortunate situations
where people assume the old behavior, and accidently have lost data
because they issue admin commands to the wrong device. For example,
I want to format /dev/nvme0n1 and /dev/nvme0n2, so I'll run:

  # nvme format /dev/nvme0

to hit up all namespaces attached to nvme0. The problem is that nvme0's
namespaces may not even be those desired namespaces, and now I've lost
data on devices I wished to preserve.

So here's a solution that no one will like: pull subsystem and controller
instances from the same IDA so that there won't be any namespace block
devices with a matching controller handle name. While this does nothing
to clear up device relationships, this will force the user to think
really hard about what they're doing and avoid such mistakes.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/nvme/host/core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a6644a2c3ef7..1d17732267f0 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -80,7 +80,6 @@ EXPORT_SYMBOL_GPL(nvme_reset_wq);
 struct workqueue_struct *nvme_delete_wq;
 EXPORT_SYMBOL_GPL(nvme_delete_wq);
 
-static DEFINE_IDA(nvme_subsystems_ida);
 static LIST_HEAD(nvme_subsystems);
 static DEFINE_MUTEX(nvme_subsystems_lock);
 
@@ -2247,7 +2246,7 @@ static void nvme_init_subnqn(struct nvme_subsystem *subsys, struct nvme_ctrl *ct
 
 static void __nvme_release_subsystem(struct nvme_subsystem *subsys)
 {
-	ida_simple_remove(&nvme_subsystems_ida, subsys->instance);
+	ida_simple_remove(&nvme_instance_ida, subsys->instance);
 	kfree(subsys);
 }
 
@@ -2366,7 +2365,7 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 	subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
 	if (!subsys)
 		return -ENOMEM;
-	ret = ida_simple_get(&nvme_subsystems_ida, 0, 0, GFP_KERNEL);
+	ret = ida_simple_get(&nvme_instance_ida, 0, 0, GFP_KERNEL);
 	if (ret < 0) {
 		kfree(subsys);
 		return ret;
@@ -3958,7 +3957,6 @@ static int __init nvme_core_init(void)
 
 static void __exit nvme_core_exit(void)
 {
-	ida_destroy(&nvme_subsystems_ida);
 	class_destroy(nvme_subsys_class);
 	class_destroy(nvme_class);
 	unregister_chrdev_region(nvme_chr_devt, NVME_MINORS);
-- 
2.14.4

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Christoph Hellwig]

On Wed, May 15, 2019@03:33:51PM -0600, Keith Busch wrote:
> So here's a solution that no one will like: pull subsystem and controller
> instances from the same IDA so that there won't be any namespace block
> devices with a matching controller handle name. While this does nothing
> to clear up device relationships, this will force the user to think
> really hard about what they're doing and avoid such mistakes.

Hmm.  So we'll get:

/dev/nvme0
 - chardev subsys X ctl 1

/dev/nvme1n1
/dev/nvme1n2
 - namespaces for subsys X

/dev/nvme2
 - chardev subsys X ctl 2

/dev/nvme3
 - chardev subsys Y ctl 1

...

This should work.  Not sure it really buys us so much, though.

Btw, I wonder if we should have udev rules for:

/dev/nvme-subsys-$NQN-ctrl1
/dev/nvme-subsys-$NQN-ctrl2
/dev/nvme-subsys-$NQN-ns1
/dev/nvme-subsys-$NQN-ns2

I have to admit that since udev/systemd moved to github I can't
be bother to send them patches, though as it would require signing
up for that crappy service.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Hannes Reinecke]

On 5/16/19 8:46 AM, Christoph Hellwig wrote:
> On Wed, May 15, 2019@03:33:51PM -0600, Keith Busch wrote:
>> So here's a solution that no one will like: pull subsystem and controller
>> instances from the same IDA so that there won't be any namespace block
>> devices with a matching controller handle name. While this does nothing
>> to clear up device relationships, this will force the user to think
>> really hard about what they're doing and avoid such mistakes.
> 
> Hmm.  So we'll get:
> 
> /dev/nvme0
>   - chardev subsys X ctl 1
> 
> /dev/nvme1n1
> /dev/nvme1n2
>   - namespaces for subsys X
> 
> /dev/nvme2
>   - chardev subsys X ctl 2
> 
> /dev/nvme3
>   - chardev subsys Y ctl 1
> 
> ...
> 
> This should work.  Not sure it really buys us so much, though.
> 
> Btw, I wonder if we should have udev rules for:
> 
> /dev/nvme-subsys-$NQN-ctrl1
> /dev/nvme-subsys-$NQN-ctrl2
> /dev/nvme-subsys-$NQN-ns1
> /dev/nvme-subsys-$NQN-ns2
> 
> I have to admit that since udev/systemd moved to github I can't
> be bother to send them patches, though as it would require signing
> up for that crappy service.
> 
That rather calls for persistent device names, no?

proposal would be to create symlinks like

/dev/disk/by-id/nvme-subsys-$NQN-ns-$NS

to keep in line with the other symlinks in /dev/disk/by-id/
The controller symlinks don't really fall into this category, though;
we could create a 'nvme' subdirectory (much like MD does), and
create symlinks in there.

And yes, I'm happy to draft up udev rules once we've come to a 
conclusion here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG N?rnberg)

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Keith Busch]

On Wed, May 15, 2019@11:46:51PM -0700, Christoph Hellwig wrote:
> On Wed, May 15, 2019@03:33:51PM -0600, Keith Busch wrote:
> > So here's a solution that no one will like: pull subsystem and controller
> > instances from the same IDA so that there won't be any namespace block
> > devices with a matching controller handle name. While this does nothing
> > to clear up device relationships, this will force the user to think
> > really hard about what they're doing and avoid such mistakes.
> 
> Hmm.  So we'll get:
> 
> /dev/nvme0
>  - chardev subsys X ctl 1
> 
> /dev/nvme1n1
> /dev/nvme1n2
>  - namespaces for subsys X
> 
> /dev/nvme2
>  - chardev subsys X ctl 2
> 
> /dev/nvme3
>  - chardev subsys Y ctl 1
> 
> ...
> 
> This should work.  Not sure it really buys us so much, though.

Right, it's not much. The only thing I want accomplish with this is to remove any excuse for
sending commands to the wrong handle. We can insist that it's wrong 
 
> Btw, I wonder if we should have udev rules for:
> 
> /dev/nvme-subsys-$NQN-ctrl1
> /dev/nvme-subsys-$NQN-ctrl2
> /dev/nvme-subsys-$NQN-ns1
> /dev/nvme-subsys-$NQN-ns2
> 
> I have to admit that since udev/systemd moved to github I can't
> be bother to send them patches, though as it would require signing
> up for that crappy service.

I can send it on your behalf if you have a rule in mind.

We do already have /dev/disk/by-{path,id,uuid} rules already, if that's
what you're looking for.

We don't have rules for the chardev though. Those devices are skipped in
60-persistent-naming.rules.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Keith Busch]

On Thu, May 16, 2019@12:03:54AM -0700, Hannes Reinecke wrote:
> On 5/16/19 8:46 AM, Christoph Hellwig wrote:
> > On Wed, May 15, 2019@03:33:51PM -0600, Keith Busch wrote:
> >> So here's a solution that no one will like: pull subsystem and controller
> >> instances from the same IDA so that there won't be any namespace block
> >> devices with a matching controller handle name. While this does nothing
> >> to clear up device relationships, this will force the user to think
> >> really hard about what they're doing and avoid such mistakes.
> > 
> > Hmm.  So we'll get:
> > 
> > /dev/nvme0
> >   - chardev subsys X ctl 1
> > 
> > /dev/nvme1n1
> > /dev/nvme1n2
> >   - namespaces for subsys X
> > 
> > /dev/nvme2
> >   - chardev subsys X ctl 2
> > 
> > /dev/nvme3
> >   - chardev subsys Y ctl 1
> > 
> > ...
> > 
> > This should work.  Not sure it really buys us so much, though.
> > 
> > Btw, I wonder if we should have udev rules for:
> > 
> > /dev/nvme-subsys-$NQN-ctrl1
> > /dev/nvme-subsys-$NQN-ctrl2
> > /dev/nvme-subsys-$NQN-ns1
> > /dev/nvme-subsys-$NQN-ns2
> > 
> > I have to admit that since udev/systemd moved to github I can't
> > be bother to send them patches, though as it would require signing
> > up for that crappy service.
> > 
> That rather calls for persistent device names, no?
> 
> proposal would be to create symlinks like
> 
> /dev/disk/by-id/nvme-subsys-$NQN-ns-$NS

Hm, the current by-id rules use the namespace's nguid or eui if it exists,
and serial + nsid if not. We can add susbsys too.

> to keep in line with the other symlinks in /dev/disk/by-id/
> The controller symlinks don't really fall into this category, though;
> we could create a 'nvme' subdirectory (much like MD does), and
> create symlinks in there.

Cool, I would also like to see persistent char dev names, but I wasn't
sure where they'd go. If we can make our own 'nvme' subdirectory, that'd
be great.
 
> And yes, I'm happy to draft up udev rules once we've come to a 
> conclusion here.

Thanks!

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Christoph Hellwig]

On Thu, May 16, 2019@08:44:52AM -0600, Keith Busch wrote:
> > /dev/disk/by-id/nvme-subsys-$NQN-ns-$NS
> 
> Hm, the current by-id rules use the namespace's nguid or eui if it exists,
> and serial + nsid if not. We can add susbsys too.

Anything that supports the subsystem NQN must support euid, nguid or
uuid, so I'm not sure it is worth it.

We still need good human readable names for the character devices,
though.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Keith Busch]

On Thu, May 16, 2019@08:40:17AM -0600, Keith Busch wrote:
> On Wed, May 15, 2019@11:46:51PM -0700, Christoph Hellwig wrote:
> > On Wed, May 15, 2019@03:33:51PM -0600, Keith Busch wrote:
> > > So here's a solution that no one will like: pull subsystem and controller
> > > instances from the same IDA so that there won't be any namespace block
> > > devices with a matching controller handle name. While this does nothing
> > > to clear up device relationships, this will force the user to think
> > > really hard about what they're doing and avoid such mistakes.
> > 
> > Hmm.  So we'll get:
> > 
> > /dev/nvme0
> >  - chardev subsys X ctl 1
> > 
> > /dev/nvme1n1
> > /dev/nvme1n2
> >  - namespaces for subsys X
> > 
> > /dev/nvme2
> >  - chardev subsys X ctl 2
> > 
> > /dev/nvme3
> >  - chardev subsys Y ctl 1
> > 
> > ...
> > 
> > This should work.  Not sure it really buys us so much, though.
> 
> Right, it's not much. The only thing I want accomplish with this is to remove any excuse for
> sending commands to the wrong handle. We can insist that it's wrong 

[finishing the sentence]

We can insist that it's wrong for anyone to have assumed a chardev is the
controller of a specific blkdev, but amount of feedback I've received
indicates the current naming is a little too easy for people to shoot
themselves in the foot. This new naming will put a stop to such mistakes
without breaking anything.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Martin K. Petersen]

Keith,

>> to keep in line with the other symlinks in /dev/disk/by-id/ The
>> controller symlinks don't really fall into this category, though; we
>> could create a 'nvme' subdirectory (much like MD does), and create
>> symlinks in there.
>
> Cool, I would also like to see persistent char dev names, but I wasn't
> sure where they'd go. If we can make our own 'nvme' subdirectory,
> that'd be great.

Should be fine. As long as /dev/disk stays intact.

It would be really nice to have a clear indication of
subsys/controller/namespace relationships. So much confusion as a result
of the current approach...

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Max Gurtovoy]

Hi Martin,

On 5/16/2019 5:58 PM, Martin K. Petersen wrote:
> Keith,
>
>>> to keep in line with the other symlinks in /dev/disk/by-id/ The
>>> controller symlinks don't really fall into this category, though; we
>>> could create a 'nvme' subdirectory (much like MD does), and create
>>> symlinks in there.
>> Cool, I would also like to see persistent char dev names, but I wasn't
>> sure where they'd go. If we can make our own 'nvme' subdirectory,
>> that'd be great.
> Should be fine. As long as /dev/disk stays intact.
>
> It would be really nice to have a clear indication of
> subsys/controller/namespace relationships. So much confusion as a result
> of the current approach...

you can use the nvme-cli. e.g (2 subsystems with 2 ns each exposed from 
target side):

Host

========

[root at server50 ~]# nvme list-subsys
nvme-subsys0 - NQN=testsubsystem_0
\
 ?+- nvme0 rdma traddr=11.212.140.146 trsvcid=4420
nvme-subsys1 - NQN=testsubsystem_1
\
 ?+- nvme1 rdma traddr=11.212.140.146 trsvcid=4421


[root at server50 ~]# nvme list-ns /dev/nvme0
[?? 0]:0x1
[?? 1]:0x2
[root at server50 ~]#
[root at server50 ~]# nvme list-ns /dev/nvme1
[?? 0]:0x1
[?? 1]:0x2


maybe we can improve it and print also the namespace during the "nvme 
list-subsys" command.

something like:

[root at server50 ~]# nvme list-subsys
nvme-subsys0 - NQN=testsubsystem_0
\
 ?+- nvme0 rdma traddr=11.212.140.146 trsvcid=4420

 ?\

 ? +- nvme0n1 SN=cf8bbff661502c51 Model=Linux

 ? +- nvme0n2 SN=cf8bbff661502c51 Model=Linux


and we'll get subsys/ctrl/ns relations in 1 cmd.

thoughts ?

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Keith Busch]

On Tue, May 21, 2019@02:54:53AM -0700, Max Gurtovoy wrote:
> maybe we can improve it and print also the namespace during the "nvme 
> list-subsys" command.
> 
> something like:
> 
> [root at server50 ~]# nvme list-subsys
> nvme-subsys0 - NQN=testsubsystem_0
> \
>  ?+- nvme0 rdma traddr=11.212.140.146 trsvcid=4420
>  ?\
>  ? +- nvme0n1 SN=cf8bbff661502c51 Model=Linux
>  ? +- nvme0n2 SN=cf8bbff661502c51 Model=Linux
> 
> 
> and we'll get subsys/ctrl/ns relations in 1 cmd.
> 
> thoughts ?

Yes, this is very useful! I'd like to add this as soon as possible.
Do you have a patch, or is this just an example?  The only change I'd
recommend is remove SN and Model since those are controller properties
rather than from the namespace.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Max Gurtovoy]

On 5/21/2019 5:35 PM, Keith Busch wrote:
> On Tue, May 21, 2019@02:54:53AM -0700, Max Gurtovoy wrote:
>> maybe we can improve it and print also the namespace during the "nvme
>> list-subsys" command.
>>
>> something like:
>>
>> [root at server50 ~]# nvme list-subsys
>> nvme-subsys0 - NQN=testsubsystem_0
>> \
>>   ?+- nvme0 rdma traddr=11.212.140.146 trsvcid=4420
>>   ?\
>>   ? +- nvme0n1 SN=cf8bbff661502c51 Model=Linux
>>   ? +- nvme0n2 SN=cf8bbff661502c51 Model=Linux
>>
>>
>> and we'll get subsys/ctrl/ns relations in 1 cmd.
>>
>> thoughts ?
> Yes, this is very useful! I'd like to add this as soon as possible.
> Do you have a patch, or is this just an example?  The only change I'd
> recommend is remove SN and Model since those are controller properties
> rather than from the namespace.

This is just an example but I can prepare something in a couple of days.

Let me know what do you prefer (and which attrs to print there)

-Max.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Keith Busch]

On Tue, May 21, 2019@05:51:25PM +0300, Max Gurtovoy wrote:
> 
> On 5/21/2019 5:35 PM, Keith Busch wrote:
> > On Tue, May 21, 2019@02:54:53AM -0700, Max Gurtovoy wrote:
> > > maybe we can improve it and print also the namespace during the "nvme
> > > list-subsys" command.
> > > 
> > > something like:
> > > 
> > > [root at server50 ~]# nvme list-subsys
> > > nvme-subsys0 - NQN=testsubsystem_0
> > > \
> > >   ?+- nvme0 rdma traddr=11.212.140.146 trsvcid=4420
> > >   ?\
> > >   ? +- nvme0n1 SN=cf8bbff661502c51 Model=Linux
> > >   ? +- nvme0n2 SN=cf8bbff661502c51 Model=Linux
> > > 
> > > 
> > > and we'll get subsys/ctrl/ns relations in 1 cmd.
> > > 
> > > thoughts ?
> > Yes, this is very useful! I'd like to add this as soon as possible.
> > Do you have a patch, or is this just an example?  The only change I'd
> > recommend is remove SN and Model since those are controller properties
> > rather than from the namespace.
> 
> This is just an example but I can prepare something in a couple of days.
> 
> Let me know what do you prefer (and which attrs to print there)

Let's just get the namespace handles (ex nvme0n1) displaying with each
of their controllers within a subsystem. Showing just this relationship
will be immediately helpful. We can can add verbose flags to get more
details in the future.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Sagi Grimberg]

>> maybe we can improve it and print also the namespace during the "nvme
>> list-subsys" command.
>>
>> something like:
>>
>> [root at server50 ~]# nvme list-subsys
>> nvme-subsys0 - NQN=testsubsystem_0
>> \
>>   ?+- nvme0 rdma traddr=11.212.140.146 trsvcid=4420
>>   ?\
>>   ? +- nvme0n1 SN=cf8bbff661502c51 Model=Linux
>>   ? +- nvme0n2 SN=cf8bbff661502c51 Model=Linux
>>
>>
>> and we'll get subsys/ctrl/ns relations in 1 cmd.
>>
>> thoughts ?
> 
> Yes, this is very useful! I'd like to add this as soon as possible.
> Do you have a patch, or is this just an example?  The only change I'd
> recommend is remove SN and Model since those are controller properties
> rather than from the namespace.

This now gives some context to the series Max posted :)

I still don't understand how this is helping more than the existing
list-subsys for what you're trying to achieve keith.

Your use-case was "I want to understand what is my controller for 
/dev/nvme0n1". This information is already available in nvme list
subsys. Why do you need the mpath-slave devices listed as well?

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Keith Busch]

On Fri, May 24, 2019@01:03:07AM -0700, Sagi Grimberg wrote:
> 
> >> maybe we can improve it and print also the namespace during the "nvme
> >> list-subsys" command.
> >>
> >> something like:
> >>
> >> [root at server50 ~]# nvme list-subsys
> >> nvme-subsys0 - NQN=testsubsystem_0
> >> \
> >>   ?+- nvme0 rdma traddr=11.212.140.146 trsvcid=4420
> >>   ?\
> >>   ? +- nvme0n1 SN=cf8bbff661502c51 Model=Linux
> >>   ? +- nvme0n2 SN=cf8bbff661502c51 Model=Linux
> >>
> >>
> >> and we'll get subsys/ctrl/ns relations in 1 cmd.
> >>
> >> thoughts ?
> > 
> > Yes, this is very useful! I'd like to add this as soon as possible.
> > Do you have a patch, or is this just an example?  The only change I'd
> > recommend is remove SN and Model since those are controller properties
> > rather than from the namespace.
> 
> This now gives some context to the series Max posted :)
> 
> I still don't understand how this is helping more than the existing
> list-subsys for what you're trying to achieve keith.
>
> Your use-case was "I want to understand what is my controller for 
> /dev/nvme0n1". This information is already available in nvme list
> subsys. Why do you need the mpath-slave devices listed as well?

The current 'list-subsys' does not show us any namespaces, actually.
You have to dig through sysfs on your own to see which namespaces belong
to which controllers, but Max has added a more user friendly way to
view this.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Sagi Grimberg]

>> Your use-case was "I want to understand what is my controller for
>> /dev/nvme0n1". This information is already available in nvme list
>> subsys. Why do you need the mpath-slave devices listed as well?
> 
> The current 'list-subsys' does not show us any namespaces, actually.
> You have to dig through sysfs on your own to see which namespaces belong
> to which controllers, but Max has added a more user friendly way to
> view this.

I was referring to your example in the change log:
"For example,
I want to format /dev/nvme0n1 and /dev/nvme0n2, so I'll run:

   # nvme format /dev/nvme0

to hit up all namespaces attached to nvme0. The problem is that nvme0's
namespaces may not even be those desired namespaces, and now I've lost
data on devices I wished to preserve."

You can still run: nvme list-subsys for each namespace and understand
which controller contains each volume so you can format correctly.

I personally think that this display, while more informative, is not
intuitive to read, mainly because we don't get a display of the mapping
between the mpath and slave devices.

Maybe for starters, when passing a namespace handle to list-subsys, the
output should display only the slave devices from each controller.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Max Gurtovoy]

On 5/24/2019 7:17 PM, Sagi Grimberg wrote:
>
>>> Your use-case was "I want to understand what is my controller for
>>> /dev/nvme0n1". This information is already available in nvme list
>>> subsys. Why do you need the mpath-slave devices listed as well?
>>
>> The current 'list-subsys' does not show us any namespaces, actually.
>> You have to dig through sysfs on your own to see which namespaces belong
>> to which controllers, but Max has added a more user friendly way to
>> view this.
>
> I was referring to your example in the change log:
> "For example,
> I want to format /dev/nvme0n1 and /dev/nvme0n2, so I'll run:
>
> ? # nvme format /dev/nvme0
>
> to hit up all namespaces attached to nvme0. The problem is that nvme0's
> namespaces may not even be those desired namespaces, and now I've lost
> data on devices I wished to preserve."
>
> You can still run: nvme list-subsys for each namespace and understand
> which controller contains each volume so you can format correctly.
>
> I personally think that this display, while more informative, is not
> intuitive to read, mainly because we don't get a display of the mapping
> between the mpath and slave devices.

Sagi,

my original patch showed the namespaces and not the mpath device since I 
developed it in non mpath environment.

Afterwards, Keith asked me to test it on mpath env and I changed my V1 
to show the mpath device (in case of mpath configuration) and "IO-ble" 
block device in case of non mpath configuration.

I thought that this was Keith intention but afterwards we agreed to 
print the "IO-ble" namespace in all the cases (Also Christoph agreed on 
that).

So I'll update the patchset to show always the "IO-ble" namespace and 
not mpath slaves that a user can't do anything interested with them.


>
> Maybe for starters, when passing a namespace handle to list-subsys, the
> output should display only the slave devices from each controller.

I see "nvme list-subsys" as "nvme list" (without any params passed). I 
don't see why one will pass namespace handle to it. maybe we can pass 
subsystem handle/name to show the description of the this specific 
subsystem:

nvme list-subsys -n nvme-subsys3

or for transport

nvme list-subsys -t tcp

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Sagi Grimberg]

> Sagi,
> 
> my original patch showed the namespaces and not the mpath device since I 
> developed it in non mpath environment.
> 
> Afterwards, Keith asked me to test it on mpath env and I changed my V1 
> to show the mpath device (in case of mpath configuration) and "IO-ble" 
> block device in case of non mpath configuration.
> 
> I thought that this was Keith intention but afterwards we agreed to 
> print the "IO-ble" namespace in all the cases (Also Christoph agreed on 
> that).

What's IO-ble?

> So I'll update the patchset to show always the "IO-ble" namespace and 
> not mpath slaves that a user can't do anything interested with them.

But a namespace in a multipath environment is not associated with a
controller. I don;t understand how you can display that without
confusing the user even more.

Unless they are not nested under controllers but under subsystems, that
is fine, and aligned with the spec.

>> Maybe for starters, when passing a namespace handle to list-subsys, the
>> output should display only the slave devices from each controller.
> 
> I see "nvme list-subsys" as "nvme list" (without any params passed). I 
> don't see why one will pass namespace handle to it.

Because the user wants to be able to associate controller(s) with
namespaces (which is exactly what keith listed in the change log!).

That user could have simply run: 'nvme list-subsys /dev/nvme0n1' and it
would immediately be able to associate the controller to format.

Overall, I don't understand what this display is buying us.

[PATCH RFC] nvme: Common subsys and controller instances IDA

[Keith Busch]

On Fri, May 24, 2019@04:23:36PM -0700, Sagi Grimberg wrote:
> 
> > Sagi,
> > 
> > my original patch showed the namespaces and not the mpath device since I
> > developed it in non mpath environment.
> > 
> > Afterwards, Keith asked me to test it on mpath env and I changed my V1
> > to show the mpath device (in case of mpath configuration) and "IO-ble"
> > block device in case of non mpath configuration.
> > 
> > I thought that this was Keith intention but afterwards we agreed to
> > print the "IO-ble" namespace in all the cases (Also Christoph agreed on
> > that).
> 
> What's IO-ble?

Just means "not hidden".
 
> > So I'll update the patchset to show always the "IO-ble" namespace and
> > not mpath slaves that a user can't do anything interested with them.
> 
> But a namespace in a multipath environment is not associated with a
> controller. I don;t understand how you can display that without
> confusing the user even more.
>
> Unless they are not nested under controllers but under subsystems, that
> is fine, and aligned with the spec.

The virtual device "head" is associated with a subsystem where its
device names comes from, but that head has namespace paths, which are
controllers, and this is just showing that topology.

It would not make sense to nest the block device names under the
subsystem because the substem may have many paths/controllers, but the
namespaces are accessible only under a subset of those paths.