linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
@ 2008-10-17  6:55 Nicholas A. Bellinger
  2008-10-17  7:44 ` Greg KH
  2008-10-17 19:39 ` Joel Becker
  0 siblings, 2 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2008-10-17  6:55 UTC (permalink / raw)
  To: Joel Becker, Greg KH
  Cc: LKML, Linux-fsdevel, linux-scsi, Linux-iSCSI.org Target Dev,
	SCST-Devel, Alan Stern, Andrew Morton, Christoph Hellwig

[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]

Hi Joel, Greg and Co,

Here is the the first working code for allowing configfs to handle
symlinks from sysfs struct kobject based code.  Here is the commit:
passing struct kobject into generic target_core_mod subsystem plugins
for locating struct block_device and struct scsi_device..  

-------------------------------------------------------------------------

In struct configfs_item_operations, added allow_link_kobject() and
drop_link_kobject() to allow struct kobject to be passed through
the configfs API.  The code for kobject symlink source sits along side
the exist configfs symlink code, and does not break existing apps that use
allow_link() or drop_link().

Also, there is two large FIXMEs with the first commit,  in
fs/configfs/symlink.c:configfs_symlink() related to comparing
struct nameidata nd.path.dentry->d_sb == sysfs_sb.

The 2nd is when doing a 'ls -la' inside of the config group directory
containg the symlink source from sysfs, the source link does not currently
appear..  I will be following up these two items..

Signed-off-by: Nicholas A. Bellinger
---
 fs/configfs/configfs_internal.h |    3 +-
 fs/configfs/symlink.c           |  168 +++++++++++++++++++++++++++------------
 include/linux/configfs.h        |    6 +-
 3 files changed, 123 insertions(+), 54 deletions(-)

the following commit can be found at:

http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=commit;h=317aefe74354c0140dc8aebff1ef9467b6f0c9ba

Comments..?  I will send along my patch to
drivers/lio-core/target_core_configfs.c in a moment to demonstrate how
to pull it all together with struct config_item_operations
allow_link_kobject() and drop_link_kobject()..

[-- Attachment #2: 0001-ConfigFS-Allow-symbolic-links-from-a-SysFS-struct.patch --]
[-- Type: application/mbox, Size: 9616 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17  6:55 [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source Nicholas A. Bellinger
@ 2008-10-17  7:44 ` Greg KH
  2008-10-17  8:22   ` Nicholas A. Bellinger
  2008-10-17 19:39 ` Joel Becker
  1 sibling, 1 reply; 14+ messages in thread
From: Greg KH @ 2008-10-17  7:44 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Joel Becker, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Thu, Oct 16, 2008 at 11:55:55PM -0700, Nicholas A. Bellinger wrote:
> Hi Joel, Greg and Co,
> 
> Here is the the first working code for allowing configfs to handle
> symlinks from sysfs struct kobject based code.  Here is the commit:
> passing struct kobject into generic target_core_mod subsystem plugins
> for locating struct block_device and struct scsi_device..  

Um, no.

You are trying to create symlinks dynamically across superblocks and
mount points?  As one of your #warning states, this isn't possible to do
correctly, nor is it even a good idea.

So I'd have to reject this patch, sorry.

What is the problem you are attempting to solve here?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17  7:44 ` Greg KH
@ 2008-10-17  8:22   ` Nicholas A. Bellinger
  2008-10-17 11:32     ` Matthew Wilcox
                       ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2008-10-17  8:22 UTC (permalink / raw)
  To: Greg KH
  Cc: Joel Becker, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, 2008-10-17 at 00:44 -0700, Greg KH wrote:
> On Thu, Oct 16, 2008 at 11:55:55PM -0700, Nicholas A. Bellinger wrote:
> > Hi Joel, Greg and Co,
> > 
> > Here is the the first working code for allowing configfs to handle
> > symlinks from sysfs struct kobject based code.  Here is the commit:
> > passing struct kobject into generic target_core_mod subsystem plugins
> > for locating struct block_device and struct scsi_device..  
> 
> Um, no.
> 
> You are trying to create symlinks dynamically across superblocks and
> mount points?  As one of your #warning states, this isn't possible to do
> correctly, nor is it even a good idea.
> 
> So I'd have to reject this patch, sorry.
> 
> What is the problem you are attempting to solve here?
> 

So, the generic target_core_mod engine that lives
under /sys/kernel/config/target/core needs a method to locate struct
block_device and struct scsi_device for access via bio_submit() and
scsi_execute_() respectively.

Originally, target_core_mod used key echoed through configfs attributes
like so:

export TARGET=/sys/kernel/config/target/core/

# Create $STORAGE_OBJECT of type Linux/BLOCK in generic target_core_mod
mkdir -p $TARGET/iblock_0/lvm_test0
# OLD METHOD to reference struct block_device
echo iblock_major=254,iblock_minor=2 > $TARGET/iblock_0/lvm_test0/control
echo 1 > $TARGET/iblock_0/lvm_test0/enable
# NEW METHOD using sysfs ->configfs symlinks to reference struct block_device
ln -s /sys/block/dm-2 $TARGET/iblock_0/lvm_test0/dm-2

# Create $STORAGE_OBJET of type Linux/SCSI in generic target_core_mod
mkdir -p $TARGET/pscsi_0/sdd
# OLD METHOD to reference struct scsi_device
echo scsi_channel_id=0,scsi_target_id=3,scsi_lun_id=0 > $TARGET/pscsi_0/sdd/control
echo 1 > $TARGET/pscsi_0/sdd/enable
# NEW METHOD using sysfs -> configfs symlinks to reference struct scsi_device
ln -s /sys/block/sdd/device $TARGET/pscsi_0/sdd/passthrough

This of course means that non TYPE_DISK of struct scsi_device would need
to appear under /sys/ to be referenced using the symlink method..  I
know that drivers/scsi/sr.c currently does not show up under /sys..

Anyways, the idea is that its *ONLY* this special case for struct
kobject as symlink source to struct config_item as symlink destination
through the config struct config_item_operations API..  For the generic
target_core_mod engine case, it seems like a good method to reference
block and scsi related data structures for struct devices that folks
want to use for target export across one or more $FABRIC modules (iSCSI,
SAS, FC, PSCSI, etc)..

I am more than happy to change the patch if there is a better way to get
the same functionality for target_core_mod  I would really not prefer
having to do targetfs, when configfs + sysfs symlinks seems to work in
my first few hours of testing, unless there is something huge that I am
missing..?.

The other point is that since configfs is based on sysfs code, it only
makes sense to provide a API through configfs to take advantage of
kernel data structures that can be referenced using wrappers to
container_of() in include/linux/ in a common way.

--nab

> thanks,
> 
> greg k-h
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17  8:22   ` Nicholas A. Bellinger
@ 2008-10-17 11:32     ` Matthew Wilcox
  2008-10-17 19:02       ` Nicholas A. Bellinger
  2008-10-17 17:39     ` Greg KH
  2008-10-17 19:48     ` Joel Becker
  2 siblings, 1 reply; 14+ messages in thread
From: Matthew Wilcox @ 2008-10-17 11:32 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Greg KH, Joel Becker, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> This of course means that non TYPE_DISK of struct scsi_device would need
> to appear under /sys/ to be referenced using the symlink method..  I
> know that drivers/scsi/sr.c currently does not show up under /sys..

Hmm?

$ ls -l /sys/block/sr0/
total 0
lrwxrwxrwx 1 root root    0 2008-10-17 07:30 bdi -> ../../class/bdi/11:0
-r--r--r-- 1 root root 4096 2008-10-17 07:30 capability
-r--r--r-- 1 root root 4096 2008-10-17 07:30 dev
lrwxrwxrwx 1 root root    0 2008-10-17 07:30 device -> ../../devices/pci0000:00/0000:00:1f.1/host3/target3:0:0/3:0:0:0
drwxr-xr-x 2 root root    0 2008-10-17 07:30 holders
drwxr-xr-x 2 root root    0 2008-10-17 07:30 power
drwxr-xr-x 3 root root    0 2008-10-17 07:30 queue
-r--r--r-- 1 root root 4096 2008-10-17 07:30 range
-r--r--r-- 1 root root 4096 2008-10-17 07:30 removable
-r--r--r-- 1 root root 4096 2008-10-17 07:30 ro
-r--r--r-- 1 root root 4096 2008-10-17 07:30 size
drwxr-xr-x 2 root root    0 2008-10-17 07:30 slaves
-r--r--r-- 1 root root 4096 2008-10-17 07:30 stat
lrwxrwxrwx 1 root root    0 2008-10-17 07:30 subsystem -> ../../block
-rw-r--r-- 1 root root 4096 2008-10-17 07:30 uevent

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17  8:22   ` Nicholas A. Bellinger
  2008-10-17 11:32     ` Matthew Wilcox
@ 2008-10-17 17:39     ` Greg KH
  2008-10-17 19:19       ` Nicholas A. Bellinger
  2008-10-17 19:48     ` Joel Becker
  2 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2008-10-17 17:39 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Joel Becker, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> On Fri, 2008-10-17 at 00:44 -0700, Greg KH wrote:
> > On Thu, Oct 16, 2008 at 11:55:55PM -0700, Nicholas A. Bellinger wrote:
> > > Hi Joel, Greg and Co,
> > > 
> > > Here is the the first working code for allowing configfs to handle
> > > symlinks from sysfs struct kobject based code.  Here is the commit:
> > > passing struct kobject into generic target_core_mod subsystem plugins
> > > for locating struct block_device and struct scsi_device..  
> > 
> > Um, no.
> > 
> > You are trying to create symlinks dynamically across superblocks and
> > mount points?  As one of your #warning states, this isn't possible to do
> > correctly, nor is it even a good idea.
> > 
> > So I'd have to reject this patch, sorry.
> > 
> > What is the problem you are attempting to solve here?
> > 
> 
> So, the generic target_core_mod engine that lives
> under /sys/kernel/config/target/core needs a method to locate struct
> block_device and struct scsi_device for access via bio_submit() and
> scsi_execute_() respectively.

Then just pass the "name" of the block device into a configfs file,
nothing more is needed, right?

> Originally, target_core_mod used key echoed through configfs attributes
> like so:
> 
> export TARGET=/sys/kernel/config/target/core/
> 
> # Create $STORAGE_OBJECT of type Linux/BLOCK in generic target_core_mod
> mkdir -p $TARGET/iblock_0/lvm_test0
> # OLD METHOD to reference struct block_device
> echo iblock_major=254,iblock_minor=2 > $TARGET/iblock_0/lvm_test0/control
> echo 1 > $TARGET/iblock_0/lvm_test0/enable
> # NEW METHOD using sysfs ->configfs symlinks to reference struct block_device
> ln -s /sys/block/dm-2 $TARGET/iblock_0/lvm_test0/dm-2

No, you can't do a symlink across superblocks and expect it to work like
you are thinking internally.

> The other point is that since configfs is based on sysfs code, it only
> makes sense to provide a API through configfs to take advantage of
> kernel data structures that can be referenced using wrappers to
> container_of() in include/linux/ in a common way.

No, just because configfs looks like sysfs internally, does not mean
they are related in any manner.  We have been down this argument many
times in the past, please don't bring it up again, it has no bearing on
this at all.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17 11:32     ` Matthew Wilcox
@ 2008-10-17 19:02       ` Nicholas A. Bellinger
  0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2008-10-17 19:02 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Greg KH, Joel Becker, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, 2008-10-17 at 05:32 -0600, Matthew Wilcox wrote:
> On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> > This of course means that non TYPE_DISK of struct scsi_device would need
> > to appear under /sys/ to be referenced using the symlink method..  I
> > know that drivers/scsi/sr.c currently does not show up under /sys..
> 
> Hmm?
> 
> $ ls -l /sys/block/sr0/
> total 0
> lrwxrwxrwx 1 root root    0 2008-10-17 07:30 bdi -> ../../class/bdi/11:0
> -r--r--r-- 1 root root 4096 2008-10-17 07:30 capability
> -r--r--r-- 1 root root 4096 2008-10-17 07:30 dev
> lrwxrwxrwx 1 root root    0 2008-10-17 07:30 device -> ../../devices/pci0000:00/0000:00:1f.1/host3/target3:0:0/3:0:0:0
> drwxr-xr-x 2 root root    0 2008-10-17 07:30 holders
> drwxr-xr-x 2 root root    0 2008-10-17 07:30 power
> drwxr-xr-x 3 root root    0 2008-10-17 07:30 queue
> -r--r--r-- 1 root root 4096 2008-10-17 07:30 range
> -r--r--r-- 1 root root 4096 2008-10-17 07:30 removable
> -r--r--r-- 1 root root 4096 2008-10-17 07:30 ro
> -r--r--r-- 1 root root 4096 2008-10-17 07:30 size
> drwxr-xr-x 2 root root    0 2008-10-17 07:30 slaves
> -r--r--r-- 1 root root 4096 2008-10-17 07:30 stat
> lrwxrwxrwx 1 root root    0 2008-10-17 07:30 subsystem -> ../../block
> -rw-r--r-- 1 root root 4096 2008-10-17 07:30 uevent
> 

Oh good, I guess I missed that this has been added a while ago for
TYPE_CDROM.  I had been looking at trying to change something with sr0 a
while back, I think it was an incorrectly set max_sectors on the
Playstation3, but I could not find the was to do this with TYPE_CDROM as
is possible with TYPE_DISK.

So now that those two work, what about other SCSI device types that
appear as character devices?  The most obvious ones that come to mind
are TYPE_MEDIUM_CHANGER and TYPE_TAPE.  As they do not appear
under /sys/block, to enable the SCSI passthrough for target_core_mod
using symlinks from sysfs -> configfs using the patch, the sysfs source
needs to point to struct device..  For example, with the current code,
the export of sr0 would now look like:

# Enable sr0 for DVD target passthrough access
mkdir $TARGET/core/pscsi_0/dvd_rom
ln -s /sys/block/sr0/device $TARGET/core/pscsi_0/dvd_rom/dvd_passthrough

So what about for TYPE_TYPE..?  I am guessing you would need to grab the
struct device (to reference the struct scsi_device by struct kobject
using the configfs patch) through /sys/class/$SCSI_HCTL/device, for the
SCSI passthrough case, right..?

# Also enable a SCSI tape for target passthrough access..?
mkdir $TARGET/core/pscsi_1/my_superturbotapearray
ln -s /sys/class/1:0:0:0/device $TARGET/core/pscsi_1/my_superturbotapearray/tape_passthrough

# MEDIUM_CHANGER too..?
mkdir $TARGET/core/pscsi_1/my_superturbomc
ln -s /sys/class/1:0:1:0/device $TARGET/core/pscsi_1/my_superturbomc/mc_passthrough

I don't have real MEDIUM_CHANGER onsite, so if anyone wanted to test
this with my current patches in lio-core-2.6.git please let me know.
I will try hooking up my old SCSI tape drive this weekend and see if
TYPE_TAPE works. :-)

The idea is through the change to allow sysfs source symlinks (the patch
still needs to handle follow_link() correctly) to pass any struct
kobject through the configfs struct config_item_operations API while
calling kobject_get() and kobject_put() to protect/release the reference
to struct kobject inside of fs/configfs/symlink.c.

--nab


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17 17:39     ` Greg KH
@ 2008-10-17 19:19       ` Nicholas A. Bellinger
  2008-10-17 20:10         ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Nicholas A. Bellinger @ 2008-10-17 19:19 UTC (permalink / raw)
  To: Greg KH
  Cc: Joel Becker, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, 2008-10-17 at 10:39 -0700, Greg KH wrote:
> On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> > On Fri, 2008-10-17 at 00:44 -0700, Greg KH wrote:
> > > On Thu, Oct 16, 2008 at 11:55:55PM -0700, Nicholas A. Bellinger wrote:
> > > > Hi Joel, Greg and Co,
> > > > 
> > > > Here is the the first working code for allowing configfs to handle
> > > > symlinks from sysfs struct kobject based code.  Here is the commit:
> > > > passing struct kobject into generic target_core_mod subsystem plugins
> > > > for locating struct block_device and struct scsi_device..  
> > > 
> > > Um, no.
> > > 
> > > You are trying to create symlinks dynamically across superblocks and
> > > mount points?  As one of your #warning states, this isn't possible to do
> > > correctly, nor is it even a good idea.
> > > 
> > > So I'd have to reject this patch, sorry.
> > > 
> > > What is the problem you are attempting to solve here?
> > > 
> > 
> > So, the generic target_core_mod engine that lives
> > under /sys/kernel/config/target/core needs a method to locate struct
> > block_device and struct scsi_device for access via bio_submit() and
> > scsi_execute_() respectively.
> 
> Then just pass the "name" of the block device into a configfs file,
> nothing more is needed, right?
> 

What are the preferred methods for accessing struct block_device and
struct scsi_device from "name"..?

> > Originally, target_core_mod used key echoed through configfs attributes
> > like so:
> > 
> > export TARGET=/sys/kernel/config/target/core/
> > 
> > # Create $STORAGE_OBJECT of type Linux/BLOCK in generic target_core_mod
> > mkdir -p $TARGET/iblock_0/lvm_test0
> > # OLD METHOD to reference struct block_device
> > echo iblock_major=254,iblock_minor=2 > $TARGET/iblock_0/lvm_test0/control
> > echo 1 > $TARGET/iblock_0/lvm_test0/enable
> > # NEW METHOD using sysfs ->configfs symlinks to reference struct block_device
> > ln -s /sys/block/dm-2 $TARGET/iblock_0/lvm_test0/dm-2
> 
> No, you can't do a symlink across superblocks and expect it to work like
> you are thinking internally.
> 

<nod>  As I am coding configfs_follow_symlink() to work with sysfs
source symlinks I am starting to see that.. :-)  I am still going to
hack on it for a bit and see if I can get it running so that it works..

> > The other point is that since configfs is based on sysfs code, it only
> > makes sense to provide a API through configfs to take advantage of
> > kernel data structures that can be referenced using wrappers to
> > container_of() in include/linux/ in a common way.
> 
> No, just because configfs looks like sysfs internally, does not mean
> they are related in any manner.  We have been down this argument many
> times in the past, please don't bring it up again, it has no bearing on
> this at all.
> 

Well, the goal is to use as much existing infrastructure as possible for
the generic target_core_mod when referencing Linux $STORAGE_OBJECTs, be
they struct scsi_device, struct block_device or struct file.. :-)

Having *SOME* type of communication between sysfs and configfs so that
those kernel data structures mentioned above can be accessed through
configfs still does seem like it could be useful..

Thanks for your comments,

--nab



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17  6:55 [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source Nicholas A. Bellinger
  2008-10-17  7:44 ` Greg KH
@ 2008-10-17 19:39 ` Joel Becker
  2008-10-17 19:50   ` Nicholas A. Bellinger
  1 sibling, 1 reply; 14+ messages in thread
From: Joel Becker @ 2008-10-17 19:39 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Greg KH, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Thu, Oct 16, 2008 at 11:55:55PM -0700, Nicholas A. Bellinger wrote:
> Hi Joel, Greg and Co,
> 
> Here is the the first working code for allowing configfs to handle
> symlinks from sysfs struct kobject based code.  Here is the commit:
> passing struct kobject into generic target_core_mod subsystem plugins
> for locating struct block_device and struct scsi_device..  

NAK.

As Greg said, this just doesn't work.  Don't go down this path.

Joel

-- 

"I always thought the hardest questions were those I could not answer.
 Now I know they are the ones I can never ask."
			- Charlie Watkins

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17  8:22   ` Nicholas A. Bellinger
  2008-10-17 11:32     ` Matthew Wilcox
  2008-10-17 17:39     ` Greg KH
@ 2008-10-17 19:48     ` Joel Becker
  2008-10-17 20:03       ` Nicholas A. Bellinger
  2008-10-17 20:07       ` Nicholas A. Bellinger
  2 siblings, 2 replies; 14+ messages in thread
From: Joel Becker @ 2008-10-17 19:48 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Greg KH, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> On Fri, 2008-10-17 at 00:44 -0700, Greg KH wrote:
> > What is the problem you are attempting to solve here?
> > 
> 
> So, the generic target_core_mod engine that lives
> under /sys/kernel/config/target/core needs a method to locate struct
> block_device and struct scsi_device for access via bio_submit() and
> scsi_execute_() respectively.
> 
> Originally, target_core_mod used key echoed through configfs attributes
> like so:
> 
> export TARGET=/sys/kernel/config/target/core/
> 
> # Create $STORAGE_OBJECT of type Linux/BLOCK in generic target_core_mod
> mkdir -p $TARGET/iblock_0/lvm_test0
> # OLD METHOD to reference struct block_device
> echo iblock_major=254,iblock_minor=2 > $TARGET/iblock_0/lvm_test0/control
> echo 1 > $TARGET/iblock_0/lvm_test0/enable
> # NEW METHOD using sysfs ->configfs symlinks to reference struct block_device
> ln -s /sys/block/dm-2 $TARGET/iblock_0/lvm_test0/dm-2

	Pass an open file descriptor.  In bash(1):

    exec 3<>/dev/dm-7
    echo 3 >$TARGET/iblock_0/lvm_test0/control  # I'd call it 'fd'

In kernel, in the ->store() function of 'control':

	p = (char *)page;
	fd = simple_strtol(p, &p, 0);
	filp = fget(fd);
	inode = igrab(filp->f_mapping->host);
	dev = I_BDEV(filp->f_mapping->host);
	blkdev_get(dev, FMODE_WRITE | FMODE_READ, 0);

Error handling is left up to you (validate the strtol, fd range, ISBLK,
etc).  This assumes you want the struct block_device and want to pin it
in memory.  I assume you do.

Joel

-- 

Life's Little Instruction Book #450

	"Don't be afraid to say, 'I need help.'"

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17 19:39 ` Joel Becker
@ 2008-10-17 19:50   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2008-10-17 19:50 UTC (permalink / raw)
  To: Joel Becker
  Cc: Greg KH, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, 2008-10-17 at 12:39 -0700, Joel Becker wrote:
> On Thu, Oct 16, 2008 at 11:55:55PM -0700, Nicholas A. Bellinger wrote:
> > Hi Joel, Greg and Co,
> > 
> > Here is the the first working code for allowing configfs to handle
> > symlinks from sysfs struct kobject based code.  Here is the commit:
> > passing struct kobject into generic target_core_mod subsystem plugins
> > for locating struct block_device and struct scsi_device..  
> 
> NAK.
> 
> As Greg said, this just doesn't work.  Don't go down this path.
> 

Fair enough, I will kill it from my tree. :-)

Thanks for your comments!

--nab




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17 19:48     ` Joel Becker
@ 2008-10-17 20:03       ` Nicholas A. Bellinger
  2008-10-17 20:07       ` Nicholas A. Bellinger
  1 sibling, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2008-10-17 20:03 UTC (permalink / raw)
  To: Joel Becker
  Cc: Greg KH, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, 2008-10-17 at 12:48 -0700, Joel Becker wrote:
> On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> > On Fri, 2008-10-17 at 00:44 -0700, Greg KH wrote:
> > > What is the problem you are attempting to solve here?
> > > 
> > 
> > So, the generic target_core_mod engine that lives
> > under /sys/kernel/config/target/core needs a method to locate struct
> > block_device and struct scsi_device for access via bio_submit() and
> > scsi_execute_() respectively.
> > 
> > Originally, target_core_mod used key echoed through configfs attributes
> > like so:
> > 
> > export TARGET=/sys/kernel/config/target/core/
> > 
> > # Create $STORAGE_OBJECT of type Linux/BLOCK in generic target_core_mod
> > mkdir -p $TARGET/iblock_0/lvm_test0
> > # OLD METHOD to reference struct block_device
> > echo iblock_major=254,iblock_minor=2 > $TARGET/iblock_0/lvm_test0/control
> > echo 1 > $TARGET/iblock_0/lvm_test0/enable
> > # NEW METHOD using sysfs ->configfs symlinks to reference struct block_device
> > ln -s /sys/block/dm-2 $TARGET/iblock_0/lvm_test0/dm-2
> 
> 	Pass an open file descriptor.  In bash(1):
> 
>     exec 3<>/dev/dm-7
>     echo 3 >$TARGET/iblock_0/lvm_test0/control  # I'd call it 'fd'
> 
> In kernel, in the ->store() function of 'control':
> 
> 	p = (char *)page;
> 	fd = simple_strtol(p, &p, 0);
> 	filp = fget(fd);
> 	inode = igrab(filp->f_mapping->host);
> 	dev = I_BDEV(filp->f_mapping->host);
> 	blkdev_get(dev, FMODE_WRITE | FMODE_READ, 0);
> 
> Error handling is left up to you (validate the strtol, fd range, ISBLK,
> etc).  This assumes you want the struct block_device and want to pin it
> in memory.  I assume you do.
> 

<nod>  Sounds good..  I will go about converting the target_core_mod
subsystem plugins to use this method.. 

Thanks Joel!

--nab

> Joel
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17 19:48     ` Joel Becker
  2008-10-17 20:03       ` Nicholas A. Bellinger
@ 2008-10-17 20:07       ` Nicholas A. Bellinger
  1 sibling, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2008-10-17 20:07 UTC (permalink / raw)
  To: Joel Becker
  Cc: Greg KH, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, 2008-10-17 at 12:48 -0700, Joel Becker wrote:
> On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> > On Fri, 2008-10-17 at 00:44 -0700, Greg KH wrote:
> > > What is the problem you are attempting to solve here?
> > > 
> > 
> > So, the generic target_core_mod engine that lives
> > under /sys/kernel/config/target/core needs a method to locate struct
> > block_device and struct scsi_device for access via bio_submit() and
> > scsi_execute_() respectively.
> > 
> > Originally, target_core_mod used key echoed through configfs attributes
> > like so:
> > 
> > export TARGET=/sys/kernel/config/target/core/
> > 
> > # Create $STORAGE_OBJECT of type Linux/BLOCK in generic target_core_mod
> > mkdir -p $TARGET/iblock_0/lvm_test0
> > # OLD METHOD to reference struct block_device
> > echo iblock_major=254,iblock_minor=2 > $TARGET/iblock_0/lvm_test0/control
> > echo 1 > $TARGET/iblock_0/lvm_test0/enable
> > # NEW METHOD using sysfs ->configfs symlinks to reference struct block_device
> > ln -s /sys/block/dm-2 $TARGET/iblock_0/lvm_test0/dm-2
> 
> 	Pass an open file descriptor.  In bash(1):
> 
>     exec 3<>/dev/dm-7
>     echo 3 >$TARGET/iblock_0/lvm_test0/control  # I'd call it 'fd'
> 
> In kernel, in the ->store() function of 'control':
> 
> 	p = (char *)page;
> 	fd = simple_strtol(p, &p, 0);
> 	filp = fget(fd);
> 	inode = igrab(filp->f_mapping->host);
> 	dev = I_BDEV(filp->f_mapping->host);
> 	blkdev_get(dev, FMODE_WRITE | FMODE_READ, 0);
> 
> Error handling is left up to you (validate the strtol, fd range, ISBLK,
> etc).  This assumes you want the struct block_device and want to pin it
> in memory.  I assume you do.
> 

Btw, ideally I would like to do the same type of thing for accessing
drivers/scsi for both TYPE_DISK and non TYPE_DISK..  I will have a look
and see if something is possible to access struct scsi_device in all
device type cases..

--nab


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17 19:19       ` Nicholas A. Bellinger
@ 2008-10-17 20:10         ` Greg KH
  2008-10-18  2:41           ` Nicholas A. Bellinger
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2008-10-17 20:10 UTC (permalink / raw)
  To: Nicholas A. Bellinger
  Cc: Joel Becker, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, Oct 17, 2008 at 12:19:33PM -0700, Nicholas A. Bellinger wrote:
> On Fri, 2008-10-17 at 10:39 -0700, Greg KH wrote:
> > On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> > > On Fri, 2008-10-17 at 00:44 -0700, Greg KH wrote:
> > > > On Thu, Oct 16, 2008 at 11:55:55PM -0700, Nicholas A. Bellinger wrote:
> > > > > Hi Joel, Greg and Co,
> > > > > 
> > > > > Here is the the first working code for allowing configfs to handle
> > > > > symlinks from sysfs struct kobject based code.  Here is the commit:
> > > > > passing struct kobject into generic target_core_mod subsystem plugins
> > > > > for locating struct block_device and struct scsi_device..  
> > > > 
> > > > Um, no.
> > > > 
> > > > You are trying to create symlinks dynamically across superblocks and
> > > > mount points?  As one of your #warning states, this isn't possible to do
> > > > correctly, nor is it even a good idea.
> > > > 
> > > > So I'd have to reject this patch, sorry.
> > > > 
> > > > What is the problem you are attempting to solve here?
> > > > 
> > > 
> > > So, the generic target_core_mod engine that lives
> > > under /sys/kernel/config/target/core needs a method to locate struct
> > > block_device and struct scsi_device for access via bio_submit() and
> > > scsi_execute_() respectively.
> > 
> > Then just pass the "name" of the block device into a configfs file,
> > nothing more is needed, right?
> > 
> 
> What are the preferred methods for accessing struct block_device and
> struct scsi_device from "name"..?

call "find_by_name" for a type (this would be block type) and get the
return value?  Can't remember the actual function call, but it's obvious
if you look at device.h.

> > > Originally, target_core_mod used key echoed through configfs attributes
> > > like so:
> > > 
> > > export TARGET=/sys/kernel/config/target/core/
> > > 
> > > # Create $STORAGE_OBJECT of type Linux/BLOCK in generic target_core_mod
> > > mkdir -p $TARGET/iblock_0/lvm_test0
> > > # OLD METHOD to reference struct block_device
> > > echo iblock_major=254,iblock_minor=2 > $TARGET/iblock_0/lvm_test0/control
> > > echo 1 > $TARGET/iblock_0/lvm_test0/enable
> > > # NEW METHOD using sysfs ->configfs symlinks to reference struct block_device
> > > ln -s /sys/block/dm-2 $TARGET/iblock_0/lvm_test0/dm-2
> > 
> > No, you can't do a symlink across superblocks and expect it to work like
> > you are thinking internally.
> > 
> 
> <nod>  As I am coding configfs_follow_symlink() to work with sysfs
> source symlinks I am starting to see that.. :-)  I am still going to
> hack on it for a bit and see if I can get it running so that it works..

Even if you get it "to work", it's not going to be able to be accepted,
sorry.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source.
  2008-10-17 20:10         ` Greg KH
@ 2008-10-18  2:41           ` Nicholas A. Bellinger
  0 siblings, 0 replies; 14+ messages in thread
From: Nicholas A. Bellinger @ 2008-10-18  2:41 UTC (permalink / raw)
  To: Greg KH
  Cc: Joel Becker, LKML, Linux-fsdevel, linux-scsi,
	Linux-iSCSI.org Target Dev, SCST-Devel, Alan Stern, Andrew Morton,
	Christoph Hellwig

On Fri, 2008-10-17 at 13:10 -0700, Greg KH wrote:
> On Fri, Oct 17, 2008 at 12:19:33PM -0700, Nicholas A. Bellinger wrote:
> > On Fri, 2008-10-17 at 10:39 -0700, Greg KH wrote:
> > > On Fri, Oct 17, 2008 at 01:22:18AM -0700, Nicholas A. Bellinger wrote:
> > > > On Fri, 2008-10-17 at 00:44 -0700, Greg KH wrote:
> > > > > On Thu, Oct 16, 2008 at 11:55:55PM -0700, Nicholas A. Bellinger wrote:
> > > > > > Hi Joel, Greg and Co,
> > > > > > 
> > > > > > Here is the the first working code for allowing configfs to handle
> > > > > > symlinks from sysfs struct kobject based code.  Here is the commit:
> > > > > > passing struct kobject into generic target_core_mod subsystem plugins
> > > > > > for locating struct block_device and struct scsi_device..  
> > > > > 
> > > > > Um, no.
> > > > > 
> > > > > You are trying to create symlinks dynamically across superblocks and
> > > > > mount points?  As one of your #warning states, this isn't possible to do
> > > > > correctly, nor is it even a good idea.
> > > > > 
> > > > > So I'd have to reject this patch, sorry.
> > > > > 
> > > > > What is the problem you are attempting to solve here?
> > > > > 
> > > > 
> > > > So, the generic target_core_mod engine that lives
> > > > under /sys/kernel/config/target/core needs a method to locate struct
> > > > block_device and struct scsi_device for access via bio_submit() and
> > > > scsi_execute_() respectively.
> > > 
> > > Then just pass the "name" of the block device into a configfs file,
> > > nothing more is needed, right?
> > > 
> > 
> > What are the preferred methods for accessing struct block_device and
> > struct scsi_device from "name"..?
> 
> call "find_by_name" for a type (this would be block type) and get the
> return value?  Can't remember the actual function call, but it's obvious
> if you look at device.h.
> 

Got it, thanks..

> > > > Originally, target_core_mod used key echoed through configfs attributes
> > > > like so:
> > > > 
> > > > export TARGET=/sys/kernel/config/target/core/
> > > > 
> > > > # Create $STORAGE_OBJECT of type Linux/BLOCK in generic target_core_mod
> > > > mkdir -p $TARGET/iblock_0/lvm_test0
> > > > # OLD METHOD to reference struct block_device
> > > > echo iblock_major=254,iblock_minor=2 > $TARGET/iblock_0/lvm_test0/control
> > > > echo 1 > $TARGET/iblock_0/lvm_test0/enable
> > > > # NEW METHOD using sysfs ->configfs symlinks to reference struct block_device
> > > > ln -s /sys/block/dm-2 $TARGET/iblock_0/lvm_test0/dm-2
> > > 
> > > No, you can't do a symlink across superblocks and expect it to work like
> > > you are thinking internally.
> > > 
> > 
> > <nod>  As I am coding configfs_follow_symlink() to work with sysfs
> > source symlinks I am starting to see that.. :-)  I am still going to
> > hack on it for a bit and see if I can get it running so that it works..
> 
> Even if you get it "to work", it's not going to be able to be accepted,
> sorry.
> 

<nod> not a problem, it was just something that I wanted to see if it
was even possible.  I will refocus on using the method to locate what
Linux storage objects can be used for target_core_mod that Joel
mentioned and your recommendations.

Thanks,

--nab


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2008-10-18  2:43 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-17  6:55 [PATCH] [ConfigFS]: Allow symbolic links from a SysFS struct kobject source Nicholas A. Bellinger
2008-10-17  7:44 ` Greg KH
2008-10-17  8:22   ` Nicholas A. Bellinger
2008-10-17 11:32     ` Matthew Wilcox
2008-10-17 19:02       ` Nicholas A. Bellinger
2008-10-17 17:39     ` Greg KH
2008-10-17 19:19       ` Nicholas A. Bellinger
2008-10-17 20:10         ` Greg KH
2008-10-18  2:41           ` Nicholas A. Bellinger
2008-10-17 19:48     ` Joel Becker
2008-10-17 20:03       ` Nicholas A. Bellinger
2008-10-17 20:07       ` Nicholas A. Bellinger
2008-10-17 19:39 ` Joel Becker
2008-10-17 19:50   ` Nicholas A. Bellinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).