RE: How to add/drop SCSI drives from within the driver?

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

Thanks for the suggestion. After more exploration, looks like different
distribution have different implementations for /sbin/hotplug. This may
aggravate the issue for applications. For now, we will stick with a wait and
watch after bus scan :-(

Will probe the linux-hotplug-devel@lists.sourceforge.net list for more
pointers

Thanks

===========================
Atul Mukker
Architect, Drivers and BIOS
LSI Logic Corporation


> -----Original Message-----
> From: Patrick Mansfield [mailto:patmans@us.ibm.com] 
> Sent: Tuesday, January 25, 2005 11:52 AM
> To: Mukker, Atul
> Cc: 'James Bottomley'; Linux Kernel; SCSI Mailing List
> Subject: Re: How to add/drop SCSI drives from within the driver?
> 
> Atul -
> 
> On Tue, Jan 25, 2005 at 11:27:36AM -0500, Mukker, Atul wrote:
> > After writing the "- - -" to the scan attribute, the management 
> > applications assume the udev has created the relevant 
> entries in the 
> > /dev directly and try to use the devices _immediately_ and 
> fail to see 
> > the devices
> > 
> > Is there a hotplug event which would tell the management 
> applications 
> > that the device nodes have actually been created now and 
> ready to be used?
> 
> Read the udev man page section, the part right before 
> "FILES". Try putting a script under /etc/dev.d/default/*.dev. 
> Then you can get more specific with an /etc/dev.d/scsi/*.dev 
> script or something else.
> 
> I just tried something simple but did not get it working.
> 
> Try linux-hotplug-devel@lists.sourceforge.net list for help.
> 
> -- Patrick Mansfield
> 

Re: How to add/drop SCSI drives from within the driver?

[Brian King]

Newer kernels also have kobject_uevent, which lets any application use 
netlink to look for hotplug events.

-Brian

Mukker, Atul wrote:
> Thanks for the suggestion. After more exploration, looks like different
> distribution have different implementations for /sbin/hotplug. This may
> aggravate the issue for applications. For now, we will stick with a wait and
> watch after bus scan :-(
> 
> Will probe the linux-hotplug-devel@lists.sourceforge.net list for more
> pointers
> 
> Thanks
> 
> ===========================
> Atul Mukker
> Architect, Drivers and BIOS
> LSI Logic Corporation
> 
> 
> 
>>-----Original Message-----
>>From: Patrick Mansfield [mailto:patmans@us.ibm.com] 
>>Sent: Tuesday, January 25, 2005 11:52 AM
>>To: Mukker, Atul
>>Cc: 'James Bottomley'; Linux Kernel; SCSI Mailing List
>>Subject: Re: How to add/drop SCSI drives from within the driver?
>>
>>Atul -
>>
>>On Tue, Jan 25, 2005 at 11:27:36AM -0500, Mukker, Atul wrote:
>>
>>>After writing the "- - -" to the scan attribute, the management 
>>>applications assume the udev has created the relevant 
>>
>>entries in the 
>>
>>>/dev directly and try to use the devices _immediately_ and 
>>
>>fail to see 
>>
>>>the devices
>>>
>>>Is there a hotplug event which would tell the management 
>>
>>applications 
>>
>>>that the device nodes have actually been created now and 
>>
>>ready to be used?
>>
>>Read the udev man page section, the part right before 
>>"FILES". Try putting a script under /etc/dev.d/default/*.dev. 
>>Then you can get more specific with an /etc/dev.d/scsi/*.dev 
>>script or something else.
>>
>>I just tried something simple but did not get it working.
>>
>>Try linux-hotplug-devel@lists.sourceforge.net list for help.
>>
>>-- Patrick Mansfield
>>
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

> 
> And what do you mean "different implementations for /sbin/hotplug"?
> What distros do not use the standard "linux-hotplug" type 
> scripts, or if not the scripts, the same functionality?

You are right, even though distributions (I checked Red Hat and SuSE) have
different /sbin/hotplug scripts (e.g., SuSE 9.2 will not execute files from
/etc/hotplug.d whereas Red Hat does) udev will be invoked in all cases,
which will take care of creating device nodes.

But our concern is that how would the applications get the cue that udev has
actually created the nodes for the new devices? 

Make sure an agent is called after the, e.g., scsi agents are executed from
/etc/hotplug directory (which happen to be scsi.agent, scsi_device.agent,
scsi_host.agent in one and only scsi.agent in other distribution), by
writing an rc like script?

Or more likely, by placing our agent in /etc/dev.d directory. Unfortunately,
there seems be not a consensus here as well. On system has "default" and
"net" directories and other has "block", "input", "net", "tty"?


Thanks

--------------------------------
Atul Mukker
Architect, RAID Drivers and BIOS
LSI Logic Corporation

Re: How to add/drop SCSI drives from within the driver?

['Patrick Mansfield']

[moving to hotplug list ...]

On Wed, Jan 26, 2005 at 06:23:16PM -0500, Mukker, Atul wrote:

> Or more likely, by placing our agent in /etc/dev.d directory. Unfortunately,
> there seems be not a consensus here as well. On system has "default" and
> "net" directories and other has "block", "input", "net", "tty"?

Those are all kernel subsystem names, and as such all are supported
directory structures for udev, the part that is distro specific is that
they supply different scripts.

There won't be any "scsi" devices since they have no dev (the upper level
drivers have them); sg shows up as scsi_generic. sd is block.

You will still have to figure out via sysfs if you want to run your agent
even for subsystem block (i.e. figure out the host/driver type, I assume
you don't want to run it on hd or standard scsi disk drives).

I don't know why this is an issue for "new" devices, this should be a
problem for you when they first show up; existing and new devs should be
handled the same way.

-- Patrick Mansfield

RE: How to add/drop SCSI drives from within the driver?

[Greg KH]

> Thanks for the suggestion. After more exploration, looks like different
> distribution have different implementations for /sbin/hotplug. This may
> aggravate the issue for applications. For now, we will stick with a wait and
> watch after bus scan :-(

What do you mean?  Just use the /etc/dev.d notification stuff that all
distros that use udev have.

And what do you mean "different implementations for /sbin/hotplug"?
What distros do not use the standard "linux-hotplug" type scripts, or if
not the scripts, the same functionality?

thanks,

greg k-h

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

> 
> > After the new logical drives are created with "- - -" written to the
> > scsi_host scan attribute, there is a highly noticeable delay before
> device
> > names (e.g., sda) appears in the /dev directory. If the management
> > application tried to access the device immediately after creating new,
> the
> > access fails. Putting a 1 second delay helped, but of course this is not
> a
> > deterministic solution.
> >
> > What are the other possibilities?
> 
> Well, how about hotplug.  The device addition actually triggers a hot
> plug event already (there's no need to add anything, it's done by the
> mid-layer), so if you just listen for that, you'll know when the scan
> has detected a device.


After writing the "- - -" to the scan attribute, the management applications
assume the udev has created the relevant entries in the /dev directly and
try to use the devices _immediately_ and fail to see the devices

Is there a hotplug event which would tell the management applications that
the device nodes have actually been created now and ready to be used?

I tried this simple script to re-create the failure. Assume there is one
scsi disk, which is the installation disk. Now load the megaraid driver,
with a few logical drive already created.

# insmod megaraid_mm.ko; insmod megaraid_mbox.ko; ls -l /dev/sd*
<driver loads>
<not all scsi devices are available>

<try again after a brief delay>
# ls -l /dev/sd*	# all devices show up now

Thanks
-Atul

Re: How to add/drop SCSI drives from within the driver?

[Patrick Mansfield]

Atul -

On Tue, Jan 25, 2005 at 11:27:36AM -0500, Mukker, Atul wrote:
> After writing the "- - -" to the scan attribute, the management applications
> assume the udev has created the relevant entries in the /dev directly and
> try to use the devices _immediately_ and fail to see the devices
> 
> Is there a hotplug event which would tell the management applications that
> the device nodes have actually been created now and ready to be used?

Read the udev man page section, the part right before "FILES". Try
putting a script under /etc/dev.d/default/*.dev. Then you can get more
specific with an /etc/dev.d/scsi/*.dev script or something else.

I just tried something simple but did not get it working.

Try linux-hotplug-devel@lists.sourceforge.net list for help.

-- Patrick Mansfield

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

All right! The implementation is complete for this and the driver has
thoroughly gone through testing. Everything looks good except for a minor
glitch.

After the new logical drives are created with "- - -" written to the
scsi_host scan attribute, there is a highly noticeable delay before device
names (e.g., sda) appears in the /dev directory. If the management
application tried to access the device immediately after creating new, the
access fails. Putting a 1 second delay helped, but of course this is not a
deterministic solution.

What are the other possibilities?

Thanks
-Atul Mukker
LSI Logic

> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@SteelEye.com] 
> Sent: Wednesday, December 15, 2004 1:49 PM
> To: Matt Domsch
> Cc: Salyzyn, Mark; Bagalkote, Sreenivas; brking@us.ibm.com; 
> Linux Kernel; SCSI Mailing List; bunk@fs.tum.de; Andrew 
> Morton; Ju, Seokmann; Doelfel, Hardy; Mukker, Atul
> Subject: Re: How to add/drop SCSI drives from within the driver?
> 
> On Wed, 2004-12-15 at 01:24 -0600, Matt Domsch wrote:
> > James, I've been thinking about this a little more, and you 
> may be on 
> > to something here. Let each driver add files as such:
> > 
> > /sys/class/scsi_host
> >  |-- host0
> >  |   |-- add_logical_drive
> >  |   |-- remove_logical_drive
> >  |   `-- rescan_logical_drive
> > 
> > Then we can go 2 ways with this.
> > 1) driver functions directly call scsi_add_device(), 
> > scsi_remove_device(), and something for rescan (option 2 
> handles this 
> > one cleanly for us).  ATM, megaraid_mbox doesn't implement a rescan 
> > function, so this point may be moot.
> > 
> > 2) driver functions call a midlayer library function, which invokes
> >    /sbin/hotplug with appropriate data, and add a new /etc/hotplug.d
> >    helper app which would then write to these files:
> > 
> > /sys/class/scsi_host
> > |-- host0
> > |   |-- scan
> > /sys/devices/pci0000:0x/0000:0x:0x.0/host0
> > |-- 0:0:0:0
> > |   |-- delete
> > |   |-- rescan
> > 
> > to do likewise.
> 
> I'll buy this (option 2).. it seems like a good way to export 
> the megaraid specific information and at the same time 
> integrate it more fully into the evolving hotplug infrastructure.
> 
> James
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-kernel" in the body of a message to 
> majordomo@vger.kernel.org More majordomo info at  
> http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

RE: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Fri, 2005-01-21 at 17:11 -0500, Mukker, Atul wrote:
> All right! The implementation is complete for this and the driver has
> thoroughly gone through testing. Everything looks good except for a minor
> glitch.

That's good news.

> After the new logical drives are created with "- - -" written to the
> scsi_host scan attribute, there is a highly noticeable delay before device
> names (e.g., sda) appears in the /dev directory. If the management
> application tried to access the device immediately after creating new, the
> access fails. Putting a 1 second delay helped, but of course this is not a
> deterministic solution.
> 
> What are the other possibilities?

Well, how about hotplug.  The device addition actually triggers a hot
plug event already (there's no need to add anything, it's done by the
mid-layer), so if you just listen for that, you'll know when the scan
has detected a device.

James

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

> > the management app, there is no getting around knowing HCTL 
> mapping. 
> > The app must know the HCTL quad of a logical drive.
> 
> Actually, if that's all you're trying to do, what about
> 
> echo '- - -' > /sys/class/scsi_host/host<n>/scan
> 
> That'll trigger a rescan of the entire card and the device 
> will be found and added?
For this to happen, the applications must at lease know 'n' in host<n>,
otherwise it will have to trigger a rescan on all controllers. Which would
be an overkill. How about publishing the adapter class attribute as well?
This would allow applications to correlate the adapter handle with the class
attribute.

> 
> Then, if you simply publish your LD number as an extra 
> parameter of the device, you can look through /sys to find it.
Taken

Apologize for the late post, we were evaluating your feedback. It looks
good, thanks!


--------
Atul Mukker
Architect, Drivers and BIOS
LSI Logic Corporation

RE: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Thu, 2005-01-06 at 09:20 -0500, Mukker, Atul wrote:
> For this to happen, the applications must at lease know 'n' in host<n>,
> otherwise it will have to trigger a rescan on all controllers. Which would
> be an overkill. How about publishing the adapter class attribute as well?
> This would allow applications to correlate the adapter handle with the class
> attribute.

I don't see why not ... it's your driver, you can publish whatever extra
information you need as scsi_device attributes; that was one of the
designs of the extensible attribute system.

> Apologize for the late post, we were evaluating your feedback. It looks
> good, thanks!

Great ... so the problem is now solved and just needs to be implemented.

James

RE: How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

>
>On Mon, 2005-01-03 at 18:02 -0500, Bagalkote, Sreenivas wrote:
>> o    Everybody understands that as long as the SCSI 
>scan/rescan is triggered
>> by 
>> the management app, there is no getting around knowing HCTL 
>mapping. The app
>> must know the HCTL quad of a logical drive.
>
>Actually, if that's all you're trying to do, what about
>
>echo '- - -' > /sys/class/scsi_host/host<n>/scan
>
>That'll trigger a rescan of the entire card and the device 
>will be found
>and added?
>

A minor point is that an application should ideally force scan only those
drives
that it has added. But more importantly, this will not help an application
to delete
the drives. Correct?

Sreenivas

RE: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Tue, 2005-01-04 at 12:25 -0500, Bagalkote, Sreenivas wrote:
> A minor point is that an application should ideally force scan only those
> drives

Yes, it's a bit overkill, but it would work ... it's not like
reconfiguration is something you do every day.

> that it has added. But more importantly, this will not help an application
> to delete
> the drives. Correct?

Well, no, I think that would work under this scheme, too:  to delete a
drive, it must already exist, so if you've published the logical number
in sysfs, you can easily find it by logical ID and delete it by host
channel, pun and lun.

James

RE: How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

Hello All,

I am trying to stoke this thread once more. Please review the brief summary
of all discussion and a new sysfs based proposal.

o    The more ideal and long-term way to handle addition and removal of
devices is through hotplug mechanism. Currently it is not fully developed.
Moreover, the megaraid SCSI driver doesn't get any event notification from
the FW that the configuration has been changed.

o    Management App that communicates to FW via driver private ioctls is the
one that creates/modifies the configuration. This is transparent to the
driver.

o    Logical drives being "logical", the driver exports the logical drives
on virtual
SCSI addresses. The management app that knows about LD0, LD1 .. LDn, does
not know the LDn to HCTL mapping (Host:Channel:Target:Lun)

o    Everybody understands that as long as the SCSI scan/rescan is triggered
by 
the management app, there is no getting around knowing HCTL mapping. The app
must know the HCTL quad of a logical drive.

o    Our original solution was that the driver returns HCTL of a LD via
ioctl. This
has been squarely rejected because kernel cannot add any more private
ioctls.

Considering that the app has to somehow _know_ the HCTL given a logical
drive
(that it has deleted or will be adding), please provide your feedback on the
following
proposal. I know that it is not radically different.

The driver would create nodes for all possible logical drives.

/sys/modules
 |-- megaraid_mbox
 |   |-- LD0
 |   |-- LD1
 |   |-- LD2
 |   |-- LD(i)
 |   |-- LD40

One of the attributes of a LD node would be HCTL information. A management
app
can then scan, delete the LDs using corresponding HCTL address.

Y'all have a great new year!

Sreenivas
LSI Logic

RE: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Mon, 2005-01-03 at 18:02 -0500, Bagalkote, Sreenivas wrote:
> o    Everybody understands that as long as the SCSI scan/rescan is triggered
> by 
> the management app, there is no getting around knowing HCTL mapping. The app
> must know the HCTL quad of a logical drive.

Actually, if that's all you're trying to do, what about

echo '- - -' > /sys/class/scsi_host/host<n>/scan

That'll trigger a rescan of the entire card and the device will be found
and added?

Then, if you simply publish your LD number as an extra parameter of the
device, you can look through /sys to find it.

James

RE: How to add/drop SCSI drives from within the driver?

[Salyzyn, Mark]

Alan Cox writes:
 
>On Iau, 2004-12-16 at 09:54, Arjan van de Ven wrote:
>> I'm strongly against adding this. The reason for that is that once an
>> ioctl is added, it realistically will and can never go away.
>> LSI is free to have their own fork and give that to dell; but they
>> should and could have known that it wasn't going to fly. (same I
guess
>> for adaptec ioctls). The companies who then commit to some schedule
>> realize they take a huge risk, but that is no reason to foul up the
>> kernel more. 
>
>I agree. I'd like to see an agreed standard interface for dropping and
>managing physical volumes and drives, as well as a standard interface
>for dropping/managing logical volumes.

Simple transactions, perhaps, but there are many checks, balances and
complexities associated with a full management application that do not
make sense to reside in the kernel.

Aacraid uses a FIB to communicate a wide variety of RAID management
commands.
Dpt_i2o uses an I2O private frame to communicate the RAID management
commands.

The remaining ioctls pick up driver or OS internal information as has
been discussed thus far. Moving these pieces of information to sysfs
does make some sense.

However, in order to add the ability to manage the arrays, especially if
they follow the complexity of the container arrangement, hotspare
assignments, the multitude of array types and their configuration needs
and various other RAID or Cache policies; one will find the code sucked
into the driver to be a lead weight. Adding a `translation' layer from
FIB, I2O and other frames to a common set is *not* going to be
lightweight and will not be full coverage. The recent efforts by HP to
push CSMI added 30% to the size of the driver (Adaptec Branched
version). Yuck (this is a scientific term) and only added `monitoring'
and 'drive passthrough' capabilities.

Array status monitoring is complicated enough to pull out into an
application (the recent dropping of the firmware print messages and
replacement with the aeventd application).

By all means, let's set a standard for rudimentary array manipulation,
but let's not loose sight of the complicated needs of the consumers of
RAID equipment. Sysfs does not look like it will scale well towards a
full up management interface.

Sincerely -- Mark Salyzyn

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

> On Wed, 2004-12-15 at 15:30 -0600, Matt Domsch wrote:
> > Do you plan to apply LSI's driver patch which adds the 
> driver-private 
> > ioctl to provide the mapping from logical drive address to 
> HCTL value?
> > Both Dell and LSI have products which are lined up to use this new 
> > ioctl because it's the most expedient thing to do, 
> maintains internal 
> > project schedules, etc, which delaying until this sysfs 
> mechanism hits 
> > will greatly impact those schedules. (I know, many folks on 
> this list 
> > don't care about business-side impacts of choices made on-list.)
> 
> I'm strongly against adding this. The reason for that is that 
> once an ioctl is added, it realistically will and can never go away.

This issue has gone from the utility of the ioctl in question (which has
been discussed in detail on the list) to now outright rejection of the
driver ioctls. If this is the idea, drivers would need to consciously move
the driver only ioctls to sysfs. Which may be good for simple ioctls like
these but might now be elegant for complex transfers. So, IMO, drivers need
both of the interfaces and should make a good judgment call on services
provided by each interface.

>
> that is no reason to foul up the kernel more. 
> 
The ioctl in question has been discussed in detail and was deemed
appropriate ioctl to provide important service to the management
applications for device hot plugging. Schedules are one side of the story,
but we must not overlook the importance of the reason to have this ioctl in
the first place. If there are other preferred ways to implement the similar
functionality, we would be more than happy to consider that.

Thanks
-Atul Mukker

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

> 
> Your management apps currently issue a private ioctl 
> MEGAIOC_QNADAP which returns the number of 
> megaraid_mm-handled adapters in the system.  How do you map a 
> megaraid adapter number to a struct Scsi_Host device, to be 
> sure you're acting on the controller you think you are?
Megaraid_mm module maintains all the controllers on a list
(mraid_mm_get_adapter), and each of the adapter maintains a pointer to the
host object.

> 
> SCSI LLDDs don't show up in sysfs under /sys/bus/scsi/drivers 
> at present, which is where, I think, you would want to put 
> megaraid_mm with links to show the scsi_host and pci_dev 
> associated with this adapter.  Something like this:
> 
> /sys
> |-- bus
> |   `-- drivers
> |       `-- scsi
> |           `-- megaraid_mm
> |               `-- adapter0
> |                   |-- pci_dev -> 
> ../../../../../devices/pci0000:03/0000:03:06.0
> |                   `-- scsi_host -> 
> |../../../../../class/scsi/scsi_host/host0
> |-- class
> |   `-- scsi
> |       `-- scsi_host
> |           `-- host0
> `-- devices
>     `-- pci0000:03
>         `-- 0000:03:06.0
Megaraid_mm is not a 'scsi' driver but only a conduit to pass the commands
from application to the megaraid_mbox scsi module. 

Re: How to add/drop SCSI drives from within the driver?

[Matt Domsch]

On Wed, Dec 15, 2004 at 02:42:21PM -0500, Mukker, Atul wrote:
> > 
> > Your management apps currently issue a private ioctl 
> > MEGAIOC_QNADAP which returns the number of 
> > megaraid_mm-handled adapters in the system.  How do you map a 
> > megaraid adapter number to a struct Scsi_Host device, to be 
> > sure you're acting on the controller you think you are?
>
> Megaraid_mm module maintains all the controllers on a list
> (mraid_mm_get_adapter), and each of the adapter maintains a pointer
> to the host object.

Yes, of course, I was unclear.  How do your userspace library and
management tools know that when they send commands via the
/dev/megaraid ioctls for adapter #5, that it's actually going to talk
to the PCI device at 0:03:06.0 (presumably the one it wants to do
configuration on) and not another megaraid-driven PCI device at
0:06:06.0 (which is not the one it wants) ?

If your management application doesn't care, or solves this in another
manner, please advise.

> Megaraid_mm is not a 'scsi' driver but only a conduit to pass the commands
> from application to the megaraid_mbox scsi module. 

But from your userspace tools perspective, it doesn't know that.  All
it knows is that (right now) it opens /dev/megaraid and issues ioctls
to a certain adapter number, yes?

What I'm trying to get to is a mapping, visible to userspace apps,
something like this.  You're right, megaraid_mm isn't a scsi driver,
so perhaps rather than having adapter0 with parent tree of
/sys/bus/scsi/drivers/megaraid_mm, it should be exposed with parent
tree /sys/module/megaraid_mm/, like so:

/sys
|-- class
|   `-- scsi_host
|       `-- host0
|-- devices
|   `-- pci0000:03
|       `-- 0000:03:06.0
`-- module
    |-- megaraid_mbox
    `-- megaraid_mm
        `-- adapter0
            |-- host0 -> ../../../class/scsi_host/host0
            `-- pci_dev -> ../../../devices/pci0000:03/0000:03:06.0

Your struct adapter_t has this mapping already, kernel-internal.  But
I think your userspace tools need to know the mapping too, to make
sure that adapter0 is the device they really intend to send management
commands to.

I think this question (do I have the right adapter), is analogous to
the question before (do I have the right HCTL tuple for this logical
drive), which my patch of last night may help address.

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

RE: How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

>> +} __attribute__ ((packed));
>
>I believe, per the discussion, that this was unnecessary and 
>possibly even incorrect.  Please repost one more time with 
>this removed.
>

Yes, thank you. I will do that. The patch that I submitted earlier
had an one more critical fix. I will separate them into two patches
and submit them as PATCH 1/2 & 2/2 as well.

Thanks,
Sreenivas
LSI Logic

RE: How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

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

All,

I am submitting the final patch that incorporates Matt's suggestions.

Thank you,
Sreenivas


diff -Naur old-rc2/Documentation/scsi/ChangeLog.megaraid
new-rc2/Documentation/scsi/ChangeLog.megaraid
--- old-rc2/Documentation/scsi/ChangeLog.megaraid	2004-12-07
16:40:23.000000000 -0500
+++ new-rc2/Documentation/scsi/ChangeLog.megaraid	2004-12-09
17:38:58.000000000 -0500
@@ -1,3 +1,22 @@
+Release Date	: Thu Dec  9 17:25:27 EST 2004 - Sreenivas Bagalkote
<sreenib@lsil.com>
+
+Current Version	: 2.20.4.2 (scsi module), 2.20.2.3 (cmm module)
+Older Version	: 2.20.4.1 (scsi module), 2.20.2.2 (cmm module)
+
+i.	Fixed a bug in deallocating kioc's dma buffer in megaraid_mm module
+ii.	Introduced driver ioctl that returns scsi address for a given ld.
+	
+	"Why can't the existing sysfs interfaces be used to do this?"
+		- Brian King (brking@us.ibm.com)
+	
+	"I've looked into solving this another way, but I cannot see how
+	to get this driver-private mapping of logical drive number-> HCTL
+	without putting code something like this into the driver."
+
+	"...and by providing a mapping a function to userspace, the driver
+	is free to change its mapping algorithm in the future if necessary
.."
+		- Matt Domsch (Matt_Domsch@dell.com)
+
 Release Date	: Thu Nov  4 18:24:56 EST 2004 - Sreenivas Bagalkote
<sreenib@lsil.com>
 
 Current Version	: 2.20.4.1 (scsi module), 2.20.2.2 (cmm module)
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
--- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-07
16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-09
17:05:18.000000000 -0500
@@ -51,6 +51,7 @@
 #define MEGAIOC_QNADAP		'm'	/* Query # of adapters		*/
 #define MEGAIOC_QDRVRVER	'e'	/* Query driver version		*/
 #define MEGAIOC_QADAPINFO   	'g'	/* Query adapter information	*/
+#define MEGAIOC_GET_SADDR	'a'	/* Get SCSI address of an LD	*/
 
 #define USCSICMD		0x80
 #define UIOC_RD			0x00001
@@ -62,6 +63,7 @@
 #define GET_ADAP_INFO		0x30000
 #define GET_CAP			0x40000
 #define GET_STATS		0x50000
+#define GET_LD_SADDR		0x60000
 #define GET_IOCTL_VERSION	0x01
 
 #define EXT_IOCTL_SIGN_SZ	16
@@ -142,7 +144,7 @@
 
 	caddr_t			buf_vaddr;
 	dma_addr_t		buf_paddr;
-	uint8_t			pool_index;
+	int8_t			pool_index;
 	uint8_t			free_buf;
 
 	uint8_t			timedout;
@@ -217,6 +219,27 @@
 
 } __attribute__ ((packed)) mcontroller_t;
 
+/**
+ * ld_device_address	: SCSI device address for the logical drives
+ * 
+ * @host_no		: host# to which LD belogs; as understood by
mid-layer
+ * @channel		: channel on which LD is exported
+ * @target		: target on which the LD is exported
+ * @lun			: lun on which the LD is exported
+ * @ld			: the LD for which this information is sought
+ * @reserved		: reserved fields; must be set to zero
+ *
+ * NOTE			: Applications set the LD number and expect
the 
+ * 			  SCSI address to be returned in the first four
fields
+ */
+struct ld_device_address {
+	uint32_t	host_no;
+	uint32_t	channel;
+	uint32_t	scsi_id;
+	uint32_t	lun;
+	uint32_t	ld;
+	uint32_t	reserved[3];
+} __attribute__ ((packed));
 
 /**
  * mm_dmapool_t	: Represents one dma pool with just one buffer
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.c
new-rc2/drivers/scsi/megaraid/megaraid_mbox.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-07
16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-09
17:42:33.538473752 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mbox.c
- * Version	: v2.20.4.1 (Nov 04 2004)
+ * Version	: v2.20.4.2 (Dec  9 2004)
  *
  * Authors:
  * 	Atul Mukker		<Atul.Mukker@lsil.com>
@@ -3642,6 +3642,7 @@
 megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, uint32_t
action)
 {
 	adapter_t *adapter;
+	struct ld_device_address* lda;
 
 	if (action != IOCTL_ISSUE) {
 		con_log(CL_ANN, (KERN_WARNING
@@ -3670,6 +3671,30 @@
 
 		return kioc->status;
 
+	case GET_LD_SADDR:
+		
+		lda = (struct ld_device_address*)(unsigned
long)kioc->buf_vaddr;
+
+		if (lda->ld > MAX_LOGICAL_DRIVES_40LD) {
+			lda->host_no	= 0xffffffff;
+			lda->channel	= 0xffffffff;
+			lda->scsi_id	= 0xffffffff;
+			lda->lun	= 0xffffffff;
+			kioc->status	= -ENODEV;
+			kioc->done(kioc);
+			return kioc->status;
+		}
+
+		lda->host_no	= adapter->host->host_no;
+		lda->channel	= adapter->max_channel;
+		lda->scsi_id	= (lda->ld < adapter->init_id) ? 
+						lda->ld : lda->ld + 1;
+		lda->lun	= 0;
+		kioc->status	= 0;
+		kioc->done(kioc);
+
+		return kioc->status;
+
 	case MBOX_CMD:
 
 		return megaraid_mbox_mm_command(adapter, kioc);
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.h
new-rc2/drivers/scsi/megaraid/megaraid_mbox.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-07
16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-09
17:41:34.706417584 -0500
@@ -21,8 +21,8 @@
 #include "megaraid_ioctl.h"
 
 
-#define MEGARAID_VERSION	"2.20.4.1"
-#define MEGARAID_EXT_VERSION	"(Release Date: Thu Nov  4 17:44:59 EST
2004)"
+#define MEGARAID_VERSION	"2.20.4.2"
+#define MEGARAID_EXT_VERSION	"(Release Date: Thu Dec  9 17:25:27 EST
2004)"
 
 
 /*
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.c
new-rc2/drivers/scsi/megaraid/megaraid_mm.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-07
16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-09
18:12:16.485424616 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mm.c
- * Version	: v2.20.2.2 (Nov 04 2004)
+ * Version	: v2.20.2.3 (Dec  9 2004)
  *
  * Common management module
  */
@@ -373,6 +373,20 @@
 			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
 				return (-ENOMEM);
 		}
+		else if (subopcode == MEGAIOC_GET_SADDR) {
+
+			kioc->opcode	= GET_LD_SADDR;
+			kioc->data_dir	= UIOC_RD;
+			kioc->xferlen	= sizeof(struct ld_device_address);
+
+			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
+				return (-ENOMEM);
+
+			if (copy_from_user(kioc->buf_vaddr, mimd.data,
+							kioc->xferlen )) {
+				return (-EFAULT);
+			}
+		}
 		else {
 			con_log(CL_ANN, (KERN_WARNING
 					"megaraid cmm: Invalid subop\n"));
@@ -614,23 +628,27 @@
 	mm_dmapool_t	*pool;
 	unsigned long	flags;
 
-	pool = &adp->dma_pool_list[kioc->pool_index];
+	if (kioc->pool_index != -1) {
+		pool = &adp->dma_pool_list[kioc->pool_index];
 
-	/* This routine may be called in non-isr context also */
-	spin_lock_irqsave(&pool->lock, flags);
+		/* This routine may be called in non-isr context also */
+		spin_lock_irqsave(&pool->lock, flags);
 
-	/*
-	 * While attaching the dma buffer, if we didn't get the required
-	 * buffer from the pool, we would have allocated it at the run time
-	 * and set the free_buf flag. We must free that buffer. Otherwise,
-	 * just mark that the buffer is not in use
-	 */
-	if (kioc->free_buf == 1)
-		pci_pool_free(pool->handle, kioc->buf_vaddr,
kioc->buf_paddr);
-	else
-		pool->in_use = 0;
+		/*
+		 * While attaching the dma buffer, if we didn't get the 
+		 * required buffer from the pool, we would have allocated 
+		 * it at the run time and set the free_buf flag. We must 
+		 * free that buffer. Otherwise, just mark that the buffer is

+		 * not in use
+		 */
+		if (kioc->free_buf == 1)
+			pci_pool_free(pool->handle, kioc->buf_vaddr, 
+							kioc->buf_paddr);
+		else
+			pool->in_use = 0;
 
-	spin_unlock_irqrestore(&pool->lock, flags);
+		spin_unlock_irqrestore(&pool->lock, flags);
+	}
 
 	/* Return the kioc to the free pool */
 	spin_lock_irqsave(&adp->kioc_pool_lock, flags);
@@ -809,6 +827,15 @@
 
 			return 0;
 
+		case MEGAIOC_GET_SADDR:
+
+			if (copy_to_user(kmimd.data, kioc->buf_vaddr,
+					sizeof(struct ld_device_address))) {
+				return (-EFAULT);
+			}
+			
+			return kioc->status;
+
 		default:
 			return (-EINVAL);
 		}
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.h
new-rc2/drivers/scsi/megaraid/megaraid_mm.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-07
16:40:15.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-09
18:12:10.336359416 -0500
@@ -29,9 +29,9 @@
 #include "megaraid_ioctl.h"
 
 
-#define LSI_COMMON_MOD_VERSION	"2.20.2.2"
+#define LSI_COMMON_MOD_VERSION	"2.20.2.3"
 #define LSI_COMMON_MOD_EXT_VERSION	\
-		"(Release Date: Thu Nov  4 17:46:29 EST 2004)"
+		"(Release Date: Thu Dec  9 17:25:27 EST 2004)"
 
 
 #define LSI_DBGLVL			dbglevel



[-- Attachment #2: megaraid-hctl.patch --]
[-- Type: application/octet-stream, Size: 8638 bytes --]

diff -Naur old-rc2/Documentation/scsi/ChangeLog.megaraid new-rc2/Documentation/scsi/ChangeLog.megaraid
--- old-rc2/Documentation/scsi/ChangeLog.megaraid	2004-12-07 16:40:23.000000000 -0500
+++ new-rc2/Documentation/scsi/ChangeLog.megaraid	2004-12-09 17:38:58.000000000 -0500
@@ -1,3 +1,22 @@
+Release Date	: Thu Dec  9 17:25:27 EST 2004 - Sreenivas Bagalkote <sreenib@lsil.com>
+
+Current Version	: 2.20.4.2 (scsi module), 2.20.2.3 (cmm module)
+Older Version	: 2.20.4.1 (scsi module), 2.20.2.2 (cmm module)
+
+i.	Fixed a bug in deallocating kioc's dma buffer in megaraid_mm module
+ii.	Introduced driver ioctl that returns scsi address for a given ld.
+	
+	"Why can't the existing sysfs interfaces be used to do this?"
+		- Brian King (brking@us.ibm.com)
+	
+	"I've looked into solving this another way, but I cannot see how
+	to get this driver-private mapping of logical drive number-> HCTL
+	without putting code something like this into the driver."
+
+	"...and by providing a mapping a function to userspace, the driver
+	is free to change its mapping algorithm in the future if necessary .."
+		- Matt Domsch (Matt_Domsch@dell.com)
+
 Release Date	: Thu Nov  4 18:24:56 EST 2004 - Sreenivas Bagalkote <sreenib@lsil.com>
 
 Current Version	: 2.20.4.1 (scsi module), 2.20.2.2 (cmm module)
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
--- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-07 16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-09 17:05:18.000000000 -0500
@@ -51,6 +51,7 @@
 #define MEGAIOC_QNADAP		'm'	/* Query # of adapters		*/
 #define MEGAIOC_QDRVRVER	'e'	/* Query driver version		*/
 #define MEGAIOC_QADAPINFO   	'g'	/* Query adapter information	*/
+#define MEGAIOC_GET_SADDR	'a'	/* Get SCSI address of an LD	*/
 
 #define USCSICMD		0x80
 #define UIOC_RD			0x00001
@@ -62,6 +63,7 @@
 #define GET_ADAP_INFO		0x30000
 #define GET_CAP			0x40000
 #define GET_STATS		0x50000
+#define GET_LD_SADDR		0x60000
 #define GET_IOCTL_VERSION	0x01
 
 #define EXT_IOCTL_SIGN_SZ	16
@@ -142,7 +144,7 @@
 
 	caddr_t			buf_vaddr;
 	dma_addr_t		buf_paddr;
-	uint8_t			pool_index;
+	int8_t			pool_index;
 	uint8_t			free_buf;
 
 	uint8_t			timedout;
@@ -217,6 +219,27 @@
 
 } __attribute__ ((packed)) mcontroller_t;
 
+/**
+ * ld_device_address	: SCSI device address for the logical drives
+ * 
+ * @host_no		: host# to which LD belogs; as understood by mid-layer
+ * @channel		: channel on which LD is exported
+ * @target		: target on which the LD is exported
+ * @lun			: lun on which the LD is exported
+ * @ld			: the LD for which this information is sought
+ * @reserved		: reserved fields; must be set to zero
+ *
+ * NOTE			: Applications set the LD number and expect the 
+ * 			  SCSI address to be returned in the first four fields
+ */
+struct ld_device_address {
+	uint32_t	host_no;
+	uint32_t	channel;
+	uint32_t	scsi_id;
+	uint32_t	lun;
+	uint32_t	ld;
+	uint32_t	reserved[3];
+} __attribute__ ((packed));
 
 /**
  * mm_dmapool_t	: Represents one dma pool with just one buffer
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.c new-rc2/drivers/scsi/megaraid/megaraid_mbox.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-07 16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-09 17:42:33.538473752 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mbox.c
- * Version	: v2.20.4.1 (Nov 04 2004)
+ * Version	: v2.20.4.2 (Dec  9 2004)
  *
  * Authors:
  * 	Atul Mukker		<Atul.Mukker@lsil.com>
@@ -3642,6 +3642,7 @@
 megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, uint32_t action)
 {
 	adapter_t *adapter;
+	struct ld_device_address* lda;
 
 	if (action != IOCTL_ISSUE) {
 		con_log(CL_ANN, (KERN_WARNING
@@ -3670,6 +3671,30 @@
 
 		return kioc->status;
 
+	case GET_LD_SADDR:
+		
+		lda = (struct ld_device_address*)(unsigned long)kioc->buf_vaddr;
+
+		if (lda->ld > MAX_LOGICAL_DRIVES_40LD) {
+			lda->host_no	= 0xffffffff;
+			lda->channel	= 0xffffffff;
+			lda->scsi_id	= 0xffffffff;
+			lda->lun	= 0xffffffff;
+			kioc->status	= -ENODEV;
+			kioc->done(kioc);
+			return kioc->status;
+		}
+
+		lda->host_no	= adapter->host->host_no;
+		lda->channel	= adapter->max_channel;
+		lda->scsi_id	= (lda->ld < adapter->init_id) ? 
+						lda->ld : lda->ld + 1;
+		lda->lun	= 0;
+		kioc->status	= 0;
+		kioc->done(kioc);
+
+		return kioc->status;
+
 	case MBOX_CMD:
 
 		return megaraid_mbox_mm_command(adapter, kioc);
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.h new-rc2/drivers/scsi/megaraid/megaraid_mbox.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-07 16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-09 17:41:34.706417584 -0500
@@ -21,8 +21,8 @@
 #include "megaraid_ioctl.h"
 
 
-#define MEGARAID_VERSION	"2.20.4.1"
-#define MEGARAID_EXT_VERSION	"(Release Date: Thu Nov  4 17:44:59 EST 2004)"
+#define MEGARAID_VERSION	"2.20.4.2"
+#define MEGARAID_EXT_VERSION	"(Release Date: Thu Dec  9 17:25:27 EST 2004)"
 
 
 /*
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.c new-rc2/drivers/scsi/megaraid/megaraid_mm.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-07 16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-09 18:12:16.485424616 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mm.c
- * Version	: v2.20.2.2 (Nov 04 2004)
+ * Version	: v2.20.2.3 (Dec  9 2004)
  *
  * Common management module
  */
@@ -373,6 +373,20 @@
 			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
 				return (-ENOMEM);
 		}
+		else if (subopcode == MEGAIOC_GET_SADDR) {
+
+			kioc->opcode	= GET_LD_SADDR;
+			kioc->data_dir	= UIOC_RD;
+			kioc->xferlen	= sizeof(struct ld_device_address);
+
+			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
+				return (-ENOMEM);
+
+			if (copy_from_user(kioc->buf_vaddr, mimd.data,
+							kioc->xferlen )) {
+				return (-EFAULT);
+			}
+		}
 		else {
 			con_log(CL_ANN, (KERN_WARNING
 					"megaraid cmm: Invalid subop\n"));
@@ -614,23 +628,27 @@
 	mm_dmapool_t	*pool;
 	unsigned long	flags;
 
-	pool = &adp->dma_pool_list[kioc->pool_index];
+	if (kioc->pool_index != -1) {
+		pool = &adp->dma_pool_list[kioc->pool_index];
 
-	/* This routine may be called in non-isr context also */
-	spin_lock_irqsave(&pool->lock, flags);
+		/* This routine may be called in non-isr context also */
+		spin_lock_irqsave(&pool->lock, flags);
 
-	/*
-	 * While attaching the dma buffer, if we didn't get the required
-	 * buffer from the pool, we would have allocated it at the run time
-	 * and set the free_buf flag. We must free that buffer. Otherwise,
-	 * just mark that the buffer is not in use
-	 */
-	if (kioc->free_buf == 1)
-		pci_pool_free(pool->handle, kioc->buf_vaddr, kioc->buf_paddr);
-	else
-		pool->in_use = 0;
+		/*
+		 * While attaching the dma buffer, if we didn't get the 
+		 * required buffer from the pool, we would have allocated 
+		 * it at the run time and set the free_buf flag. We must 
+		 * free that buffer. Otherwise, just mark that the buffer is 
+		 * not in use
+		 */
+		if (kioc->free_buf == 1)
+			pci_pool_free(pool->handle, kioc->buf_vaddr, 
+							kioc->buf_paddr);
+		else
+			pool->in_use = 0;
 
-	spin_unlock_irqrestore(&pool->lock, flags);
+		spin_unlock_irqrestore(&pool->lock, flags);
+	}
 
 	/* Return the kioc to the free pool */
 	spin_lock_irqsave(&adp->kioc_pool_lock, flags);
@@ -809,6 +827,15 @@
 
 			return 0;
 
+		case MEGAIOC_GET_SADDR:
+
+			if (copy_to_user(kmimd.data, kioc->buf_vaddr,
+					sizeof(struct ld_device_address))) {
+				return (-EFAULT);
+			}
+			
+			return kioc->status;
+
 		default:
 			return (-EINVAL);
 		}
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.h new-rc2/drivers/scsi/megaraid/megaraid_mm.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-07 16:40:15.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-09 18:12:10.336359416 -0500
@@ -29,9 +29,9 @@
 #include "megaraid_ioctl.h"
 
 
-#define LSI_COMMON_MOD_VERSION	"2.20.2.2"
+#define LSI_COMMON_MOD_VERSION	"2.20.2.3"
 #define LSI_COMMON_MOD_EXT_VERSION	\
-		"(Release Date: Thu Nov  4 17:46:29 EST 2004)"
+		"(Release Date: Thu Dec  9 17:25:27 EST 2004)"
 
 
 #define LSI_DBGLVL			dbglevel

Re: How to add/drop SCSI drives from within the driver?

[Matt Domsch]

On Thu, Dec 09, 2004 at 06:37:19PM -0500, Bagalkote, Sreenivas wrote:
> I am submitting the final patch that incorporates Matt's suggestions.
> +} __attribute__ ((packed));

I believe, per the discussion, that this was unnecessary and possibly
even incorrect.  Please repost one more time with this removed.

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

 
> > 8. So, all driver has to do to assist applications is to 
> provide the 
> > logical drive number to scsi address mapping. Application 
> would say, 
> > hey! I added/removed logical drive number 5, driver 
> reverts, here is 
> > the scsi address for it "host:2, channel:5, target:5 lun:0" :-)
> 
> How does the driver supply the mapping if it does not know 
> about the "change"? Is it reserving or telling user space the 
> hctl that will be used?
This point was thoroughly explained in my previous post item 2. Yes, driver
reserves a virtual bus on which all logical drives would be exported.

> 
> If so, why can't the driver call scsi_add_device(h,c,t,l) 
> after the ioctl to create the logical drive completes?
This idea was already turned down, see mails on linux-scsi on 12/3

> 
> Though I am all for using hotplug to initiate scanning from 
> user space.
good

Re: How to add/drop SCSI drives from within the driver?

['Patrick Mansfield']

On Thu, Dec 09, 2004 at 12:31:37PM -0500, Mukker, Atul wrote:

> > How does the driver supply the mapping if it does not know 
> > about the "change"? Is it reserving or telling user space the 
> > hctl that will be used?
> This point was thoroughly explained in my previous post item 2. Yes, driver
> reserves a virtual bus on which all logical drives would be exported.

OK ... 

> > If so, why can't the driver call scsi_add_device(h,c,t,l) 
> > after the ioctl to create the logical drive completes?
> This idea was already turned down, see mails on linux-scsi on 12/3

Well you can call scsi_add_device() from the driver, the objections I saw
were to add an ioctl that would just call scsi_add_device().

But it doesn't matter if adding driver post-ioctl specific actions is not
an option.

-- Patrick Mansfield

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

 
> > >The real way I'd like to handle this is via hotplug.  The hotplug 
> > >event would transmit the HCTL in the environment.
> > >Whether the drive actually gets incorporated into the system and 
> > >where is user policy, so it's appropriate that it should be in 
> > >userland.
> > 
> > James, it is the application that is adding the drive. So 
> it is not a 
> > hotplug event for the driver.
> 
> Then perhaps I don't understand what the issue is.  If the 
> application is adding the drive, then surely it would know 
> the numbers.  If not, then the driver must communicate this 
> back, and that's what the hotplug would be about.
> 
> James
> 
> 
Let me elaborate it.

1. The management applications decide to created new logical drives or
remove some of the existing ones.

2. How applications would do this, is by sending commands to the megaraid
firmware via driver. Driver does not intercept these commands for a simple
reason that the driver code would become unnecessarily bulky to understanad
all the possible ways applications can change the number of logical devices.

3. Once the change in configuration has happened, someone must notify kernel
about new or removed devices.

4. Since megaraid driver does not know about these changes, it cannot notify
kernel.

5. So this becomes the responsibility of the application which caused the
change in configuration. Application dilemma is, all it know is it created a
few devices and removed some. But there is no way for it to relate the
affected devices with the way how kernel was or would be seeing them, that
is, the affected device's scsi address. Remember, the affected devices are
only logical, there is no physical bus, target, lun associated with them.
Driver creates this mapping on the fly.

6. That's where application seeks help from driver and requests for the scsi
address driver would be using for the affected devices.

7. Once it has the scsi address for the devices in question, depending on
the application and system administrator's preference, application would
either use sysfs to add/remove affected devices or cause a hotplug event
resulting in the configured behavior.

8. So, all driver has to do to assist applications is to provide the logical
drive number to scsi address mapping. Application would say, hey! I
added/removed logical drive number 5, driver reverts, here is the scsi
address for it "host:2, channel:5, target:5 lun:0" :-)

-Atul Mukker

Re: How to add/drop SCSI drives from within the driver?

[Patrick Mansfield]

On Thu, Dec 09, 2004 at 09:11:15AM -0500, Mukker, Atul wrote:

> 4. Since megaraid driver does not know about these changes, it cannot notify
> kernel.

> 8. So, all driver has to do to assist applications is to provide the logical
> drive number to scsi address mapping. Application would say, hey! I
> added/removed logical drive number 5, driver reverts, here is the scsi
> address for it "host:2, channel:5, target:5 lun:0" :-)

How does the driver supply the mapping if it does not know about the
"change"? Is it reserving or telling user space the hctl that will be
used?

If so, why can't the driver call scsi_add_device(h,c,t,l) after the ioctl
to create the logical drive completes?

Though I am all for using hotplug to initiate scanning from user space.

-- Patrick Mansfield

RE: How to add/drop SCSI drives from within the driver?

[Salyzyn, Mark]

Bryan Henderson writes:
>>can only be good because the applications and
>>driver could be compiled on different archs.
>
>I'm having a hard time imagining where the application and the kernel 
>could reasonably have been compiled for different architectures.
They're 
>running on the same machine, aren't they?

We have management applications that run remote. But we still
standardize on byte ordering as well.

Sincerely -- Mark Salyzyn

RE: How to add/drop SCSI drives from within the driver?

[Arjan van de Ven]

On Thu, 2004-12-09 at 08:27 -0500, Salyzyn, Mark wrote:
> Bryan Henderson writes:
> >>can only be good because the applications and
> >>driver could be compiled on different archs.
> >
> >I'm having a hard time imagining where the application and the kernel 
> >could reasonably have been compiled for different architectures.
> They're 
> >running on the same machine, aren't they?
> 
> We have management applications that run remote. But we still
> standardize on byte ordering as well.

the packed in a userspace visible struct is a mistake.
Packing for a hw struct is basically ok-ish, but please don't expose the hw struct to userspace,
instead give userspace a sane, type safe and 32/64 bit translation safe interface and convert internally.
Pretty please?

RE: How to add/drop SCSI drives from within the driver?

[Salyzyn, Mark]

>On Wed, 2004-12-08 at 15:08 -0500, Bryan Henderson wrote:
>>>> I would also suggest __attribute__((packed)) since you're sharing
with
>>>That's a bad idea, it forces the compiler to pessimise accesses to
the
>>>struct on most hardware.
>> How?  We're assuming that it won't actually affect the layout of this

>> struct, so what does the compiler pessimize?
>it also makes the compiler assume the struct members aren't aligned to
>the natural alignment and thus it'll emit more expensive access paterns
>on architectures that care about such things.

Time to fix the compiler?

Years of writing in itty bitty living spaces resulting in application
specific coding patterns has convinced me of one fact; it is not worth
it.

We are talking about not using a feature of the compiler for kernel/user
space portability in a rarely used interface because of petty
optimization issues.

Sincerely -- Mark Salyzyn

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

> On Wed, 2004-12-08 at 15:08 -0500, Bryan Henderson wrote:
> > >> I would also suggest __attribute__((packed)) since 
> you're sharing 
> > >> with userspace just to ensure you can never get it grown by the 
> > >> compiler (it shouldn't be, but just to be safe).
> > >
> > >That's a bad idea, it forces the compiler to pessimise accesses to 
> > >the struct on most hardware.
> > 
> > How?  We're assuming that it won't actually affect the 
> layout of this 
> > struct, so what does the compiler pessimize?
> 
> it also makes the compiler assume the struct members aren't 
> aligned to the natural alignment and thus it'll emit more 
> expensive access paterns on architectures that care about such things.

My basics tell me that compiler would align the structure using the
alignment restriction of the biggest member, so this structure would still
be same irrespective of __attribute__((packed)) and result in same
instructions. May be wrong. But the point is, it will be an assumption so
specifying proper attributes can only be good because the applications and
driver could be compiled on different archs.

How much overhead can there be, it's just some data sent by the driver to
application via ioctl?

Thanks
-Atul

RE: How to add/drop SCSI drives from within the driver?

[Bryan Henderson]

>My basics tell me that compiler would align the structure using the
>alignment restriction of the biggest member,

The compiler doesn't align the structure in the common case that it is 
dynamically allocated.

Apparently, the compiler assumes, with a naked struct declaration, that 
the user has done this, though.  With __attribute__((packed)), it does 
not.  And that seems reasonable to me.  These two properties of a struct 
(not allocated aligned and no alignment padding within) tend to go 
together.

>can only be good because the applications and
>driver could be compiled on different archs.

I'm having a hard time imagining where the application and the kernel 
could reasonably have been compiled for different architectures.  They're 
running on the same machine, aren't they?

>How much overhead can there be, it's just some data sent by the driver to
>application via ioctl?

This is the relevant point.  With the extra cycles being unnoticeable in 
this case, we should all but ignore that in favor of making code portable, 
readable, and hard for programmers to misunderstand.  It's not entirely 
clear that the packed structure is more portable, though.

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

> The real way I'd like to handle this is via hotplug.  The 
> hotplug event would transmit the HCTL in the environment.  
> Whether the drive actually gets incorporated into the system 
> and where is user policy, so it's appropriate that it should 
> be in userland.
Hmm, is it possible for applications to create hotplug events? If it is,
this could a good idea as matter of system policies. Regardless, our
management applications interfaces with the firmware in terms of logical
drive numbers. Driver converts this number to the scsi host, bus, target etc
during all interactions with kernel. In order for applications to actually
notify kernel about arrival or removal of logical drives (hotplug event or
sysfs), it has to know the corresponding mapping used by driver, while
registering or would be registering, the device in question.

Since the mapping is private to the driver, it only can tell to application,
for a give logical driver *number* the corresponding scsi address

> > Removing a drive:- There is no sane way for the application 
> to map out 
> > drives to /dev/sd<x>. If application has a way of knowing 
> the HCTL of 
> > a deleted drive, then using that HCTL, it can match the 
> corresponding 
> > SCSI device name (/dev/sd<x>) and use sysfs to remove that drive.
> 
> Since The sysfs device name contains H:C:T:L surely you can 
> just do a find on /sys?

Again, there is no way for applications to relate H:C:T:L with a logical
drive number. This information must come from the driver.

Thanks
-Atul Mukker

RE: How to add/drop SCSI drives from within the driver?

[Salyzyn, Mark]

James Bottomley writes:
> On Wed, 2004-12-08 at 01:16, Bagalkote, Sreenivas wrote:
>> Adding a drive:- For application to use sysfs to scan newly added
drive,
>> it needs to know the HCTL (SCSI address - Host, Channel, Target &
Lun)
>> of the drive. Driver is the only one that knows the mapping between a

>> drive and the corresponding HCTL.
>The real way I'd like to handle this is via hotplug.  The hotplug event
>would transmit the HCTL in the environment.  Whether the drive actually
>gets incorporated into the system and where is user policy, so it's
>appropriate that it should be in userland.

The problem is the aac based cards generate events (AIFs) that are
picked up by the driver. To go all the way to userland to interpret
these events and back to the driver is a waste and a source of failures.
Only the Firmware knows when an array zeroing has completed in order to
bring the device online.

>This same infrastructure could be used by fibre channel login, scsi
>enclosure events etc.

I would need to emulate an SES to propagate array changes?

>We have some of the hotplug infrastructure in SCSI, but not quite
enough
>for this ... you'll need an additional API.

What was wrong with scsi_scan_single_target (add), scsi_rescan_device
(remove/change)?

Sincerely -- Mark Salyzyn

RE: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Wed, 2004-12-08 at 13:42 -0500, Salyzyn, Mark wrote:
> James Bottomley writes:
> >The real way I'd like to handle this is via hotplug.  The hotplug event
> >would transmit the HCTL in the environment.  Whether the drive actually
> >gets incorporated into the system and where is user policy, so it's
> >appropriate that it should be in userland.
> 
> The problem is the aac based cards generate events (AIFs) that are
> picked up by the driver. To go all the way to userland to interpret
> these events and back to the driver is a waste and a source of failures.
> Only the Firmware knows when an array zeroing has completed in order to
> bring the device online.

Hotplug is the standard way of handling configuration changes (whether
requested or forced).  There's value to handling things in a standard
manner.  If the current implementation needs improving, then you're free
to do it (and it would benefit far more than just SCSI...).

So the firmware calls the SCSI API which triggers the hotplug event and
adds the device ... there's no problem.

> >This same infrastructure could be used by fibre channel login, scsi
> >enclosure events etc.
> 
> I would need to emulate an SES to propagate array changes?

No.  Merely that the hotplug API would also be usable by SES (and other
things).

James

Re: How to add/drop SCSI drives from within the driver?

[Matt Domsch]

On Wed, Dec 08, 2004 at 02:01:21PM -0600, James Bottomley wrote:
> On Wed, 2004-12-08 at 13:42 -0500, Salyzyn, Mark wrote:
> > James Bottomley writes:
> > >The real way I'd like to handle this is via hotplug.  The hotplug event
> > >would transmit the HCTL in the environment.  Whether the drive actually
> > >gets incorporated into the system and where is user policy, so it's
> > >appropriate that it should be in userland.
> 
> Hotplug is the standard way of handling configuration changes (whether
> requested or forced).  There's value to handling things in a standard
> manner.  If the current implementation needs improving, then you're free
> to do it (and it would benefit far more than just SCSI...).
> 
> So the firmware calls the SCSI API which triggers the hotplug event and
> adds the device ... there's no problem.

James, I've been thinking about this a little more, and you may be on
to something here. Let each driver add files as such:

/sys/class/scsi_host
 |-- host0
 |   |-- add_logical_drive
 |   |-- remove_logical_drive
 |   `-- rescan_logical_drive

Then we can go 2 ways with this.
1) driver functions directly call scsi_add_device(),
scsi_remove_device(), and something for rescan (option 2 handles this
one cleanly for us).  ATM, megaraid_mbox doesn't implement a rescan
function, so this point may be moot. 

2) driver functions call a midlayer library function, which invokes
   /sbin/hotplug with appropriate data, and add a new /etc/hotplug.d
   helper app which would then write to these files:

/sys/class/scsi_host
|-- host0
|   |-- scan
/sys/devices/pci0000:0x/0000:0x:0x.0/host0
|-- 0:0:0:0
|   |-- delete
|   |-- rescan

to do likewise.


Patch below against scsi-misc-2.6, as a proof of concept for option 1
above, option 2 would take a little more time to do and get right, but
I want to get feedback as to direction before digging further.

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

===== drivers/scsi/megaraid/megaraid_mbox.c 1.11 vs edited =====
--- 1.11/drivers/scsi/megaraid/megaraid_mbox.c	2004-11-18 16:12:10 -06:00
+++ edited/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-15 01:18:19 -06:00
@@ -455,6 +455,133 @@ static struct pci_driver megaraid_pci_dr
 
 
 /*
+ * sysfs class device support
+ * creates three files:
+ * /sys/class/scsi_host
+ * |-- host0
+ * |   |-- add_logical_drive
+ * |   |-- remove_logical_drive
+ * |   `-- rescan_logical_drive
+ *
+ * These make the midlayer invoke /sbin/hotplug, which then calls back into sysfs
+ * /sys/class/scsi_host
+ * |-- host0
+ * |   |-- scan
+ *
+ * and
+ *
+ * /sys/devices/pci0000:0x/0000:0x:0x.0/host0
+ * |-- 0:0:0:0
+ * |   |-- delete
+ * |   |-- rescan
+ *
+ * respectively.  This allows userspace applications to work
+ * using their logical drive number, and lets the driver translate
+ * that into host, channel, id, and lun values which the other
+ * mechanisms require.  This is similar to how hot plug CPU works.
+ */
+
+/**
+ * lda_to_hcil()
+ * @adapter
+ * @lda - logical drive address
+ * @host_no
+ * @channel
+ * @id
+ * @lun
+ *
+ * converts a logical drive address into a host, channel id, lun tuple.
+ */
+
+static inline int megaraid_lda_to_hcil(struct Scsi_Host *shost, u32 lda,
+				       u32 *host_no, u32 *channel, u32 *id, u32 *lun)
+{
+	if (lda > MAX_LOGICAL_DRIVES_40LD)
+		return -EINVAL;
+	*host_no = shost->host_no;
+	*channel = shost->max_channel;
+	*id      = (lda < shost->this_id) ? lda : lda + 1;
+	*lun     = 0;
+	return 0;
+}
+
+enum megaraid_host_action {
+	LD_ADD    = 1,
+	LD_REMOVE = 2,
+	LD_RESCAN = 3,
+};
+
+static int megaraid_host_store(struct class_device *class_dev, const char *buf, size_t count, int action)
+{
+        struct Scsi_Host *shost = class_to_shost(class_dev);
+	struct scsi_device *sdev;
+	int fields=0, rc=-EINVAL;
+	u32 lda=0, host_no=0, channel=0, id=0, lun=0;
+	fields = sscanf(buf, "%u", &lda);
+	if (fields != 1)
+		return rc;
+	if (lda > MAX_LOGICAL_DRIVES_40LD)
+		return rc;
+	rc = megaraid_lda_to_hcil(shost, lda, &host_no, &channel, &id, &lun);
+	if (rc)
+		return rc;
+
+	/*
+	 * This is where the call-out to /sbin/hotplug would go.
+	 * for now, short-circuit that and just call the necessary
+	 * functions directly.
+	 */
+	rc = -EINVAL;
+	switch (action) {
+	case LD_ADD:
+		scsi_add_device(shost, channel, id, lun);
+		break;
+	case LD_REMOVE:
+		sdev = scsi_device_lookup(shost, channel, id, lun);
+		if (sdev) {
+			scsi_remove_device(sdev);
+			scsi_device_put(sdev);
+			return -EINVAL;
+		}
+		break;
+	case LD_RESCAN:
+		return -ENOSYS;
+	default:
+		return -EINVAL;
+	}
+	return count;
+}
+
+static ssize_t megaraid_host_store_add_ld(struct class_device *class_dev, const char *buf, size_t count)
+{
+	return megaraid_host_store(class_dev, buf, count, LD_ADD);
+}
+static ssize_t megaraid_host_store_remove_ld(struct class_device *class_dev, const char *buf, size_t count)
+{
+	return megaraid_host_store(class_dev, buf, count, LD_REMOVE);
+}
+static ssize_t megaraid_host_store_rescan_ld(struct class_device *class_dev, const char *buf, size_t count)
+{
+	return megaraid_host_store(class_dev, buf, count, LD_RESCAN);
+}
+
+
+#define MEGARAID_HOST_WOATTR(_name,_store) \
+CLASS_DEVICE_ATTR(_name, S_IWUSR|S_IWGRP, NULL, _store)
+
+static MEGARAID_HOST_WOATTR(add_logical_drive, megaraid_host_store_add_ld);
+static MEGARAID_HOST_WOATTR(remove_logical_drive, megaraid_host_store_remove_ld);
+static MEGARAID_HOST_WOATTR(rescan_logical_drive, megaraid_host_store_rescan_ld);
+
+/* Host attributes initializer */
+static struct class_device_attribute *megaraid_host_attrs[] = {
+	&class_device_attr_add_logical_drive,
+	&class_device_attr_remove_logical_drive,
+	&class_device_attr_rescan_logical_drive,
+	NULL,
+};
+
+/*
  * Scsi host template for megaraid unified driver
  */
 static struct scsi_host_template megaraid_template_g = {
@@ -467,6 +594,7 @@ static struct scsi_host_template megarai
 	.eh_bus_reset_handler		= megaraid_reset_handler,
 	.eh_host_reset_handler		= megaraid_reset_handler,
 	.use_clustering			= ENABLE_CLUSTERING,
+	.shost_attrs			= megaraid_host_attrs,
 };
 
 

Re: How to add/drop SCSI drives from within the driver?

[Matt Domsch]

On Wed, Dec 15, 2004 at 01:24:53AM -0600, Matt Domsch wrote:
> /sys/class/scsi_host
>  |-- host0
>  |   |-- add_logical_drive
>  |   |-- remove_logical_drive
>  |   `-- rescan_logical_drive

Atul, Sreenivas:

Your management apps currently issue a private ioctl MEGAIOC_QNADAP
which returns the number of megaraid_mm-handled adapters in the
system.  How do you map a megaraid adapter number to a struct
Scsi_Host device, to be sure you're acting on the controller you think
you are?

SCSI LLDDs don't show up in sysfs under /sys/bus/scsi/drivers at
present, which is where, I think, you would want to put megaraid_mm
with links to show the scsi_host and pci_dev associated with this
adapter.  Something like this:

/sys
|-- bus
|   `-- drivers
|       `-- scsi
|           `-- megaraid_mm
|               `-- adapter0
|                   |-- pci_dev -> ../../../../../devices/pci0000:03/0000:03:06.0
|                   `-- scsi_host -> ../../../../../class/scsi/scsi_host/host0
|-- class
|   `-- scsi
|       `-- scsi_host
|           `-- host0
`-- devices
    `-- pci0000:03
        `-- 0000:03:06.0

Thoughts?
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Wed, 2004-12-15 at 10:48 -0600, Matt Domsch wrote:
> SCSI LLDDs don't show up in sysfs under /sys/bus/scsi/drivers at
> present, which is where, I think, you would want to put megaraid_mm
> with links to show the scsi_host and pci_dev associated with this
> adapter.  Something like this:
> 
> /sys
> |-- bus
> |   `-- drivers
> |       `-- scsi
> |           `-- megaraid_mm
> |               `-- adapter0
> |                   |-- pci_dev -> ../../../../../devices/pci0000:03/0000:03:06.0

Actually, there is a reason why this doesn't happen:  The drivers
directory shows only drivers belonging to a bus.  The megaraid_mm driver
is actually a PCI driver, and thus belongs to the PCI bus.  So, you'll
see this under

/sys/bus/pci/drivers/megaraid_mm/<pci_id>/host<n>

That should be sufficient to obtain all instances.  Note: the instances
aren't numerically indexed under this scheme (unlike your adapter<n>)
they'd be indexed by a unique pci_id.

So it looks like all the information is accessible today (if you look
for it in a slightly different way).  Is there anything currently
missing?

James

Re: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Wed, 2004-12-15 at 01:24 -0600, Matt Domsch wrote:
> James, I've been thinking about this a little more, and you may be on
> to something here. Let each driver add files as such:
> 
> /sys/class/scsi_host
>  |-- host0
>  |   |-- add_logical_drive
>  |   |-- remove_logical_drive
>  |   `-- rescan_logical_drive
> 
> Then we can go 2 ways with this.
> 1) driver functions directly call scsi_add_device(),
> scsi_remove_device(), and something for rescan (option 2 handles this
> one cleanly for us).  ATM, megaraid_mbox doesn't implement a rescan
> function, so this point may be moot. 
> 
> 2) driver functions call a midlayer library function, which invokes
>    /sbin/hotplug with appropriate data, and add a new /etc/hotplug.d
>    helper app which would then write to these files:
> 
> /sys/class/scsi_host
> |-- host0
> |   |-- scan
> /sys/devices/pci0000:0x/0000:0x:0x.0/host0
> |-- 0:0:0:0
> |   |-- delete
> |   |-- rescan
> 
> to do likewise.

I'll buy this (option 2).. it seems like a good way to export the
megaraid specific information and at the same time integrate it more
fully into the evolving hotplug infrastructure.

James

Re: How to add/drop SCSI drives from within the driver?

[Matt Domsch]

On Wed, Dec 15, 2004 at 01:49:18PM -0500, James Bottomley wrote:
> On Wed, 2004-12-15 at 01:24 -0600, Matt Domsch wrote:
> > James, I've been thinking about this a little more, and you may be on
> > to something here. Let each driver add files as such:
> > 
> > /sys/class/scsi_host
> >  |-- host0
> >  |   |-- add_logical_drive
> >  |   |-- remove_logical_drive
> >  |   `-- rescan_logical_drive
> > 
> > Then we can go 2 ways with this.
> > 1) driver functions directly call scsi_add_device(),
> > scsi_remove_device(), and something for rescan (option 2 handles this
> > one cleanly for us).  ATM, megaraid_mbox doesn't implement a rescan
> > function, so this point may be moot. 
> > 
> > 2) driver functions call a midlayer library function, which invokes
> >    /sbin/hotplug with appropriate data, and add a new /etc/hotplug.d
> >    helper app which would then write to these files:
> > to do likewise.
> 
> I'll buy this (option 2).. it seems like a good way to export the
> megaraid specific information and at the same time integrate it more
> fully into the evolving hotplug infrastructure.

