Re: [PATCH] qla2xxx: Add SysFS hook for FC-NVMe autoconnect

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/15/18 5:40 PM, Ewan D. Milne wrote:
On Thu, 2018-11-15 at 13:52 +0100, Hannes Reinecke wrote:
On 11/14/18 6:13 PM, Ewan D. Milne wrote:
On Tue, 2018-11-13 at 09:49 -0800, Bart Van Assche wrote:
On Tue, 2018-11-13 at 17:38 +0000, Madhani, Himanshu wrote:
On Nov 13, 2018, at 6:23 AM, Bart Van Assche <bvanassche@xxxxxxx> wrote:
On Tue, 2018-11-13 at 01:02 +0000, Madhani, Himanshu wrote:
I see other drivers also use similar information populated for NVMe
Connection at boot time.

Hi Himanshu,

Which other drivers are you referring to?

Thanks,

Bart.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/dri
vers/scsi/lpfc/lpfc_attr.c

Hello James,

Please either move the information exported by lpfc_nvme_info_show() into
debugfs or split the information exported by that function into multiple
sysfs attributes. Otherwise you will get flamed by Greg KH as soon as he
encounters that code because of not following the rules explained in
Documentation/filesystems/sysfs.txt.

Bart.

Better yet, is there some way we could get at least the connection
information needed for discovery moved into the NVMe/FC transport
layer so that we don't need to have separate implementations that
have to be parsed?


Sigh.

It's already been solved, there's no need for this attribute.

nvme-fc already sends a uevent whenever a new nvme rport is attached,
(check nvme_fc_register_remoteport()->nvme_fc_signal_discovery_scan().

James Smart added a sysfs entry for retriggering these uevents, and I've
posted a set of scripts (... at least I think I did :-) utilizing those.

Please do _NOT_ do this; better rely on common mechanisms here.

And please, next time please cc linux-nvme so that we can track it better.

Cheers,

Hannes

So, the uevent can take care of the discovery mechanism, however there
are other reasons why one would want to be able to know e.g. what the
WWPNs of the Initiator & Target ports of the NVMe connections are.
For example, we would typically want to capture this information when
troubleshooting a system remotely.  There are also SAN test environments
that have a need for this information, which is one of the reasons
for the original patch posting.  I don't think it matters too much how
we do it, but the information is needed somehow.

I suppose a udev rule could put the information into a file, but that
seems awkard, and the infomation neeeded is in kernel objects, so why
not make it visible?

The uevent is of course necessary as it signifies an event, but the
desired info here is the *state*.

I somewhat concur here in that we really do need some sysfs entry holding the remote port information. However, lumping this all into one file and locate that file in the _SCSI_ host sysfs directory is _so_ fundamentally wrong :-)

What we should be doing is coming up with an abstraction here, allowing for 'real' FC remote port sysfs entries for NVMe. At the moment FC-NVMe has loads of internal structures (cf nvme_fc_lport, nvme_fc_rport) which are never _ever_ mirrored in sysfs. And that is the real issue we should be addressing; once we have these structures in sysfs we won't need to invest in some band-aids like the 'info' fields.

Cheers,

Hannes
--
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@xxxxxxx			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux