linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: sekharan@us.ibm.com
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	dm-devel <dm-devel@redhat.com>
Subject: Re: [PATCH 7/7] scsi_dh: attach to hardware handler from dm-mpath
Date: Tue, 20 May 2008 14:41:42 +0200	[thread overview]
Message-ID: <4832C706.5040202@suse.de> (raw)
In-Reply-To: <1211221247.21974.382.camel@chandra-ubuntu>

Hi Chandra,

Chandra Seetharaman wrote:
> On Mon, 2008-05-19 at 12:21 +0200, Hannes Reinecke wrote:
>> Hi Chandra,
>>
[ .. ]
>> Yes, but this only verifies that the _handler_ exist, not that it is
>> attached to this sdev
> 
> Agreed. And it works properly with the original design.
> 
> Here is my thinking on how it would work with the new feature (ability
> to attach any device using dh_state) you added:
>  - User adds a new device
>  - User knows which scsi hardware handler their device can attach to,
>    lets say, "mydev".
>  - User attaches the device with scsi hardware handler (writing "mydev"
>    to "dh_state").
>    Device is now attached to "mydev"
>  - User updates his/her multipath.conf file with "mydev" for the
>    specific device.
>  - User invokes multipath, which checks for the existence of "mydev"
>    and allows the table insertion.
>  - When pg_init was required in the future, multipath calls "activate"
>    for "mydev" correctly.
> 
> Wouldn't this work ?
> 
> Note: In the absence of this patch (7/7) only.
> 
Yes, but only for the first time. After reboot the user would
have to do the reconfiguration again.

>>> Hardware handler module is attached to the device itself (thru
>>> try_module_get() in attach), so, the module will exist as long as the
>>> device exists.
>>>
>> Not quite. It's only attached automatically if the device map has
>> an entry for this module.
>> However, given this patchset we can now easily manually attach any
>> device to the device handler. Or at least try to do so.
>> It's then up to the device handler to refuse binding if none of
>> the necessary pages/information etc. was found.
> 
> Doesn't dh_state solve this issue ?
> 
In theory, yes.

>>> Hence, there is no need to attach it again from the multipath layer.
>>>
>> Yes, you do. The user might have it's own custom configuration file,
>> covering new/unknown/unhandled/testing devices, which out of necessity
>> are _not_ hardcoded in the device table of the device handler.
>> So multipath has to have a way of attaching device handler to those
>> devices, too.
> 
> dh_state allows the user to do this, in which case why do we need to do
> this in multipath layer ?
> 
Because then we (read: I) don't have to modify multipath-tools.
If we don't take this patch we'll have to modify multipath-tools
to check for the dh_state attribute explicitly and activate
the hardware handler directly via sysfs.

But this also means that existing multipath-tools programs won't
be able to correctly activate the hardware handler in
_some_ cases (ie those cases, where the hardware table for the
device_handler doesn't contain the device in question).
And I can just imagine the bugs I'll be getting ...
So if we decide to take that road we should kill the hardware
handler feature from the device-mapper multipath interface
altogether to notify the user that the programs won't work
anymore. Otherwise it's just heading for trouble.

So I'd rather keep the userland interface stable here and add
some code to the kernel.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
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

  reply	other threads:[~2008-05-20 12:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-14 14:43 [PATCH 7/7] scsi_dh: attach to hardware handler from dm-mpath Hannes Reinecke
2008-05-15  2:50 ` Chandra Seetharaman
2008-05-15  9:46   ` Hannes Reinecke
2008-05-16 18:53     ` Chandra Seetharaman
2008-05-19 10:21       ` Hannes Reinecke
2008-05-19 18:20         ` Chandra Seetharaman
2008-05-20 12:41           ` Hannes Reinecke [this message]
2008-05-20 18:52             ` Chandra Seetharaman
2008-05-21  6:18               ` Hannes Reinecke
2008-05-22  8:14                 ` James Bottomley
2008-05-23 11:40                   ` Hannes Reinecke
2008-05-23 14:06                     ` James Bottomley
2008-05-19 19:11         ` [dm-devel] Re: [PATCH 7/7] scsi_dh: attach to hardware handler fromdm-mpath Shyam_Iyer
2008-05-19 20:01           ` Chandra Seetharaman
  -- strict thread matches above, loose matches on Subject: below --
2008-05-20 14:05 [PATCH 7/7] scsi_dh: attach to hardware handler from dm-mpath Hannes Reinecke
2008-05-23  2:08 ` Chandra Seetharaman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4832C706.5040202@suse.de \
    --to=hare@suse.de \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=dm-devel@redhat.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sekharan@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).