Great, I think that's the right long-term solution.  Can we consider
other paths for the short term, while we evolve the hotplug
infrastructure?  I don't know all the details involved in jumping
straight to option 2 today.

Do you plan to apply LSI's driver patch which adds the driver-private
ioctl to provide the mapping from logical drive address to HCTL value?
Both Dell and LSI have products which are lined up to use this new
ioctl because it's the most expedient thing to do, maintains internal
project schedules, etc, which delaying until this sysfs mechanism hits
will greatly impact those schedules. (I know, many folks on this list
don't care about business-side impacts of choices made on-list.)

If you were to apply LSI's ioctl patch, then that patch becomes stage
0.

Stage 1 is what I posted last night, once tested, and the mgmt apps
convert to use that.

The above can happen pretty quickly I think, i.e. before the holidays.

Stage 2 is option 2 above, which removes the driver calling
scsi_add_device() and friends directly.  Userspace apps converted for
stage 1 are none-the-wiser, as their interface hasn't changed.

This is a Jan-Feb kind of thing, to give proper time for all concerns
to be voiced and included in the kernel-internal APIs, /sbin/hotplug
environment, and userspace helper.

Stage 3, convert all other drivers who need a similar mechanism over
to the new helper functions added in stage 2.

This would be ongoing.


Thoughts?

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: How to add/drop SCSI drives from within the driver?

[Arjan van de Ven]

On Wed, 2004-12-15 at 15:30 -0600, Matt Domsch wrote:
> Do you plan to apply LSI's driver patch which adds the driver-private
> ioctl to provide the mapping from logical drive address to HCTL value?
> Both Dell and LSI have products which are lined up to use this new
> ioctl because it's the most expedient thing to do, maintains internal
> project schedules, etc, which delaying until this sysfs mechanism hits
> will greatly impact those schedules. (I know, many folks on this list
> don't care about business-side impacts of choices made on-list.)

I'm strongly against adding this. The reason for that is that once an
ioctl is added, it realistically will and can never go away.
LSI is free to have their own fork and give that to dell; but they
should and could have known that it wasn't going to fly. (same I guess
for adaptec ioctls). The companies who then commit to some schedule
realize they take a huge risk, but that is no reason to foul up the
kernel more. 

Re: How to add/drop SCSI drives from within the driver?

[Alan Cox]

On Iau, 2004-12-16 at 09:54, Arjan van de Ven wrote:
> I'm strongly against adding this. The reason for that is that once an
> ioctl is added, it realistically will and can never go away.
> LSI is free to have their own fork and give that to dell; but they
> should and could have known that it wasn't going to fly. (same I guess
> for adaptec ioctls). The companies who then commit to some schedule
> realize they take a huge risk, but that is no reason to foul up the
> kernel more. 

I agree. I'd like to see an agreed standard interface for dropping and
managing physical volumes and drives, as well as a standard interface
for dropping/managing logical volumes.

RE: How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

>
>On Wed, 2004-12-08 at 01:16, Bagalkote, Sreenivas wrote:
>> Adding a drive:- For application to use sysfs to scan newly added 
>> drive, it needs to know the HCTL (SCSI address - Host, 
>Channel, Target 
>> & Lun) of the drive. Driver is the only one that knows the mapping 
>> between a drive and the corresponding HCTL.
>
>The real way I'd like to handle this is via hotplug.  The 
>hotplug event would transmit the HCTL in the environment.  
>Whether the drive actually gets incorporated into the system 
>and where is user policy, so it's appropriate that it should 
>be in userland.

James, it is the application that is adding the drive. So it is not a
hotplug
event for the driver.

>
>This same infrastructure could be used by fibre channel login, 
>scsi enclosure events etc.
>
>We have some of the hotplug infrastructure in SCSI, but not 
>quite enough for this ... you'll need an additional API.
>
>> Removing a drive:- There is no sane way for the application 
>to map out 
>> drives to /dev/sd<x>. If application has a way of knowing 
>the HCTL of 
>> a deleted drive, then using that HCTL, it can match the 
>corresponding 
>> SCSI device name (/dev/sd<x>) and use sysfs to remove that drive.
>
>Since The sysfs device name contains H:C:T:L surely you can 
>just do a find on /sys?
>
>James
>
>

Thank you,
sreenivas

RE: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Wed, 2004-12-08 at 12:56 -0500, Bagalkote, Sreenivas wrote:
> >The real way I'd like to handle this is via hotplug.  The 
> >hotplug event would transmit the HCTL in the environment.  
> >Whether the drive actually gets incorporated into the system 
> >and where is user policy, so it's appropriate that it should 
> >be in userland.
> 
> James, it is the application that is adding the drive. So it is not a
> hotplug
> event for the driver.

Then perhaps I don't understand what the issue is.  If the application
is adding the drive, then surely it would know the numbers.  If not,
then the driver must communicate this back, and that's what the hotplug
would be about.

James

RE: How to add/drop SCSI drives from within the driver?

[Mukker, Atul]

 
> devices projected on channels N+1..M.  It now projects them 
> as physical devices on channels 0..N (N=number of physical 
> channels on the controller {1,2,4} so far), and the logical 
> drives all on channel N+1.
It makes more sense to export the devices (non-disk, ROMB) on the physical
channel at their actual address. That the reason virtual driver were pushed
out to virtual channel

> 
> Please don't typedef structs, especially not one that's 32 
> bytes long.   
> 
> > +	uint32_t	host_no;
> 
> __u32 is the preferred type when sharing such with userspace.
Yes, typedefs are evil. Is there are good reason to prefer __u32 since
driver conventionally uses uint32_t. The latter definition is available to
userspace as well, right?

> 
> > +	uint32_t	channel;
> > +	uint32_t	scsi_id;
> > +	uint32_t	lun;
> > +	uint32_t	ld;
> > +	uint32_t	reserved[3];
> 
> Why pad this out?
Not pad, possible future expansion of the definition of this packet.

> 
> > +} ld_device_address_t;
> 
> I would also suggest __attribute__((packed)) since you're 
> sharing with userspace just to ensure you can never get it 
> grown by the compiler (it shouldn't be, but just to be safe).
Yes

> 
> Also, as I believe you intend to add some form of INQUIRY 
> Page 81 unique ID to your firmware sets at some point, 
> perhaps this is the logical place to export such information 
> too.  This would then match SCSI_IOCTL_GET_IDLUN in concept.  

The scsi megaraid stack does not have unique identifiers for the logical
drives, and when it happens for future FW, it may not be 32-bits. But you
are right, we should accommodate for this eventuality.


> >  	adapter_t *adapter;
> > +	ld_device_address_t* lda;
> 
> this will be struct ld_device_address *lda of course.
Yes

> 
> > +		if ((lda->ld < 0) || (lda->ld > 
> MAX_LOGICAL_DRIVES_40LD)) {
> 
> It's a __u32, it can't be < 0 (and the compiler should have 
You are right, only the latter check is valid


> warned at that).  Why MAX_LOGICAL_DRIVES_40LD and not 
> MAX_LOGICAL_DRIVES_64LD (which strangely is defined as 65) 
> which is used elsewhere in the driver for such mapping?
For megaraid_mbox, the max limit is 40LD, 64LD is defined in the
mega_common.h, which will be valid for other stacks. 

> 
> > +		lda->scsi_id	= (lda->ld < adapter->init_id) ? 
> > +						lda->ld : lda->ld + 1;
> 
> Ahh, you project an initiator onto the logical drive channel, 
> at init_id, yes?  That wasn't obvious to me at first, is it 
> necessary to do so?  There's no real SCSI bus contention 
> issue, and you can't address the initiator directly.  (again, 
> the driver does this today, and such a change would need to 
> be reflected in this routine
Unfortunately, the mid-layer does not allow initiator id per bus, for
multi-bus devices like megaraid. Since, we have to reserve initiator id for
physical channels, it carries over to virtual channel as well.

This also explains why MAX_LOGICAL_DRIVES_64LD is defined as (64+1)!, since
we lose one spot for initiator id.

Thanks
-Atul Mukker

RE: How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

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

Hello All,

Last week we proposed to add couple of IOCTLs to scan/remove the LDs.
It was suggested on the list that our application go sysfs route instead.
We are taking that route. (Matt Domsh, Christoph, JBG and Brian King - 
thanks for your feedback! It is appreciated.)

Please review our updated proposal. Considering that our application can 
add and remove drives on the fly:

Adding a drive:- For application to use sysfs to scan newly added drive,
it needs to know the HCTL (SCSI address - Host, Channel, Target & Lun)
of the drive. Driver is the only one that knows the mapping between a 
drive and the corresponding HCTL.

Removing a drive:- There is no sane way for the application to map out
drives to /dev/sd<x>. If application has a way of knowing the HCTL of a
deleted drive, then using that HCTL, it can match the corresponding SCSI
device name (/dev/sd<x>) and use sysfs to remove that drive.

To this end, we are requesting to be allowed to add a small IOCTL to the
driver that returns the HCTL for an LD. Our strong case is really the
"Adding
a drive" case. Somebody may suggest that an application can blindly
scan all channels and all targets on a host when it adds one drive. That
would
_not_ be correct thing to do. Application should ideally scan only the
drives that 
it has added.

I am attaching and inlining the patch that implements this logic.

Thank you,
Sreenivas
LSI Logic

diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
--- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-07
16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-07
23:57:51.415674008 -0500
@@ -51,6 +51,7 @@
 #define MEGAIOC_QNADAP		'm'	/* Query # of adapters		*/
 #define MEGAIOC_QDRVRVER	'e'	/* Query driver version		*/
 #define MEGAIOC_QADAPINFO   	'g'	/* Query adapter information	*/
+#define MEGAIOC_GET_SADDR	'a'	/* Get SCSI address of an LD	*/
 
 #define USCSICMD		0x80
 #define UIOC_RD			0x00001
@@ -62,6 +63,7 @@
 #define GET_ADAP_INFO		0x30000
 #define GET_CAP			0x40000
 #define GET_STATS		0x50000
+#define GET_LD_SADDR		0x60000
 #define GET_IOCTL_VERSION	0x01
 
 #define EXT_IOCTL_SIGN_SZ	16
@@ -217,6 +219,27 @@
 
 } __attribute__ ((packed)) mcontroller_t;
 
+/**
+ * ld_device_address_t	: SCSI device address for the logical drives
+ * 
+ * @host_no		: host# to which LD belogs; as understood by
mid-layer
+ * @channel		: channel on which LD is exported
+ * @target		: target on which the LD is exported
+ * @lun			: lun on which the LD is exported
+ * @ld			: the LD for which this information is sought
+ * @reserved		: reserved fields; must be set to zero
+ *
+ * NOTE			: Applications set the LD number and expect
the 
+ * 			  SCSI address to be returned in the first four
fields
+ */
+typedef struct {
+	uint32_t	host_no;
+	uint32_t	channel;
+	uint32_t	scsi_id;
+	uint32_t	lun;
+	uint32_t	ld;
+	uint32_t	reserved[3];
+} ld_device_address_t;
 
 /**
  * mm_dmapool_t	: Represents one dma pool with just one buffer
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.c
new-rc2/drivers/scsi/megaraid/megaraid_mbox.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-07
16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-08
01:29:39.420330024 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mbox.c
- * Version	: v2.20.4.1 (Nov 04 2004)
+ * Version	: v2.20.4.1+ TEST VERSION
  *
  * Authors:
  * 	Atul Mukker		<Atul.Mukker@lsil.com>
@@ -3642,6 +3642,7 @@
 megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, uint32_t
action)
 {
 	adapter_t *adapter;
+	ld_device_address_t* lda;
 
 	if (action != IOCTL_ISSUE) {
 		con_log(CL_ANN, (KERN_WARNING
@@ -3670,6 +3671,30 @@
 
 		return kioc->status;
 
+	case GET_LD_SADDR:
+		
+		lda = (ld_device_address_t*)(unsigned long)kioc->buf_vaddr;
+
+		if ((lda->ld < 0) || (lda->ld > MAX_LOGICAL_DRIVES_40LD)) {
+			lda->host_no	= 0xffffffff;
+			lda->channel	= 0xffffffff;
+			lda->scsi_id	= 0xffffffff;
+			lda->lun	= 0xffffffff;
+			kioc->status	= -ENODEV;
+			kioc->done(kioc);
+			return kioc->status;
+		}
+
+		lda->host_no	= adapter->host->host_no;
+		lda->channel	= adapter->max_channel;
+		lda->scsi_id	= (lda->ld < adapter->init_id) ? 
+						lda->ld : lda->ld + 1;
+		lda->lun	= 0;
+		kioc->status	= 0;
+		kioc->done(kioc);
+
+		return kioc->status;
+
 	case MBOX_CMD:
 
 		return megaraid_mbox_mm_command(adapter, kioc);
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.h
new-rc2/drivers/scsi/megaraid/megaraid_mbox.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-07
16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-07
23:53:35.047647872 -0500
@@ -21,8 +21,8 @@
 #include "megaraid_ioctl.h"
 
 
-#define MEGARAID_VERSION	"2.20.4.1"
-#define MEGARAID_EXT_VERSION	"(Release Date: Thu Nov  4 17:44:59 EST
2004)"
+#define MEGARAID_VERSION	"2.20.4.1+ TEST VERSION"
+#define MEGARAID_EXT_VERSION	"(Release Date: TEST VERSION)"
 
 
 /*
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.c
new-rc2/drivers/scsi/megaraid/megaraid_mm.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-07
16:40:16.015101592 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-08
01:27:35.171218760 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mm.c
- * Version	: v2.20.2.2 (Nov 04 2004)
+ * Version	: v2.20.2.2+ TEST VERSION
  *
  * Common management module
  */
@@ -373,6 +373,20 @@
 			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
 				return (-ENOMEM);
 		}
+		else if (subopcode == MEGAIOC_GET_SADDR) {
+
+			kioc->opcode	= GET_LD_SADDR;
+			kioc->data_dir	= UIOC_RD;
+			kioc->xferlen	= sizeof(ld_device_address_t);
+
+			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
+				return (-ENOMEM);
+
+			if (copy_from_user(kioc->buf_vaddr, mimd.data,
+							kioc->xferlen )) {
+				return (-EFAULT);
+			}
+		}
 		else {
 			con_log(CL_ANN, (KERN_WARNING
 					"megaraid cmm: Invalid subop\n"));
@@ -809,6 +823,15 @@
 
 			return 0;
 
+		case MEGAIOC_GET_SADDR:
+
+			if (copy_to_user(kmimd.data, kioc->buf_vaddr,
+					sizeof(ld_device_address_t))) {
+				return (-EFAULT);
+			}
+			
+			return kioc->status;
+
 		default:
 			return (-EINVAL);
 		}
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.h
new-rc2/drivers/scsi/megaraid/megaraid_mm.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-07
16:40:15.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-07
23:52:50.975347880 -0500
@@ -29,9 +29,9 @@
 #include "megaraid_ioctl.h"
 
 
-#define LSI_COMMON_MOD_VERSION	"2.20.2.2"
+#define LSI_COMMON_MOD_VERSION	"2.20.2.2+ TEST VERSION"
 #define LSI_COMMON_MOD_EXT_VERSION	\
-		"(Release Date: Thu Nov  4 17:46:29 EST 2004)"
+		"(Release Date: TEST VERSION)"
 
 
 #define LSI_DBGLVL			dbglevel



>-----Original Message-----
>From: Matt Domsch [mailto:Matt_Domsch@dell.com] 
>Sent: Friday, December 03, 2004 12:11 PM
>To: Bagalkote, Sreenivas
>Cc: 'brking@us.ibm.com'; 'James Bottomley'; 
>'linux-kernel@vger.kernel.org'; 'linux-scsi@vger.kernel.org'; 
>'bunk@fs.tum.de'; 'Andrew Morton'; Ju, Seokmann; Doelfel, 
>Hardy; Mukker, Atul
>Subject: Re: How to add/drop SCSI drives from within the driver?
>
>On Fri, Dec 03, 2004 at 10:29:29AM -0500, Bagalkote, Sreenivas wrote:
>> I agree. The sysfs method would have been the most logical 
>way of doing it.
>> But then application becomes sysfs dependent. We really 
>cannot do that.
>
>Why?  With my efibootmgr (a userspace app like your 
>megalib/megamgr/megamon), it looks for the existance of 
>/sys/whatever/, and then operates with that if it is present.  
>Else it looks for the existance of /proc/whatever (in your case
>/proc/scsi/scsi) and does likewise.  No driver ioctl needed.
> 
>> Given that we have to do it from within the driver, is whatever I am 
>> doing right?
>
>Doing it within the driver means you've got to release a new 
>driver for each affected OS, to avoid having to update the 
>userspace app on each OS.  I claim it's much easier to update 
>a userspace lib/app than it is to update a driver on each 
>installed system.
>
>Thanks,
>Matt
>
>--
>Matt Domsch
>Sr. Software Engineer, Lead Engineer
>Dell Linux Solutions linux.dell.com & www.dell.com/linux Linux 
>on Dell mailing lists @ http://lists.us.dell.com
>


[-- Attachment #2: megaraid_hctl.patch --]
[-- Type: application/octet-stream, Size: 5695 bytes --]

diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
--- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-07 16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-07 23:57:51.415674008 -0500
@@ -51,6 +51,7 @@
 #define MEGAIOC_QNADAP		'm'	/* Query # of adapters		*/
 #define MEGAIOC_QDRVRVER	'e'	/* Query driver version		*/
 #define MEGAIOC_QADAPINFO   	'g'	/* Query adapter information	*/
+#define MEGAIOC_GET_SADDR	'a'	/* Get SCSI address of an LD	*/
 
 #define USCSICMD		0x80
 #define UIOC_RD			0x00001
@@ -62,6 +63,7 @@
 #define GET_ADAP_INFO		0x30000
 #define GET_CAP			0x40000
 #define GET_STATS		0x50000
+#define GET_LD_SADDR		0x60000
 #define GET_IOCTL_VERSION	0x01
 
 #define EXT_IOCTL_SIGN_SZ	16
@@ -217,6 +219,27 @@
 
 } __attribute__ ((packed)) mcontroller_t;
 
+/**
+ * ld_device_address_t	: SCSI device address for the logical drives
+ * 
+ * @host_no		: host# to which LD belogs; as understood by mid-layer
+ * @channel		: channel on which LD is exported
+ * @target		: target on which the LD is exported
+ * @lun			: lun on which the LD is exported
+ * @ld			: the LD for which this information is sought
+ * @reserved		: reserved fields; must be set to zero
+ *
+ * NOTE			: Applications set the LD number and expect the 
+ * 			  SCSI address to be returned in the first four fields
+ */
+typedef struct {
+	uint32_t	host_no;
+	uint32_t	channel;
+	uint32_t	scsi_id;
+	uint32_t	lun;
+	uint32_t	ld;
+	uint32_t	reserved[3];
+} ld_device_address_t;
 
 /**
  * mm_dmapool_t	: Represents one dma pool with just one buffer
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.c new-rc2/drivers/scsi/megaraid/megaraid_mbox.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-07 16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-08 01:29:39.420330024 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mbox.c
- * Version	: v2.20.4.1 (Nov 04 2004)
+ * Version	: v2.20.4.1+ TEST VERSION
  *
  * Authors:
  * 	Atul Mukker		<Atul.Mukker@lsil.com>
@@ -3642,6 +3642,7 @@
 megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, uint32_t action)
 {
 	adapter_t *adapter;
+	ld_device_address_t* lda;
 
 	if (action != IOCTL_ISSUE) {
 		con_log(CL_ANN, (KERN_WARNING
@@ -3670,6 +3671,30 @@
 
 		return kioc->status;
 
+	case GET_LD_SADDR:
+		
+		lda = (ld_device_address_t*)(unsigned long)kioc->buf_vaddr;
+
+		if ((lda->ld < 0) || (lda->ld > MAX_LOGICAL_DRIVES_40LD)) {
+			lda->host_no	= 0xffffffff;
+			lda->channel	= 0xffffffff;
+			lda->scsi_id	= 0xffffffff;
+			lda->lun	= 0xffffffff;
+			kioc->status	= -ENODEV;
+			kioc->done(kioc);
+			return kioc->status;
+		}
+
+		lda->host_no	= adapter->host->host_no;
+		lda->channel	= adapter->max_channel;
+		lda->scsi_id	= (lda->ld < adapter->init_id) ? 
+						lda->ld : lda->ld + 1;
+		lda->lun	= 0;
+		kioc->status	= 0;
+		kioc->done(kioc);
+
+		return kioc->status;
+
 	case MBOX_CMD:
 
 		return megaraid_mbox_mm_command(adapter, kioc);
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.h new-rc2/drivers/scsi/megaraid/megaraid_mbox.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-07 16:40:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-07 23:53:35.047647872 -0500
@@ -21,8 +21,8 @@
 #include "megaraid_ioctl.h"
 
 
-#define MEGARAID_VERSION	"2.20.4.1"
-#define MEGARAID_EXT_VERSION	"(Release Date: Thu Nov  4 17:44:59 EST 2004)"
+#define MEGARAID_VERSION	"2.20.4.1+ TEST VERSION"
+#define MEGARAID_EXT_VERSION	"(Release Date: TEST VERSION)"
 
 
 /*
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.c new-rc2/drivers/scsi/megaraid/megaraid_mm.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-07 16:40:16.015101592 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-08 01:27:35.171218760 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mm.c
- * Version	: v2.20.2.2 (Nov 04 2004)
+ * Version	: v2.20.2.2+ TEST VERSION
  *
  * Common management module
  */
@@ -373,6 +373,20 @@
 			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
 				return (-ENOMEM);
 		}
+		else if (subopcode == MEGAIOC_GET_SADDR) {
+
+			kioc->opcode	= GET_LD_SADDR;
+			kioc->data_dir	= UIOC_RD;
+			kioc->xferlen	= sizeof(ld_device_address_t);
+
+			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
+				return (-ENOMEM);
+
+			if (copy_from_user(kioc->buf_vaddr, mimd.data,
+							kioc->xferlen )) {
+				return (-EFAULT);
+			}
+		}
 		else {
 			con_log(CL_ANN, (KERN_WARNING
 					"megaraid cmm: Invalid subop\n"));
@@ -809,6 +823,15 @@
 
 			return 0;
 
+		case MEGAIOC_GET_SADDR:
+
+			if (copy_to_user(kmimd.data, kioc->buf_vaddr,
+					sizeof(ld_device_address_t))) {
+				return (-EFAULT);
+			}
+			
+			return kioc->status;
+
 		default:
 			return (-EINVAL);
 		}
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.h new-rc2/drivers/scsi/megaraid/megaraid_mm.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-07 16:40:15.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-07 23:52:50.975347880 -0500
@@ -29,9 +29,9 @@
 #include "megaraid_ioctl.h"
 
 
-#define LSI_COMMON_MOD_VERSION	"2.20.2.2"
+#define LSI_COMMON_MOD_VERSION	"2.20.2.2+ TEST VERSION"
 #define LSI_COMMON_MOD_EXT_VERSION	\
-		"(Release Date: Thu Nov  4 17:46:29 EST 2004)"
+		"(Release Date: TEST VERSION)"
 
 
 #define LSI_DBGLVL			dbglevel

Re: How to add/drop SCSI drives from within the driver?

[Matt Domsch]

On Wed, Dec 08, 2004 at 02:16:24AM -0500, Bagalkote, Sreenivas wrote:
> To this end, we are requesting to be allowed to add a small IOCTL to the
> driver that returns the HCTL for an LD.

I've looked into solving this another way, but I cannot see how to get
this driver-private mapping of logical drive number -> HCTL without
putting code something like this into the driver.  It's a figment of
how logical drives are projected as actual drives by the driver, and
by providing a mapping function to userspace, the driver is free to
change its mapping algorithm in the future if necessary without
breaking userspace management apps that relied on a set mapping
strategy.  Megaraid has changed their mapping strategy before (logical
drives used to be projected on channels 0..N, 16 drives per channel,
with physical non-disk devices projected on channels N+1..M.  It now
projects them as physical devices on channels 0..N (N=number of
physical channels on the controller {1,2,4} so far), and the logical
drives all on channel N+1.


>  Our strong case is really the "Adding a drive" case. Somebody may
> suggest that an application can blindly scan all channels and all
> targets on a host when it adds one drive. That would _not_ be
> correct thing to do. Application should ideally scan only the drives
> that it has added.

I'm less worried about the add case then about the remove case.
Blindly removing devices, then adding them back in, as the 'scanbus'
script widely available on the internet does, is a recipe for
disaster, frought with race conditions on 2.4 kernels (races between
sg_open() and scsi-remove-single-device, and between
scsi-add-single-device and scsi-remove-single-device, just to name
two), and outright dangerous on 2.6 (you can remove an in-use device
such as your boot disk if you use the /proc interface as root).

> --- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-07
> 16:40:16.000000000 -0500
> +++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-07
> 23:57:51.415674008 -0500
> @@ -217,6 +219,27 @@
>  
>  } __attribute__ ((packed)) mcontroller_t;
>  
> +/**
> + * ld_device_address_t	: SCSI device address for the logical drives
> + * 
> + * @host_no		: host# to which LD belogs; as understood by
> mid-layer
> + * @channel		: channel on which LD is exported
> + * @target		: target on which the LD is exported
> + * @lun			: lun on which the LD is exported
> + * @ld			: the LD for which this information is sought
> + * @reserved		: reserved fields; must be set to zero
> + *
> + * NOTE			: Applications set the LD number and expect
> the 
> + * 			  SCSI address to be returned in the first four
> fields
> + */
> +typedef struct {

Please don't typedef structs, especially not one that's 32 bytes long.   

> +	uint32_t	host_no;

__u32 is the preferred type when sharing such with userspace.

> +	uint32_t	channel;
> +	uint32_t	scsi_id;
> +	uint32_t	lun;
> +	uint32_t	ld;
> +	uint32_t	reserved[3];

Why pad this out?

> +} ld_device_address_t;

I would also suggest __attribute__((packed)) since you're sharing with
userspace just to ensure you can never get it grown by the compiler
(it shouldn't be, but just to be safe).

Also, as I believe you intend to add some form of INQUIRY Page 81
unique ID to your firmware sets at some point, perhaps this is the
logical place to export such information too.  This would then match
SCSI_IOCTL_GET_IDLUN in concept.  

> --- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-07
> 16:40:16.000000000 -0500
> +++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-08
> 01:29:39.420330024 -0500
> @@ -3642,6 +3642,7 @@
>  megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, uint32_t
> action)
>  {
>  	adapter_t *adapter;
> +	ld_device_address_t* lda;

this will be struct ld_device_address *lda of course.

> +	case GET_LD_SADDR:

Trivial: I prefer cases which are >10 lines to be moved out-of-line
into a static inline function, for readability.

> +		
> +		lda = (ld_device_address_t*)(unsigned long)kioc->buf_vaddr;
> +
> +		if ((lda->ld < 0) || (lda->ld > MAX_LOGICAL_DRIVES_40LD)) {

It's a __u32, it can't be < 0 (and the compiler should have warned at
that).  Why MAX_LOGICAL_DRIVES_40LD and not MAX_LOGICAL_DRIVES_64LD
(which strangely is defined as 65) which is used elsewhere in the
driver for such mapping?

> +		lda->scsi_id	= (lda->ld < adapter->init_id) ? 
> +						lda->ld : lda->ld + 1;

Ahh, you project an initiator onto the logical drive channel, at
init_id, yes?  That wasn't obvious to me at first, is it necessary to
do so?  There's no real SCSI bus contention issue, and you can't
address the initiator directly.  (again, the driver does this today,
and such a change would need to be reflected in this routine


The rest seems sane to me.

Thanks,
Matt


-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: How to add/drop SCSI drives from within the driver?

[Matthew Wilcox]

On Wed, Dec 08, 2004 at 08:07:55AM -0600, Matt Domsch wrote:
> > +} ld_device_address_t;
> 
> I would also suggest __attribute__((packed)) since you're sharing with
> userspace just to ensure you can never get it grown by the compiler
> (it shouldn't be, but just to be safe).

That's a bad idea, it forces the compiler to pessimise accesses to the
struct on most hardware.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: How to add/drop SCSI drives from within the driver?

[Bryan Henderson]

>> I would also suggest __attribute__((packed)) since you're sharing with
>> userspace just to ensure you can never get it grown by the compiler
>> (it shouldn't be, but just to be safe).
>
>That's a bad idea, it forces the compiler to pessimise accesses to the
>struct on most hardware.

How?  We're assuming that it won't actually affect the layout of this 
struct, so what does the compiler pessimize?

Re: How to add/drop SCSI drives from within the driver?

[Arjan van de Ven]

On Wed, 2004-12-08 at 15:08 -0500, Bryan Henderson wrote:
> >> I would also suggest __attribute__((packed)) since you're sharing with
> >> userspace just to ensure you can never get it grown by the compiler
> >> (it shouldn't be, but just to be safe).
> >
> >That's a bad idea, it forces the compiler to pessimise accesses to the
> >struct on most hardware.
> 
> How?  We're assuming that it won't actually affect the layout of this 
> struct, so what does the compiler pessimize?

it also makes the compiler assume the struct members aren't aligned to
the natural alignment and thus it'll emit more expensive access paterns
on architectures that care about such things.
-- 

RE: How to add/drop SCSI drives from within the driver?

[James Bottomley]

On Wed, 2004-12-08 at 01:16, Bagalkote, Sreenivas wrote:
> Adding a drive:- For application to use sysfs to scan newly added drive,
> it needs to know the HCTL (SCSI address - Host, Channel, Target & Lun)
> of the drive. Driver is the only one that knows the mapping between a 
> drive and the corresponding HCTL.

The real way I'd like to handle this is via hotplug.  The hotplug event
would transmit the HCTL in the environment.  Whether the drive actually
gets incorporated into the system and where is user policy, so it's
appropriate that it should be in userland.

This same infrastructure could be used by fibre channel login, scsi
enclosure events etc.

We have some of the hotplug infrastructure in SCSI, but not quite enough
for this ... you'll need an additional API.

> Removing a drive:- There is no sane way for the application to map out
> drives to /dev/sd<x>. If application has a way of knowing the HCTL of a
> deleted drive, then using that HCTL, it can match the corresponding SCSI
> device name (/dev/sd<x>) and use sysfs to remove that drive.

Since The sysfs device name contains H:C:T:L surely you can just do a
find on /sys?

James

Re: How to add/drop SCSI drives from within the driver?

[Brian King]

Bagalkote, Sreenivas wrote:
> Adding a drive:- For application to use sysfs to scan newly added drive,
> it needs to know the HCTL (SCSI address - Host, Channel, Target & Lun)
> of the drive. Driver is the only one that knows the mapping between a 
> drive and the corresponding HCTL.

Can you explain from a high level how megaraid adds logical devices?
I assume a management app of some sort sends a command to the adapter
to create a disk array. What interface is used for this?
Does this trigger any notification from the adapter to the device
driver?

> Removing a drive:- There is no sane way for the application to map out
> drives to /dev/sd<x>. 

This information is all available in sysfs. Have you looked at libsysfs
at all?



-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center

RE: How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

4 Against        0 For

That settles it. Thank you all for your replies.

Sreenivas
LSI Logic 

>-----Original Message-----
>From: Matt Domsch [mailto:Matt_Domsch@dell.com] 
>Sent: Friday, December 03, 2004 12:14 PM
>To: Bagalkote, Sreenivas
>Cc: 'brking@us.ibm.com'; 'James Bottomley'; 
>'linux-kernel@vger.kernel.org'; 'linux-scsi@vger.kernel.org'; 
>'bunk@fs.tum.de'; 'Andrew Morton'; Ju, Seokmann; Doelfel, 
>Hardy; Mukker, Atul
>Subject: Re: How to add/drop SCSI drives from within the driver?
>
>On Fri, Dec 03, 2004 at 11:11:01AM -0600, Matt Domsch wrote:
>> Doing it within the driver means you've got to release a new driver 
>> for each affected OS, to avoid having to update the userspace app on 
>> each OS.  I claim it's much easier to update a userspace 
>lib/app than 
>> it is to update a driver on each installed system.
>
>And, you're going to have to update the userspace lib/app to 
>add the new ioctl()-invocation to it anyhow.  So take the hit 
>*only* there and make it use either /sys or /proc, whichever 
>is available, no kernel changes.  Yes?
>
>--
>Matt Domsch
>Sr. Software Engineer, Lead Engineer
>Dell Linux Solutions linux.dell.com & www.dell.com/linux Linux 
>on Dell mailing lists @ http://lists.us.dell.com
>

RE: How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

I agree. The sysfs method would have been the most logical way of doing it.
But then application becomes sysfs dependent. We really cannot do that.

Given that we have to do it from within the driver, is whatever I am doing
right?

Thanks,
Sreenivas

>-----Original Message-----
>From: Brian King [mailto:brking@us.ibm.com] 
>Sent: Friday, December 03, 2004 10:11 AM
>To: Bagalkote, Sreenivas
>Cc: 'James Bottomley'; 'linux-kernel@vger.kernel.org'; 
>'linux-scsi@vger.kernel.org'; 'bunk@fs.tum.de'; 'Andrew 
>Morton'; 'Matt_Domsch@dell.com'; Ju, Seokmann; Doelfel, Hardy; 
>Mukker, Atul
>Subject: Re: How to add/drop SCSI drives from within the driver?
>
>This looks to be adding an LLD specific interface to userspace 
>to add/delete disks. Why can't the existing sysfs interfaces 
>be used to to this? (scan attribute on host and delete 
>attribute on device).
>
>-Brian
>
>Bagalkote, Sreenivas wrote:
>> Hello All,
>> 
>> I am trying to implement a feature in my SCSI driver, where it can 
>> trigger the SCSI mid-layer to scan or remove a particular drive. I 
>> appreciate any help in nudging me in the right direction.
>> 
>> The exported functions -
>> 
>> scsi_add_device( host, channel, target, lun ) scsi_remove_device( 
>> struct scsi_device* )
>> 
>> seem to work well in my limited testing. Are there any 
>caveats in this 
>> method? Does this method work well without exceptions? I am inlining 
>> the full patch for megaraid SCSI driver taken against 
>2.6.10-rc2. I am 
>> also attaching it to this mail.
>> 
>> Thank you,
>> Sreenivas
>> 
>> 
>> ----
>> diff -Naur old-rc3/drivers/scsi/megaraid/mega_common.h
>> new-rc2/drivers/scsi/megaraid/mega_common.h
>> --- old-rc2/drivers/scsi/megaraid/mega_common.h	2004-10-18
>> 17:54:31.000000000 -0400
>> +++ new-rc2/drivers/scsi/megaraid/mega_common.h	2004-12-02
>> 20:23:35.000000000 -0500
>> @@ -242,6 +242,15 @@
>>  					[SCP2TARGET(scp)] & 
>0xFF);	\
>>  	}
>>  
>> +/**
>> + * MRAID_LD_TARGET
>> + * @param adp		- Adapter's soft state
>> + * @param ld		- Logical drive number
>> + *
>> + * Macro to retrieve the SCSI target id of a logical drive  */ 
>> +#define MRAID_LD_TARGET(adp, ld) (((ld) < (adp)->init_id) ? (ld) : 
>> +(ld)+1)
>> 
>> +
>>  /*
>>   * ### Helper routines ###
>>   */
>> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
>> new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
>> --- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-02
>> 20:20:16.000000000 -0500
>> +++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-02
>> 20:23:25.000000000 -0500
>> @@ -51,8 +51,11 @@
>>  #define MEGAIOC_QNADAP		'm'	/* Query # of 
>adapters		*/
>>  #define MEGAIOC_QDRVRVER	'e'	/* Query driver version	
>	*/
>>  #define MEGAIOC_QADAPINFO   	'g'	/* Query 
>adapter information	*/
>> +#define MEGAIOC_ADD_LD		'a'
>> +#define MEGAIOC_DEL_LD		'r'
>>  
>>  #define USCSICMD		0x80
>> +#define UIOC_NONE		0x00000
>>  #define UIOC_RD			0x00001
>>  #define UIOC_WR			0x00002
>>  
>> @@ -62,6 +65,8 @@
>>  #define GET_ADAP_INFO		0x30000
>>  #define GET_CAP			0x40000
>>  #define GET_STATS		0x50000
>> +#define	ADD_LD			0x60000
>> +#define DEL_LD			0x70000
>>  #define GET_IOCTL_VERSION	0x01
>>  
>>  #define EXT_IOCTL_SIGN_SZ	16
>> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.c
>> new-rc2/drivers/scsi/megaraid/megaraid_mbox.c
>> --- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-02
>> 20:20:16.000000000 -0500
>> +++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-02
>> 20:32:33.408278216 -0500
>> @@ -10,7 +10,7 @@
>>   *	   2 of the License, or (at your option) any later version.
>>   *
>>   * FILE		: megaraid_mbox.c
>> - * Version	: v2.20.4.1 (Nov 04 2004)
>> + * Version	: v2.20.4.1+ TEST VERSION
>>   *
>>   * Authors:
>>   * 	Atul Mukker		<Atul.Mukker@lsil.com>
>> @@ -3642,6 +3642,10 @@
>>  megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, 
>> uint32_t
>> action)
>>  {
>>  	adapter_t *adapter;
>> +	uint32_t ld;
>> +	struct scsi_device* sdev;
>> +	int ch;
>> +	int tg;
>>  
>>  	if (action != IOCTL_ISSUE) {
>>  		con_log(CL_ANN, (KERN_WARNING
>> @@ -3670,6 +3674,31 @@
>>  
>>  		return kioc->status;
>>  
>> +	case ADD_LD:
>> +		ld = *(uint32_t*) kioc->buf_vaddr;
>> +		ch = adapter->max_channel;
>> +		tg = MRAID_LD_TARGET( adapter, ld );
>> +		scsi_add_device(adapter->host, ch, tg, 0);
>> +
>> +		kioc->status = 0;
>> +		kioc->done(kioc);
>> +		return kioc->status;
>> +
>> +	case DEL_LD:
>> +		ld = *(uint32_t*) kioc->buf_vaddr;
>> +		ch = adapter->max_channel;
>> +		tg = MRAID_LD_TARGET( adapter, ld );
>> +		sdev = scsi_device_lookup( adapter->host, ch, tg, 0);
>> +		
>> +		if( sdev ) {
>> +			scsi_remove_device( sdev );
>> +			scsi_device_put( sdev );
>> +		}
>> +
>> +		kioc->status = 0;
>> +		kioc->done(kioc);
>> +		return kioc->status;
>> +
>>  	case MBOX_CMD:
>>  
>>  		return megaraid_mbox_mm_command(adapter, kioc); 
>diff -Naur 
>> old-rc2/drivers/scsi/megaraid/megaraid_mbox.h
>> new-rc2/drivers/scsi/megaraid/megaraid_mbox.h
>> --- old-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-02
>> 20:20:16.000000000 -0500
>> +++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-02
>> 20:32:09.737876664 -0500
>> @@ -21,8 +21,8 @@
>>  #include "megaraid_ioctl.h"
>>  
>>  
>> -#define MEGARAID_VERSION	"2.20.4.1"
>> -#define MEGARAID_EXT_VERSION	"(Release Date: Thu Nov 
> 4 17:44:59 EST
>> 2004)"
>> +#define MEGARAID_VERSION	"2.20.4.1+ TEST VERSION"
>> +#define MEGARAID_EXT_VERSION	"(Release Date: TEST VERSION)"
>>  
>>  
>>  /*
>> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.c
>> new-rc2/drivers/scsi/megaraid/megaraid_mm.c
>> --- old-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-02
>> 20:20:16.000000000 -0500
>> +++ new-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-02
>> 20:22:57.000000000 -0500
>> @@ -373,6 +373,34 @@
>>  			if (mraid_mm_attach_buf(adp, kioc, 
>kioc->xferlen))
>>  				return (-ENOMEM);
>>  		}
>> +		else if (subopcode == MEGAIOC_ADD_LD) {
>> +
>> +			kioc->opcode	= ADD_LD;
>> +			kioc->data_dir	= UIOC_NONE;
>> +			kioc->xferlen	= sizeof(uint32_t);
>> +
>> +			if (mraid_mm_attach_buf(adp, kioc, 
>kioc->xferlen))
>> +				return -(ENOMEM);
>> +
>> +			if (copy_from_user(kioc->buf_vaddr, mimd.data,
>> +							
>kioc->xferlen)) {
>> +				return (-EFAULT);
>> +			}
>> +		}
>> +		else if (subopcode == MEGAIOC_DEL_LD) {
>> +
>> +			kioc->opcode	= DEL_LD;
>> +			kioc->data_dir	= UIOC_NONE;
>> +			kioc->xferlen	= sizeof(uint32_t);
>> +
>> +			if (mraid_mm_attach_buf(adp, kioc, 
>kioc->xferlen))
>> +				return -(ENOMEM);
>> +
>> +			if (copy_from_user(kioc->buf_vaddr, mimd.data,
>> +							
>kioc->xferlen)) {
>> +				return (-EFAULT);
>> +			}
>> +		}
>>  		else {
>>  			con_log(CL_ANN, (KERN_WARNING
>>  					"megaraid cmm: Invalid 
>subop\n")); @@ -809,6 +837,9 @@
>>  
>>  			return 0;
>>  
>> +		case MEGAIOC_ADD_LD:
>> +		case MEGAIOC_DEL_LD:
>> +			return 0;
>>  		default:
>>  			return (-EINVAL);
>>  		}
>> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.h
>> new-rc2/drivers/scsi/megaraid/megaraid_mm.h
>> --- old-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-02
>> 20:20:16.000000000 -0500
>> +++ new-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-02
>> 20:33:00.709127856 -0500
>> @@ -29,9 +29,9 @@
>>  #include "megaraid_ioctl.h"
>>  
>>  
>> -#define LSI_COMMON_MOD_VERSION	"2.20.2.2"
>> +#define LSI_COMMON_MOD_VERSION	"2.20.2.2+ TEST VERSION"
>>  #define LSI_COMMON_MOD_EXT_VERSION	\
>> -		"(Release Date: Thu Nov  4 17:46:29 EST 2004)"
>> +		"(Release Date: TEST VERSION)"
>>  
>>  
>>  #define LSI_DBGLVL			dbglevel
>> ----
>> 
>
>--
>Brian King
>eServer Storage I/O
>IBM Linux Technology Center
>

Re: How to add/drop SCSI drives from within the driver?

[Jan-Benedict Glaw]

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

On Fri, 2004-12-03 10:29:29 -0500, Bagalkote, Sreenivas <sreenib@lsil.com>
wrote in message <0E3FA95632D6D047BA649F95DAB60E570230CA70@exa-atlanta>:
> I agree. The sysfs method would have been the most logical way of doing it.

Then this is the way to go.

> But then application becomes sysfs dependent. We really cannot do that.

You can, I'm sure :)

MfG, JBG

-- 
Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481             _ O _
"Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg  _ _ O
 fuer einen Freien Staat voll Freier Bürger" | im Internet! |   im Irak!   O O O
ret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA));

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: How to add/drop SCSI drives from within the driver?

[Christoph Hellwig]

On Fri, Dec 03, 2004 at 10:29:29AM -0500, Bagalkote, Sreenivas wrote:
> I agree. The sysfs method would have been the most logical way of doing it.
> But then application becomes sysfs dependent. We really cannot do that.

And we cannot add more ioctl interfaces.

Re: How to add/drop SCSI drives from within the driver?

[Matt Domsch]

On Fri, Dec 03, 2004 at 10:29:29AM -0500, Bagalkote, Sreenivas wrote:
> I agree. The sysfs method would have been the most logical way of doing it.
> But then application becomes sysfs dependent. We really cannot do that.

Why?  With my efibootmgr (a userspace app like your
megalib/megamgr/megamon), it looks for the existance of
/sys/whatever/, and then operates with that if it is present.  Else it
looks for the existance of /proc/whatever (in your case
/proc/scsi/scsi) and does likewise.  No driver ioctl needed.
 
> Given that we have to do it from within the driver, is whatever I am doing
> right?

Doing it within the driver means you've got to release a new driver
for each affected OS, to avoid having to update the userspace app on
each OS.  I claim it's much easier to update a userspace lib/app than
it is to update a driver on each installed system.

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: How to add/drop SCSI drives from within the driver?

[Matt Domsch]

On Fri, Dec 03, 2004 at 11:11:01AM -0600, Matt Domsch wrote:
> Doing it within the driver means you've got to release a new driver
> for each affected OS, to avoid having to update the userspace app on
> each OS.  I claim it's much easier to update a userspace lib/app than
> it is to update a driver on each installed system.

And, you're going to have to update the userspace lib/app to add the
new ioctl()-invocation to it anyhow.  So take the hit *only* there and
make it use either /sys or /proc, whichever is available, no kernel
changes.  Yes?

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

How to add/drop SCSI drives from within the driver?

[Bagalkote, Sreenivas]

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

Hello All,

I am trying to implement a feature in my SCSI driver, where it can
trigger the SCSI mid-layer to scan or remove a particular drive. I 
appreciate any help in nudging me in the right direction.

The exported functions - 

scsi_add_device( host, channel, target, lun )
scsi_remove_device( struct scsi_device* )

seem to work well in my limited testing. Are there any caveats in this
method? Does this method work well without exceptions? I am inlining
the full patch for megaraid SCSI driver taken against 2.6.10-rc2. I am
also attaching it to this mail.

Thank you,
Sreenivas


----
diff -Naur old-rc3/drivers/scsi/megaraid/mega_common.h
new-rc2/drivers/scsi/megaraid/mega_common.h
--- old-rc2/drivers/scsi/megaraid/mega_common.h	2004-10-18
17:54:31.000000000 -0400
+++ new-rc2/drivers/scsi/megaraid/mega_common.h	2004-12-02
20:23:35.000000000 -0500
@@ -242,6 +242,15 @@
 					[SCP2TARGET(scp)] & 0xFF);	\
 	}
 
+/**
+ * MRAID_LD_TARGET
+ * @param adp		- Adapter's soft state
+ * @param ld		- Logical drive number
+ *
+ * Macro to retrieve the SCSI target id of a logical drive
+ */
+#define MRAID_LD_TARGET(adp, ld) (((ld) < (adp)->init_id) ? (ld) : (ld)+1)

+
 /*
  * ### Helper routines ###
  */
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
--- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-02
20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-02
20:23:25.000000000 -0500
@@ -51,8 +51,11 @@
 #define MEGAIOC_QNADAP		'm'	/* Query # of adapters		*/
 #define MEGAIOC_QDRVRVER	'e'	/* Query driver version		*/
 #define MEGAIOC_QADAPINFO   	'g'	/* Query adapter information	*/
+#define MEGAIOC_ADD_LD		'a'
+#define MEGAIOC_DEL_LD		'r'
 
 #define USCSICMD		0x80
+#define UIOC_NONE		0x00000
 #define UIOC_RD			0x00001
 #define UIOC_WR			0x00002
 
@@ -62,6 +65,8 @@
 #define GET_ADAP_INFO		0x30000
 #define GET_CAP			0x40000
 #define GET_STATS		0x50000
+#define	ADD_LD			0x60000
+#define DEL_LD			0x70000
 #define GET_IOCTL_VERSION	0x01
 
 #define EXT_IOCTL_SIGN_SZ	16
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.c
new-rc2/drivers/scsi/megaraid/megaraid_mbox.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-02
20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-02
20:32:33.408278216 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mbox.c
- * Version	: v2.20.4.1 (Nov 04 2004)
+ * Version	: v2.20.4.1+ TEST VERSION
  *
  * Authors:
  * 	Atul Mukker		<Atul.Mukker@lsil.com>
@@ -3642,6 +3642,10 @@
 megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, uint32_t
action)
 {
 	adapter_t *adapter;
+	uint32_t ld;
+	struct scsi_device* sdev;
+	int ch;
+	int tg;
 
 	if (action != IOCTL_ISSUE) {
 		con_log(CL_ANN, (KERN_WARNING
@@ -3670,6 +3674,31 @@
 
 		return kioc->status;
 
+	case ADD_LD:
+		ld = *(uint32_t*) kioc->buf_vaddr;
+		ch = adapter->max_channel;
+		tg = MRAID_LD_TARGET( adapter, ld );
+		scsi_add_device(adapter->host, ch, tg, 0);
+
+		kioc->status = 0;
+		kioc->done(kioc);
+		return kioc->status;
+
+	case DEL_LD:
+		ld = *(uint32_t*) kioc->buf_vaddr;
+		ch = adapter->max_channel;
+		tg = MRAID_LD_TARGET( adapter, ld );
+		sdev = scsi_device_lookup( adapter->host, ch, tg, 0);
+		
+		if( sdev ) {
+			scsi_remove_device( sdev );
+			scsi_device_put( sdev );
+		}
+
+		kioc->status = 0;
+		kioc->done(kioc);
+		return kioc->status;
+
 	case MBOX_CMD:
 
 		return megaraid_mbox_mm_command(adapter, kioc);
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.h
new-rc2/drivers/scsi/megaraid/megaraid_mbox.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-02
20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-02
20:32:09.737876664 -0500
@@ -21,8 +21,8 @@
 #include "megaraid_ioctl.h"
 
 
-#define MEGARAID_VERSION	"2.20.4.1"
-#define MEGARAID_EXT_VERSION	"(Release Date: Thu Nov  4 17:44:59 EST
2004)"
+#define MEGARAID_VERSION	"2.20.4.1+ TEST VERSION"
+#define MEGARAID_EXT_VERSION	"(Release Date: TEST VERSION)"
 
 
 /*
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.c
new-rc2/drivers/scsi/megaraid/megaraid_mm.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-02
20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-02
20:22:57.000000000 -0500
@@ -373,6 +373,34 @@
 			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
 				return (-ENOMEM);
 		}
+		else if (subopcode == MEGAIOC_ADD_LD) {
+
+			kioc->opcode	= ADD_LD;
+			kioc->data_dir	= UIOC_NONE;
+			kioc->xferlen	= sizeof(uint32_t);
+
+			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
+				return -(ENOMEM);
+
+			if (copy_from_user(kioc->buf_vaddr, mimd.data,
+							kioc->xferlen)) {
+				return (-EFAULT);
+			}
+		}
+		else if (subopcode == MEGAIOC_DEL_LD) {
+
+			kioc->opcode	= DEL_LD;
+			kioc->data_dir	= UIOC_NONE;
+			kioc->xferlen	= sizeof(uint32_t);
+
+			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
+				return -(ENOMEM);
+
+			if (copy_from_user(kioc->buf_vaddr, mimd.data,
+							kioc->xferlen)) {
+				return (-EFAULT);
+			}
+		}
 		else {
 			con_log(CL_ANN, (KERN_WARNING
 					"megaraid cmm: Invalid subop\n"));
@@ -809,6 +837,9 @@
 
 			return 0;
 
+		case MEGAIOC_ADD_LD:
+		case MEGAIOC_DEL_LD:
+			return 0;
 		default:
 			return (-EINVAL);
 		}
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.h
new-rc2/drivers/scsi/megaraid/megaraid_mm.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-02
20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-02
20:33:00.709127856 -0500
@@ -29,9 +29,9 @@
 #include "megaraid_ioctl.h"
 
 
-#define LSI_COMMON_MOD_VERSION	"2.20.2.2"
+#define LSI_COMMON_MOD_VERSION	"2.20.2.2+ TEST VERSION"
 #define LSI_COMMON_MOD_EXT_VERSION	\
-		"(Release Date: Thu Nov  4 17:46:29 EST 2004)"
+		"(Release Date: TEST VERSION)"
 
 
 #define LSI_DBGLVL			dbglevel
----


[-- Attachment #2: megaraid-scan.patch --]
[-- Type: application/octet-stream, Size: 5536 bytes --]

diff -Naur old-rc2/drivers/scsi/megaraid/mega_common.h new-rc2/drivers/scsi/megaraid/mega_common.h
--- old-rc2/drivers/scsi/megaraid/mega_common.h	2004-10-18 17:54:31.000000000 -0400
+++ new-rc2/drivers/scsi/megaraid/mega_common.h	2004-12-02 20:23:35.000000000 -0500
@@ -242,6 +242,15 @@
 					[SCP2TARGET(scp)] & 0xFF);	\
 	}
 
+/**
+ * MRAID_LD_TARGET
+ * @param adp		- Adapter's soft state
+ * @param ld		- Logical drive number
+ *
+ * Macro to retrieve the SCSI target id of a logical drive
+ */
+#define MRAID_LD_TARGET(adp, ld) (((ld) < (adp)->init_id) ? (ld) : (ld)+1)	
+
 /*
  * ### Helper routines ###
  */
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
--- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-02 20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-02 20:23:25.000000000 -0500
@@ -51,8 +51,11 @@
 #define MEGAIOC_QNADAP		'm'	/* Query # of adapters		*/
 #define MEGAIOC_QDRVRVER	'e'	/* Query driver version		*/
 #define MEGAIOC_QADAPINFO   	'g'	/* Query adapter information	*/
+#define MEGAIOC_ADD_LD		'a'
+#define MEGAIOC_DEL_LD		'r'
 
 #define USCSICMD		0x80
+#define UIOC_NONE		0x00000
 #define UIOC_RD			0x00001
 #define UIOC_WR			0x00002
 
@@ -62,6 +65,8 @@
 #define GET_ADAP_INFO		0x30000
 #define GET_CAP			0x40000
 #define GET_STATS		0x50000
+#define	ADD_LD			0x60000
+#define DEL_LD			0x70000
 #define GET_IOCTL_VERSION	0x01
 
 #define EXT_IOCTL_SIGN_SZ	16
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.c new-rc2/drivers/scsi/megaraid/megaraid_mbox.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-02 20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-02 20:32:33.408278216 -0500
@@ -10,7 +10,7 @@
  *	   2 of the License, or (at your option) any later version.
  *
  * FILE		: megaraid_mbox.c
- * Version	: v2.20.4.1 (Nov 04 2004)
+ * Version	: v2.20.4.1+ TEST VERSION
  *
  * Authors:
  * 	Atul Mukker		<Atul.Mukker@lsil.com>
@@ -3642,6 +3642,10 @@
 megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, uint32_t action)
 {
 	adapter_t *adapter;
+	uint32_t ld;
+	struct scsi_device* sdev;
+	int ch;
+	int tg;
 
 	if (action != IOCTL_ISSUE) {
 		con_log(CL_ANN, (KERN_WARNING
@@ -3670,6 +3674,31 @@
 
 		return kioc->status;
 
+	case ADD_LD:
+		ld = *(uint32_t*) kioc->buf_vaddr;
+		ch = adapter->max_channel;
+		tg = MRAID_LD_TARGET( adapter, ld );
+		scsi_add_device(adapter->host, ch, tg, 0);
+
+		kioc->status = 0;
+		kioc->done(kioc);
+		return kioc->status;
+
+	case DEL_LD:
+		ld = *(uint32_t*) kioc->buf_vaddr;
+		ch = adapter->max_channel;
+		tg = MRAID_LD_TARGET( adapter, ld );
+		sdev = scsi_device_lookup( adapter->host, ch, tg, 0);
+		
+		if( sdev ) {
+			scsi_remove_device( sdev );
+			scsi_device_put( sdev );
+		}
+
+		kioc->status = 0;
+		kioc->done(kioc);
+		return kioc->status;
+
 	case MBOX_CMD:
 
 		return megaraid_mbox_mm_command(adapter, kioc);
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.h new-rc2/drivers/scsi/megaraid/megaraid_mbox.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-02 20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-02 20:32:09.737876664 -0500
@@ -21,8 +21,8 @@
 #include "megaraid_ioctl.h"
 
 
-#define MEGARAID_VERSION	"2.20.4.1"
-#define MEGARAID_EXT_VERSION	"(Release Date: Thu Nov  4 17:44:59 EST 2004)"
+#define MEGARAID_VERSION	"2.20.4.1+ TEST VERSION"
+#define MEGARAID_EXT_VERSION	"(Release Date: TEST VERSION)"
 
 
 /*
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.c new-rc2/drivers/scsi/megaraid/megaraid_mm.c
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-02 20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-02 20:22:57.000000000 -0500
@@ -373,6 +373,34 @@
 			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
 				return (-ENOMEM);
 		}
+		else if (subopcode == MEGAIOC_ADD_LD) {
+
+			kioc->opcode	= ADD_LD;
+			kioc->data_dir	= UIOC_NONE;
+			kioc->xferlen	= sizeof(uint32_t);
+
+			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
+				return -(ENOMEM);
+
+			if (copy_from_user(kioc->buf_vaddr, mimd.data,
+							kioc->xferlen)) {
+				return (-EFAULT);
+			}
+		}
+		else if (subopcode == MEGAIOC_DEL_LD) {
+
+			kioc->opcode	= DEL_LD;
+			kioc->data_dir	= UIOC_NONE;
+			kioc->xferlen	= sizeof(uint32_t);
+
+			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
+				return -(ENOMEM);
+
+			if (copy_from_user(kioc->buf_vaddr, mimd.data,
+							kioc->xferlen)) {
+				return (-EFAULT);
+			}
+		}
 		else {
 			con_log(CL_ANN, (KERN_WARNING
 					"megaraid cmm: Invalid subop\n"));
@@ -809,6 +837,9 @@
 
 			return 0;
 
+		case MEGAIOC_ADD_LD:
+		case MEGAIOC_DEL_LD:
+			return 0;
 		default:
 			return (-EINVAL);
 		}
diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.h new-rc2/drivers/scsi/megaraid/megaraid_mm.h
--- old-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-02 20:20:16.000000000 -0500
+++ new-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-02 20:33:00.709127856 -0500
@@ -29,9 +29,9 @@
 #include "megaraid_ioctl.h"
 
 
-#define LSI_COMMON_MOD_VERSION	"2.20.2.2"
+#define LSI_COMMON_MOD_VERSION	"2.20.2.2+ TEST VERSION"
 #define LSI_COMMON_MOD_EXT_VERSION	\
-		"(Release Date: Thu Nov  4 17:46:29 EST 2004)"
+		"(Release Date: TEST VERSION)"
 
 
 #define LSI_DBGLVL			dbglevel

Re: How to add/drop SCSI drives from within the driver?

[Brian King]

This looks to be adding an LLD specific interface to userspace to
add/delete disks. Why can't the existing sysfs interfaces be used
to to this? (scan attribute on host and delete attribute on device).

-Brian

Bagalkote, Sreenivas wrote:
> Hello All,
> 
> I am trying to implement a feature in my SCSI driver, where it can
> trigger the SCSI mid-layer to scan or remove a particular drive. I 
> appreciate any help in nudging me in the right direction.
> 
> The exported functions - 
> 
> scsi_add_device( host, channel, target, lun )
> scsi_remove_device( struct scsi_device* )
> 
> seem to work well in my limited testing. Are there any caveats in this
> method? Does this method work well without exceptions? I am inlining
> the full patch for megaraid SCSI driver taken against 2.6.10-rc2. I am
> also attaching it to this mail.
> 
> Thank you,
> Sreenivas
> 
> 
> ----
> diff -Naur old-rc3/drivers/scsi/megaraid/mega_common.h
> new-rc2/drivers/scsi/megaraid/mega_common.h
> --- old-rc2/drivers/scsi/megaraid/mega_common.h	2004-10-18
> 17:54:31.000000000 -0400
> +++ new-rc2/drivers/scsi/megaraid/mega_common.h	2004-12-02
> 20:23:35.000000000 -0500
> @@ -242,6 +242,15 @@
>  					[SCP2TARGET(scp)] & 0xFF);	\
>  	}
>  
> +/**
> + * MRAID_LD_TARGET
> + * @param adp		- Adapter's soft state
> + * @param ld		- Logical drive number
> + *
> + * Macro to retrieve the SCSI target id of a logical drive
> + */
> +#define MRAID_LD_TARGET(adp, ld) (((ld) < (adp)->init_id) ? (ld) : (ld)+1)
> 
> +
>  /*
>   * ### Helper routines ###
>   */
> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
> new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h
> --- old-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-02
> 20:20:16.000000000 -0500
> +++ new-rc2/drivers/scsi/megaraid/megaraid_ioctl.h	2004-12-02
> 20:23:25.000000000 -0500
> @@ -51,8 +51,11 @@
>  #define MEGAIOC_QNADAP		'm'	/* Query # of adapters		*/
>  #define MEGAIOC_QDRVRVER	'e'	/* Query driver version		*/
>  #define MEGAIOC_QADAPINFO   	'g'	/* Query adapter information	*/
> +#define MEGAIOC_ADD_LD		'a'
> +#define MEGAIOC_DEL_LD		'r'
>  
>  #define USCSICMD		0x80
> +#define UIOC_NONE		0x00000
>  #define UIOC_RD			0x00001
>  #define UIOC_WR			0x00002
>  
> @@ -62,6 +65,8 @@
>  #define GET_ADAP_INFO		0x30000
>  #define GET_CAP			0x40000
>  #define GET_STATS		0x50000
> +#define	ADD_LD			0x60000
> +#define DEL_LD			0x70000
>  #define GET_IOCTL_VERSION	0x01
>  
>  #define EXT_IOCTL_SIGN_SZ	16
> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.c
> new-rc2/drivers/scsi/megaraid/megaraid_mbox.c
> --- old-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-02
> 20:20:16.000000000 -0500
> +++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.c	2004-12-02
> 20:32:33.408278216 -0500
> @@ -10,7 +10,7 @@
>   *	   2 of the License, or (at your option) any later version.
>   *
>   * FILE		: megaraid_mbox.c
> - * Version	: v2.20.4.1 (Nov 04 2004)
> + * Version	: v2.20.4.1+ TEST VERSION
>   *
>   * Authors:
>   * 	Atul Mukker		<Atul.Mukker@lsil.com>
> @@ -3642,6 +3642,10 @@
>  megaraid_mbox_mm_handler(unsigned long drvr_data, uioc_t *kioc, uint32_t
> action)
>  {
>  	adapter_t *adapter;
> +	uint32_t ld;
> +	struct scsi_device* sdev;
> +	int ch;
> +	int tg;
>  
>  	if (action != IOCTL_ISSUE) {
>  		con_log(CL_ANN, (KERN_WARNING
> @@ -3670,6 +3674,31 @@
>  
>  		return kioc->status;
>  
> +	case ADD_LD:
> +		ld = *(uint32_t*) kioc->buf_vaddr;
> +		ch = adapter->max_channel;
> +		tg = MRAID_LD_TARGET( adapter, ld );
> +		scsi_add_device(adapter->host, ch, tg, 0);
> +
> +		kioc->status = 0;
> +		kioc->done(kioc);
> +		return kioc->status;
> +
> +	case DEL_LD:
> +		ld = *(uint32_t*) kioc->buf_vaddr;
> +		ch = adapter->max_channel;
> +		tg = MRAID_LD_TARGET( adapter, ld );
> +		sdev = scsi_device_lookup( adapter->host, ch, tg, 0);
> +		
> +		if( sdev ) {
> +			scsi_remove_device( sdev );
> +			scsi_device_put( sdev );
> +		}
> +
> +		kioc->status = 0;
> +		kioc->done(kioc);
> +		return kioc->status;
> +
>  	case MBOX_CMD:
>  
>  		return megaraid_mbox_mm_command(adapter, kioc);
> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mbox.h
> new-rc2/drivers/scsi/megaraid/megaraid_mbox.h
> --- old-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-02
> 20:20:16.000000000 -0500
> +++ new-rc2/drivers/scsi/megaraid/megaraid_mbox.h	2004-12-02
> 20:32:09.737876664 -0500
> @@ -21,8 +21,8 @@
>  #include "megaraid_ioctl.h"
>  
>  
> -#define MEGARAID_VERSION	"2.20.4.1"
> -#define MEGARAID_EXT_VERSION	"(Release Date: Thu Nov  4 17:44:59 EST
> 2004)"
> +#define MEGARAID_VERSION	"2.20.4.1+ TEST VERSION"
> +#define MEGARAID_EXT_VERSION	"(Release Date: TEST VERSION)"
>  
>  
>  /*
> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.c
> new-rc2/drivers/scsi/megaraid/megaraid_mm.c
> --- old-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-02
> 20:20:16.000000000 -0500
> +++ new-rc2/drivers/scsi/megaraid/megaraid_mm.c	2004-12-02
> 20:22:57.000000000 -0500
> @@ -373,6 +373,34 @@
>  			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
>  				return (-ENOMEM);
>  		}
> +		else if (subopcode == MEGAIOC_ADD_LD) {
> +
> +			kioc->opcode	= ADD_LD;
> +			kioc->data_dir	= UIOC_NONE;
> +			kioc->xferlen	= sizeof(uint32_t);
> +
> +			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
> +				return -(ENOMEM);
> +
> +			if (copy_from_user(kioc->buf_vaddr, mimd.data,
> +							kioc->xferlen)) {
> +				return (-EFAULT);
> +			}
> +		}
> +		else if (subopcode == MEGAIOC_DEL_LD) {
> +
> +			kioc->opcode	= DEL_LD;
> +			kioc->data_dir	= UIOC_NONE;
> +			kioc->xferlen	= sizeof(uint32_t);
> +
> +			if (mraid_mm_attach_buf(adp, kioc, kioc->xferlen))
> +				return -(ENOMEM);
> +
> +			if (copy_from_user(kioc->buf_vaddr, mimd.data,
> +							kioc->xferlen)) {
> +				return (-EFAULT);
> +			}
> +		}
>  		else {
>  			con_log(CL_ANN, (KERN_WARNING
>  					"megaraid cmm: Invalid subop\n"));
> @@ -809,6 +837,9 @@
>  
>  			return 0;
>  
> +		case MEGAIOC_ADD_LD:
> +		case MEGAIOC_DEL_LD:
> +			return 0;
>  		default:
>  			return (-EINVAL);
>  		}
> diff -Naur old-rc2/drivers/scsi/megaraid/megaraid_mm.h
> new-rc2/drivers/scsi/megaraid/megaraid_mm.h
> --- old-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-02
> 20:20:16.000000000 -0500
> +++ new-rc2/drivers/scsi/megaraid/megaraid_mm.h	2004-12-02
> 20:33:00.709127856 -0500
> @@ -29,9 +29,9 @@
>  #include "megaraid_ioctl.h"
>  
>  
> -#define LSI_COMMON_MOD_VERSION	"2.20.2.2"
> +#define LSI_COMMON_MOD_VERSION	"2.20.2.2+ TEST VERSION"
>  #define LSI_COMMON_MOD_EXT_VERSION	\
> -		"(Release Date: Thu Nov  4 17:46:29 EST 2004)"
> +		"(Release Date: TEST VERSION)"
>  
>  
>  #define LSI_DBGLVL			dbglevel
> ----
> 

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center