[PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Moger, Babu]

This patch introduces the match function for rdac device handler. Without this,
sometimes handler attach fails during the device_add.  The match function was
introduced by this patch
http://www.spinics.net/lists/linux-scsi/msg54284.html

Signed-off-by: Babu Moger <babu.moger@netapp.com>
---

--- linux/drivers/scsi/device_handler/scsi_dh_rdac.c.orig	2011-10-31 11:25:44.000000000 -0500
+++ linux/drivers/scsi/device_handler/scsi_dh_rdac.c	2011-10-31 11:31:34.000000000 -0500
@@ -819,6 +819,21 @@ static const struct scsi_dh_devlist rdac
 	{NULL, NULL},
 };
 
+static bool rdac_match(struct scsi_device *sdev)
+{
+	int i;
+
+	for (i = 0; rdac_dev_list[i].vendor; i++) {
+		if (!strncmp(sdev->vendor, rdac_dev_list[i].vendor,
+			strlen(rdac_dev_list[i].vendor)) &&
+		    !strncmp(sdev->model, rdac_dev_list[i].model,
+			strlen(rdac_dev_list[i].model))) {
+			return true;
+		}
+	}
+	return false;
+}
+
 static int rdac_bus_attach(struct scsi_device *sdev);
 static void rdac_bus_detach(struct scsi_device *sdev);
 
@@ -831,6 +846,7 @@ static struct scsi_device_handler rdac_d
 	.attach = rdac_bus_attach,
 	.detach = rdac_bus_detach,
 	.activate = rdac_activate,
+	.match = rdac_match,
 };
 
 static int rdac_bus_attach(struct scsi_device *sdev)

Re: [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Hannes Reinecke]

On 11/01/2011 06:19 PM, Moger, Babu wrote:
> This patch introduces the match function for rdac device handler. Without this,
> sometimes handler attach fails during the device_add.  The match function was
> introduced by this patch
> http://www.spinics.net/lists/linux-scsi/msg54284.html
>
> Signed-off-by: Babu Moger<babu.moger@netapp.com>
> ---
>
> --- linux/drivers/scsi/device_handler/scsi_dh_rdac.c.orig	2011-10-31 11:25:44.000000000 -0500
> +++ linux/drivers/scsi/device_handler/scsi_dh_rdac.c	2011-10-31 11:31:34.000000000 -0500
> @@ -819,6 +819,21 @@ static const struct scsi_dh_devlist rdac
>   	{NULL, NULL},
>   };
>
> +static bool rdac_match(struct scsi_device *sdev)
> +{
> +	int i;
> +
> +	for (i = 0; rdac_dev_list[i].vendor; i++) {
> +		if (!strncmp(sdev->vendor, rdac_dev_list[i].vendor,
> +			strlen(rdac_dev_list[i].vendor))&&
> +		    !strncmp(sdev->model, rdac_dev_list[i].model,
> +			strlen(rdac_dev_list[i].model))) {
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
>   static int rdac_bus_attach(struct scsi_device *sdev);
>   static void rdac_bus_detach(struct scsi_device *sdev);
>
> @@ -831,6 +846,7 @@ static struct scsi_device_handler rdac_d
>   	.attach = rdac_bus_attach,
>   	.detach = rdac_bus_detach,
>   	.activate = rdac_activate,
> +	.match = rdac_match,
>   };
>
>   static int rdac_bus_attach(struct scsi_device *sdev)
>
As stated in the other mail, I guess we would need to have a check 
if the LUN is in ALUA mode.
And, btw, the _original_ intention was to allow vendor-specific 
device_handler to do some better probing, eg querying some 
vendor-specific VPD pages.
Especially for RDAC it would make far more sense to query the 
existence and format of one of the RDAC-specific VPD pages (eg 0xC2, 
0xC4, or 0xC8) and use that for matching.
Then you could do away with the vendor/model array altogether here 
and we wouldn't need to update the rdac handler every time a new 
array comes out or has been rebranded by some OEM.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

RE: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Moger, Babu]

> -----Original Message-----
> From: Hannes Reinecke [mailto:hare@suse.de]
> Sent: Wednesday, November 02, 2011 2:21 AM
> To: dm-devel@redhat.com
> Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
> function for rdac device handler
> 
> On 11/01/2011 06:19 PM, Moger, Babu wrote:
> > This patch introduces the match function for rdac device handler.
> Without this,
> > sometimes handler attach fails during the device_add.  The match
> function was
> > introduced by this patch
> > http://www.spinics.net/lists/linux-scsi/msg54284.html
> >
> > Signed-off-by: Babu Moger<babu.moger@netapp.com>
> > ---
> >
> > --- linux/drivers/scsi/device_handler/scsi_dh_rdac.c.orig	2011-10-31
> 11:25:44.000000000 -0500
> > +++ linux/drivers/scsi/device_handler/scsi_dh_rdac.c	2011-10-31
> 11:31:34.000000000 -0500
> > @@ -819,6 +819,21 @@ static const struct scsi_dh_devlist rdac
> >   	{NULL, NULL},
> >   };
> >
> > +static bool rdac_match(struct scsi_device *sdev)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; rdac_dev_list[i].vendor; i++) {
> > +		if (!strncmp(sdev->vendor, rdac_dev_list[i].vendor,
> > +			strlen(rdac_dev_list[i].vendor))&&
> > +		    !strncmp(sdev->model, rdac_dev_list[i].model,
> > +			strlen(rdac_dev_list[i].model))) {
> > +			return true;
> > +		}
> > +	}
> > +	return false;
> > +}
> > +
> >   static int rdac_bus_attach(struct scsi_device *sdev);
> >   static void rdac_bus_detach(struct scsi_device *sdev);
> >
> > @@ -831,6 +846,7 @@ static struct scsi_device_handler rdac_d
> >   	.attach = rdac_bus_attach,
> >   	.detach = rdac_bus_detach,
> >   	.activate = rdac_activate,
> > +	.match = rdac_match,
> >   };
> >
> >   static int rdac_bus_attach(struct scsi_device *sdev)
> >
> As stated in the other mail, I guess we would need to have a check
> if the LUN is in ALUA mode.
> And, btw, the _original_ intention was to allow vendor-specific
> device_handler to do some better probing, eg querying some
> vendor-specific VPD pages.
> Especially for RDAC it would make far more sense to query the
> existence and format of one of the RDAC-specific VPD pages (eg 0xC2,
> 0xC4, or 0xC8) and use that for matching.
> Then you could do away with the vendor/model array altogether here
> and we wouldn't need to update the rdac handler every time a new
> array comes out or has been rebranded by some OEM.
  
OK. I will add the check for TPGS. I will send the patches tomorrow.
For sending the VPD pages(0xC2, 0xC4 and 0xC8), I think we need be little careful here.
This includes sending these commands to every possible device in the system. That is what we want to avoid.
I will investigate more on that. That will be my next set of patches independent of this.

> 
> Cheers,
> 
> Hannes
> --
> Dr. Hannes Reinecke		      zSeries & Storage
> hare@suse.de			      +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
--
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

Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Hannes Reinecke]

On 11/02/2011 04:23 PM, Moger, Babu wrote:
>> -----Original Message-----
>> From: Hannes Reinecke [mailto:hare@suse.de]
>> Sent: Wednesday, November 02, 2011 2:21 AM
>> To: dm-devel@redhat.com
>> Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
>> function for rdac device handler
>>
>> On 11/01/2011 06:19 PM, Moger, Babu wrote:
>>> This patch introduces the match function for rdac device handler.
>> Without this,
>>> sometimes handler attach fails during the device_add.  The match
>> function was
>>> introduced by this patch
>>> http://www.spinics.net/lists/linux-scsi/msg54284.html
>>>
>>> Signed-off-by: Babu Moger<babu.moger@netapp.com>
>>> ---
>>>
>>> --- linux/drivers/scsi/device_handler/scsi_dh_rdac.c.orig	2011-10-31
>> 11:25:44.000000000 -0500
>>> +++ linux/drivers/scsi/device_handler/scsi_dh_rdac.c	2011-10-31
>> 11:31:34.000000000 -0500
>>> @@ -819,6 +819,21 @@ static const struct scsi_dh_devlist rdac
>>>    	{NULL, NULL},
>>>    };
>>>
>>> +static bool rdac_match(struct scsi_device *sdev)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; rdac_dev_list[i].vendor; i++) {
>>> +		if (!strncmp(sdev->vendor, rdac_dev_list[i].vendor,
>>> +			strlen(rdac_dev_list[i].vendor))&&
>>> +		    !strncmp(sdev->model, rdac_dev_list[i].model,
>>> +			strlen(rdac_dev_list[i].model))) {
>>> +			return true;
>>> +		}
>>> +	}
>>> +	return false;
>>> +}
>>> +
>>>    static int rdac_bus_attach(struct scsi_device *sdev);
>>>    static void rdac_bus_detach(struct scsi_device *sdev);
>>>
>>> @@ -831,6 +846,7 @@ static struct scsi_device_handler rdac_d
>>>    	.attach = rdac_bus_attach,
>>>    	.detach = rdac_bus_detach,
>>>    	.activate = rdac_activate,
>>> +	.match = rdac_match,
>>>    };
>>>
>>>    static int rdac_bus_attach(struct scsi_device *sdev)
>>>
>> As stated in the other mail, I guess we would need to have a check
>> if the LUN is in ALUA mode.
>> And, btw, the _original_ intention was to allow vendor-specific
>> device_handler to do some better probing, eg querying some
>> vendor-specific VPD pages.
>> Especially for RDAC it would make far more sense to query the
>> existence and format of one of the RDAC-specific VPD pages (eg 0xC2,
>> 0xC4, or 0xC8) and use that for matching.
>> Then you could do away with the vendor/model array altogether here
>> and we wouldn't need to update the rdac handler every time a new
>> array comes out or has been rebranded by some OEM.
>
> OK. I will add the check for TPGS. I will send the patches tomorrow.
> For sending the VPD pages(0xC2, 0xC4 and 0xC8), I think we need be
 > little careful here.
> This includes sending these commands to every possible device in the
 > system. That is what we want to avoid.
> I will investigate more on that. That will be my next set of patches
 > independent of this.
>
Fair enough.
As long as it's understood to be an interim solution, then we would 
only need to check for the TGPS bit.
Which has the neat side-effect that we don't actually have to do any 
I/O to check this, as the information is already present at that time.

While you're at it, could you please add this check for scsi_dh_emc, 
too?
The Clariion is also able to run in dual-mode, so the same check is 
required there, too.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, 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

RE: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Moger, Babu]

> -----Original Message-----
> From: Hannes Reinecke [mailto:hare@suse.de]
> Sent: Wednesday, November 02, 2011 10:34 AM
> To: device-mapper development
> Cc: Moger, Babu; Linux SCSI Mailing list
> Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
> function for rdac device handler
> 
> On 11/02/2011 04:23 PM, Moger, Babu wrote:
> >> -----Original Message-----
> >> From: Hannes Reinecke [mailto:hare@suse.de]
> >> Sent: Wednesday, November 02, 2011 2:21 AM
> >> To: dm-devel@redhat.com
> >> Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
> >> function for rdac device handler
> >>
> >> On 11/01/2011 06:19 PM, Moger, Babu wrote:
> >>> This patch introduces the match function for rdac device handler.
> >> Without this,
> >>> sometimes handler attach fails during the device_add.  The match
> >> function was
> >>> introduced by this patch
> >>> http://www.spinics.net/lists/linux-scsi/msg54284.html
> >>>
> >>> Signed-off-by: Babu Moger<babu.moger@netapp.com>
> >>> ---
> >>>
> >>> --- linux/drivers/scsi/device_handler/scsi_dh_rdac.c.orig	2011-
> 10-31
> >> 11:25:44.000000000 -0500
> >>> +++ linux/drivers/scsi/device_handler/scsi_dh_rdac.c	2011-10-31
> >> 11:31:34.000000000 -0500
> >>> @@ -819,6 +819,21 @@ static const struct scsi_dh_devlist rdac
> >>>    	{NULL, NULL},
> >>>    };
> >>>
> >>> +static bool rdac_match(struct scsi_device *sdev)
> >>> +{
> >>> +	int i;
> >>> +
> >>> +	for (i = 0; rdac_dev_list[i].vendor; i++) {
> >>> +		if (!strncmp(sdev->vendor, rdac_dev_list[i].vendor,
> >>> +			strlen(rdac_dev_list[i].vendor))&&
> >>> +		    !strncmp(sdev->model, rdac_dev_list[i].model,
> >>> +			strlen(rdac_dev_list[i].model))) {
> >>> +			return true;
> >>> +		}
> >>> +	}
> >>> +	return false;
> >>> +}
> >>> +
> >>>    static int rdac_bus_attach(struct scsi_device *sdev);
> >>>    static void rdac_bus_detach(struct scsi_device *sdev);
> >>>
> >>> @@ -831,6 +846,7 @@ static struct scsi_device_handler rdac_d
> >>>    	.attach = rdac_bus_attach,
> >>>    	.detach = rdac_bus_detach,
> >>>    	.activate = rdac_activate,
> >>> +	.match = rdac_match,
> >>>    };
> >>>
> >>>    static int rdac_bus_attach(struct scsi_device *sdev)
> >>>
> >> As stated in the other mail, I guess we would need to have a check
> >> if the LUN is in ALUA mode.
> >> And, btw, the _original_ intention was to allow vendor-specific
> >> device_handler to do some better probing, eg querying some
> >> vendor-specific VPD pages.
> >> Especially for RDAC it would make far more sense to query the
> >> existence and format of one of the RDAC-specific VPD pages (eg 0xC2,
> >> 0xC4, or 0xC8) and use that for matching.
> >> Then you could do away with the vendor/model array altogether here
> >> and we wouldn't need to update the rdac handler every time a new
> >> array comes out or has been rebranded by some OEM.
> >
> > OK. I will add the check for TPGS. I will send the patches tomorrow.
> > For sending the VPD pages(0xC2, 0xC4 and 0xC8), I think we need be
>  > little careful here.
> > This includes sending these commands to every possible device in the
>  > system. That is what we want to avoid.
> > I will investigate more on that. That will be my next set of patches
>  > independent of this.
> >
> Fair enough.
> As long as it's understood to be an interim solution, then we would
> only need to check for the TGPS bit.
> Which has the neat side-effect that we don't actually have to do any
> I/O to check this, as the information is already present at that time.
> 
> While you're at it, could you please add this check for scsi_dh_emc,
> too?
> The Clariion is also able to run in dual-mode, so the same check is
> required there, too.

Sure..  Will add the check to Clariion also..  

> 
> Cheers,
> 
> Hannes
> --
> Dr. Hannes Reinecke		      zSeries & Storage
> hare@suse.de			      +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, 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

Re: [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Mike Snitzer]

On Wed, Nov 02 2011 at 11:23am -0400,
Moger, Babu <Babu.Moger@netapp.com> wrote:

> > -----Original Message-----
> > From: Hannes Reinecke [mailto:hare@suse.de]
> > Sent: Wednesday, November 02, 2011 2:21 AM
> > To: dm-devel@redhat.com
> > Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
> > function for rdac device handler
> > 
> > On 11/01/2011 06:19 PM, Moger, Babu wrote:
> > > This patch introduces the match function for rdac device handler.
> > Without this,
> > > sometimes handler attach fails during the device_add.  The match
> > function was
> > > introduced by this patch
> > > http://www.spinics.net/lists/linux-scsi/msg54284.html
> > >
> > > Signed-off-by: Babu Moger<babu.moger@netapp.com>
> > > ---
> > >
> > > --- linux/drivers/scsi/device_handler/scsi_dh_rdac.c.orig	2011-10-31
> > 11:25:44.000000000 -0500
> > > +++ linux/drivers/scsi/device_handler/scsi_dh_rdac.c	2011-10-31
> > 11:31:34.000000000 -0500
> > > @@ -819,6 +819,21 @@ static const struct scsi_dh_devlist rdac
> > >   	{NULL, NULL},
> > >   };
> > >
> > > +static bool rdac_match(struct scsi_device *sdev)
> > > +{
> > > +	int i;
> > > +
> > > +	for (i = 0; rdac_dev_list[i].vendor; i++) {
> > > +		if (!strncmp(sdev->vendor, rdac_dev_list[i].vendor,
> > > +			strlen(rdac_dev_list[i].vendor))&&
> > > +		    !strncmp(sdev->model, rdac_dev_list[i].model,
> > > +			strlen(rdac_dev_list[i].model))) {
> > > +			return true;
> > > +		}
> > > +	}
> > > +	return false;
> > > +}
> > > +
> > >   static int rdac_bus_attach(struct scsi_device *sdev);
> > >   static void rdac_bus_detach(struct scsi_device *sdev);
> > >
> > > @@ -831,6 +846,7 @@ static struct scsi_device_handler rdac_d
> > >   	.attach = rdac_bus_attach,
> > >   	.detach = rdac_bus_detach,
> > >   	.activate = rdac_activate,
> > > +	.match = rdac_match,
> > >   };
> > >
> > >   static int rdac_bus_attach(struct scsi_device *sdev)
> > >
> > As stated in the other mail, I guess we would need to have a check
> > if the LUN is in ALUA mode.
> > And, btw, the _original_ intention was to allow vendor-specific
> > device_handler to do some better probing, eg querying some
> > vendor-specific VPD pages.
> > Especially for RDAC it would make far more sense to query the
> > existence and format of one of the RDAC-specific VPD pages (eg 0xC2,
> > 0xC4, or 0xC8) and use that for matching.
> > Then you could do away with the vendor/model array altogether here
> > and we wouldn't need to update the rdac handler every time a new
> > array comes out or has been rebranded by some OEM.
>   
> OK. I will add the check for TPGS. I will send the patches tomorrow.
> For sending the VPD pages(0xC2, 0xC4 and 0xC8), I think we need be little careful here.
> This includes sending these commands to every possible device in the system. That is what we want to avoid.
> I will investigate more on that. That will be my next set of patches independent of this.

Much appreciated.  I agree with Hannes, ideally we wouldn't need the
rdac dev_list.

What about the issue where the appropriate scsi_dh isn't attached during
scan (resulting in boot failures, trespasses, etc)?

Hannes, I know you had plans for how to address the early scsi_dh
attachment (and this match() work is a great step forward).  I just
wanted to touch base with you on what your current vision is on how to
achieve proper early scsi_dh attachment (and what the remaining TODO
is).

Thanks,
Mike

RE: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Moger, Babu]

> -----Original Message-----
> From: Mike Snitzer [mailto:snitzer@redhat.com]
> Sent: Wednesday, November 02, 2011 10:46 AM
> To: device-mapper development
> Cc: Linux SCSI Mailing list
> Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
> function for rdac device handler
> 
> On Wed, Nov 02 2011 at 11:23am -0400,
> Moger, Babu <Babu.Moger@netapp.com> wrote:
> 
> > > -----Original Message-----
> > > From: Hannes Reinecke [mailto:hare@suse.de]
> > > Sent: Wednesday, November 02, 2011 2:21 AM
> > > To: dm-devel@redhat.com
> > > Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
> > > function for rdac device handler
> > >
> > > On 11/01/2011 06:19 PM, Moger, Babu wrote:
> > > > This patch introduces the match function for rdac device handler.
> > > Without this,
> > > > sometimes handler attach fails during the device_add.  The match
> > > function was
> > > > introduced by this patch
> > > > http://www.spinics.net/lists/linux-scsi/msg54284.html
> > > >
> > > > Signed-off-by: Babu Moger<babu.moger@netapp.com>
> > > > ---
> > > >
> > > > --- linux/drivers/scsi/device_handler/scsi_dh_rdac.c.orig	2011-
> 10-31
> > > 11:25:44.000000000 -0500
> > > > +++ linux/drivers/scsi/device_handler/scsi_dh_rdac.c	2011-10-31
> > > 11:31:34.000000000 -0500
> > > > @@ -819,6 +819,21 @@ static const struct scsi_dh_devlist rdac
> > > >   	{NULL, NULL},
> > > >   };
> > > >
> > > > +static bool rdac_match(struct scsi_device *sdev)
> > > > +{
> > > > +	int i;
> > > > +
> > > > +	for (i = 0; rdac_dev_list[i].vendor; i++) {
> > > > +		if (!strncmp(sdev->vendor, rdac_dev_list[i].vendor,
> > > > +			strlen(rdac_dev_list[i].vendor))&&
> > > > +		    !strncmp(sdev->model, rdac_dev_list[i].model,
> > > > +			strlen(rdac_dev_list[i].model))) {
> > > > +			return true;
> > > > +		}
> > > > +	}
> > > > +	return false;
> > > > +}
> > > > +
> > > >   static int rdac_bus_attach(struct scsi_device *sdev);
> > > >   static void rdac_bus_detach(struct scsi_device *sdev);
> > > >
> > > > @@ -831,6 +846,7 @@ static struct scsi_device_handler rdac_d
> > > >   	.attach = rdac_bus_attach,
> > > >   	.detach = rdac_bus_detach,
> > > >   	.activate = rdac_activate,
> > > > +	.match = rdac_match,
> > > >   };
> > > >
> > > >   static int rdac_bus_attach(struct scsi_device *sdev)
> > > >
> > > As stated in the other mail, I guess we would need to have a check
> > > if the LUN is in ALUA mode.
> > > And, btw, the _original_ intention was to allow vendor-specific
> > > device_handler to do some better probing, eg querying some
> > > vendor-specific VPD pages.
> > > Especially for RDAC it would make far more sense to query the
> > > existence and format of one of the RDAC-specific VPD pages (eg
> 0xC2,
> > > 0xC4, or 0xC8) and use that for matching.
> > > Then you could do away with the vendor/model array altogether here
> > > and we wouldn't need to update the rdac handler every time a new
> > > array comes out or has been rebranded by some OEM.
> >
> > OK. I will add the check for TPGS. I will send the patches tomorrow.
> > For sending the VPD pages(0xC2, 0xC4 and 0xC8), I think we need be
> little careful here.
> > This includes sending these commands to every possible device in the
> system. That is what we want to avoid.
> > I will investigate more on that. That will be my next set of patches
> independent of this.
> 
> Much appreciated.  I agree with Hannes, ideally we wouldn't need the
> rdac dev_list.

Yes, We would like to remove the dependency on Vendor/product strings.
I will work on that. These current patches will address the current the
Attach issue which I mentioned in the description(PATCH 0/4).
I will resubmit the patches now..

> 
> What about the issue where the appropriate scsi_dh isn't attached
> during
> scan (resulting in boot failures, trespasses, etc)?
> 
> Hannes, I know you had plans for how to address the early scsi_dh
> attachment (and this match() work is a great step forward).  I just
> wanted to touch base with you on what your current vision is on how to
> achieve proper early scsi_dh attachment (and what the remaining TODO
> is).

I am not aware of any other issue at this point. Hannes may know about it.

> 
> Thanks,
> Mike
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Mike Snitzer]

On Thu, Nov 03 2011 at 10:47am -0400,
Moger, Babu <Babu.Moger@netapp.com> wrote:

> > -----Original Message-----
> > From: Mike Snitzer [mailto:snitzer@redhat.com]
> > Sent: Wednesday, November 02, 2011 10:46 AM
> > To: device-mapper development
> > Cc: Linux SCSI Mailing list
> > Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
> > function for rdac device handler
> > 
> > On Wed, Nov 02 2011 at 11:23am -0400,
> > Moger, Babu <Babu.Moger@netapp.com> wrote:
> > >
> > > OK. I will add the check for TPGS. I will send the patches tomorrow.
> > > For sending the VPD pages(0xC2, 0xC4 and 0xC8), I think we need be
> > little careful here.
> > > This includes sending these commands to every possible device in the
> > system. That is what we want to avoid.
> > > I will investigate more on that. That will be my next set of patches
> > independent of this.
> > 
> > Much appreciated.  I agree with Hannes, ideally we wouldn't need the
> > rdac dev_list.
> 
> Yes, We would like to remove the dependency on Vendor/product strings.
> I will work on that. These current patches will address the current the
> Attach issue which I mentioned in the description(PATCH 0/4).
> I will resubmit the patches now..

Great.

> > What about the issue where the appropriate scsi_dh isn't attached
> > during
> > scan (resulting in boot failures, trespasses, etc)?
> > 
> > Hannes, I know you had plans for how to address the early scsi_dh
> > attachment (and this match() work is a great step forward).  I just
> > wanted to touch base with you on what your current vision is on how to
> > achieve proper early scsi_dh attachment (and what the remaining TODO
> > is).
> 
> I am not aware of any other issue at this point. Hannes may know about it.

Yeap Hannes is aware.

I was referring to IO being issued to passive paths (ghost LUNs) because
scsi_dh isn't yet loaded.  Whereby causing the storage backend to
trespass unnecessarily.  This bouncing (and corresponding IO errors) are
avoided if the appropriate scsi_dh module is always loaded before the
storage driver (e.g. lpfc or qla2xxx).

Re: [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Mike Snitzer]

On Thu, Nov 03 2011 at 11:17am -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Thu, Nov 03 2011 at 10:47am -0400,
> Moger, Babu <Babu.Moger@netapp.com> wrote:
> 
> > > -----Original Message-----
> > > From: Mike Snitzer [mailto:snitzer@redhat.com]
> > > Sent: Wednesday, November 02, 2011 10:46 AM
> > > To: device-mapper development
> > > Cc: Linux SCSI Mailing list
> > > Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the match
> > > function for rdac device handler

...

> > > What about the issue where the appropriate scsi_dh isn't attached
> > > during
> > > scan (resulting in boot failures, trespasses, etc)?
> > > 
> > > Hannes, I know you had plans for how to address the early scsi_dh
> > > attachment (and this match() work is a great step forward).  I just
> > > wanted to touch base with you on what your current vision is on how to
> > > achieve proper early scsi_dh attachment (and what the remaining TODO
> > > is).
> > 
> > I am not aware of any other issue at this point. Hannes may know about it.
> 
> Yeap Hannes is aware.
> 
> I was referring to IO being issued to passive paths (ghost LUNs) because
> scsi_dh isn't yet loaded.  Whereby causing the storage backend to
> trespass unnecessarily.  This bouncing (and corresponding IO errors) are
> avoided if the appropriate scsi_dh module is always loaded before the
> storage driver (e.g. lpfc or qla2xxx).

I have reviewed the scsi_dh match() changes (those from Hannes that are
already upstream and the 4 patches from Babu to complete match() for
other device handlers and the scsi_dh cleanup).

Hannes, in your cover-letter from the original scsi_dh_alua match
patchset, here: http://www.spinics.net/lists/linux-scsi/msg54281.html,
you said:

"In contrast to what we've discussed at LinuxTag I have not tried
to attach the alua device handler directly from scsi_scan.
Reason is that I need to issue SCSI commands during activation,
which means I would have to attach it from near the end of
scsi_add_lun(), at which point the device_handler would be attached
via the current method anyway. So I fail to see the gain here."

I haven't picked through the scsi_dh/scsi code enough to know what "the
current method" is (but I'm reviewing the code now).  That said, a quick
recap of what you feel the relevant highlights are would be appreciated.

But I thought the issue we discussed at LinuxTag was: how do we autoload
the scsi_dh module(s) so that the device handler is even available for
attachment?


Babu, you said that your patchset to implement match() for rdac resolved
the problem of the device handler not attaching properly:
http://www.redhat.com/archives/dm-devel/2011-November/msg00032.html

But that is only the case if scsi_dh_rdac has already been loaded early
by the initramfs right?

Given the updated scsi_dh match code: should it be safe for the
initramfs to just load all scsi_dh modules (and ALUA will be preferred
if TPGS is set)?

Does it make sense to re-visit Peter Jones' modalias code to autoload
scsi_dh in kernel rather than relying on adhoc initramfs code to know to
load the modules?

Mike

RE: [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Moger, Babu]

> -----Original Message-----
> From: Mike Snitzer [mailto:snitzer@redhat.com]
> Sent: Wednesday, November 16, 2011 11:33 AM
> To: Moger, Babu; hare@suse.de
> Cc: Linux SCSI Mailing list; device-mapper development; Peter Jones
> Subject: Re: [PATCH 3/4] scsi_dh_rdac: Adding the match function for
> rdac device handler
> 
> On Thu, Nov 03 2011 at 11:17am -0400,
> Mike Snitzer <snitzer@redhat.com> wrote:
> 
> > On Thu, Nov 03 2011 at 10:47am -0400,
> > Moger, Babu <Babu.Moger@netapp.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Mike Snitzer [mailto:snitzer@redhat.com]
> > > > Sent: Wednesday, November 02, 2011 10:46 AM
> > > > To: device-mapper development
> > > > Cc: Linux SCSI Mailing list
> > > > Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the
> match
> > > > function for rdac device handler
> 
> ...
> 
> > > > What about the issue where the appropriate scsi_dh isn't attached
> > > > during
> > > > scan (resulting in boot failures, trespasses, etc)?
> > > >
> > > > Hannes, I know you had plans for how to address the early scsi_dh
> > > > attachment (and this match() work is a great step forward).  I
> just
> > > > wanted to touch base with you on what your current vision is on
> how to
> > > > achieve proper early scsi_dh attachment (and what the remaining
> TODO
> > > > is).
> > >
> > > I am not aware of any other issue at this point. Hannes may know
> about it.
> >
> > Yeap Hannes is aware.
> >
> > I was referring to IO being issued to passive paths (ghost LUNs)
> because
> > scsi_dh isn't yet loaded.  Whereby causing the storage backend to
> > trespass unnecessarily.  This bouncing (and corresponding IO errors)
> are
> > avoided if the appropriate scsi_dh module is always loaded before the
> > storage driver (e.g. lpfc or qla2xxx).
> 
> I have reviewed the scsi_dh match() changes (those from Hannes that are
> already upstream and the 4 patches from Babu to complete match() for
> other device handlers and the scsi_dh cleanup).
> 
> Hannes, in your cover-letter from the original scsi_dh_alua match
> patchset, here: http://www.spinics.net/lists/linux-scsi/msg54281.html,
> you said:
> 
> "In contrast to what we've discussed at LinuxTag I have not tried
> to attach the alua device handler directly from scsi_scan.
> Reason is that I need to issue SCSI commands during activation,
> which means I would have to attach it from near the end of
> scsi_add_lun(), at which point the device_handler would be attached
> via the current method anyway. So I fail to see the gain here."
> 
> I haven't picked through the scsi_dh/scsi code enough to know what "the
> current method" is (but I'm reviewing the code now).  That said, a
> quick
> recap of what you feel the relevant highlights are would be
> appreciated.
> 
> But I thought the issue we discussed at LinuxTag was: how do we
> autoload
> the scsi_dh module(s) so that the device handler is even available for
> attachment?
> 
> 
> Babu, you said that your patchset to implement match() for rdac
> resolved
> the problem of the device handler not attaching properly:
> http://www.redhat.com/archives/dm-devel/2011-November/msg00032.html
> 
> But that is only the case if scsi_dh_rdac has already been loaded early
> by the initramfs right?

That is correct. I had included  the handler in initramfs.

> 
> Given the updated scsi_dh match code: should it be safe for the
> initramfs to just load all scsi_dh modules (and ALUA will be preferred
> if TPGS is set)?

Yes, it would be great..

> 
> Does it make sense to re-visit Peter Jones' modalias code to autoload
> scsi_dh in kernel rather than relying on adhoc initramfs code to know
> to
> load the modules?

I don’t have complete understanding that. Can't comment.

> 
> Mike
--
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

Re: [PATCH 3/4] scsi_dh_rdac: Adding the match function for rdac device handler

[Mike Snitzer]

On Wed, Nov 16 2011 at  5:10pm -0500,
Moger, Babu <Babu.Moger@netapp.com> wrote:

> > -----Original Message-----
> > From: Mike Snitzer [mailto:snitzer@redhat.com]
> > Sent: Wednesday, November 16, 2011 11:33 AM
> > To: Moger, Babu; hare@suse.de
> > Cc: Linux SCSI Mailing list; device-mapper development; Peter Jones
> > Subject: Re: [PATCH 3/4] scsi_dh_rdac: Adding the match function for
> > rdac device handler
> > 
> > On Thu, Nov 03 2011 at 11:17am -0400,
> > Mike Snitzer <snitzer@redhat.com> wrote:
> > 
> > > On Thu, Nov 03 2011 at 10:47am -0400,
> > > Moger, Babu <Babu.Moger@netapp.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Mike Snitzer [mailto:snitzer@redhat.com]
> > > > > Sent: Wednesday, November 02, 2011 10:46 AM
> > > > > To: device-mapper development
> > > > > Cc: Linux SCSI Mailing list
> > > > > Subject: Re: [dm-devel] [PATCH 3/4] scsi_dh_rdac: Adding the
> > match
> > > > > function for rdac device handler
> > 
> > ...
> > 
> > > > > What about the issue where the appropriate scsi_dh isn't attached
> > > > > during
> > > > > scan (resulting in boot failures, trespasses, etc)?
> > > > >
> > > > > Hannes, I know you had plans for how to address the early scsi_dh
> > > > > attachment (and this match() work is a great step forward).  I
> > just
> > > > > wanted to touch base with you on what your current vision is on
> > how to
> > > > > achieve proper early scsi_dh attachment (and what the remaining
> > TODO
> > > > > is).
> > > >
> > > > I am not aware of any other issue at this point. Hannes may know
> > about it.
> > >
> > > Yeap Hannes is aware.
> > >
> > > I was referring to IO being issued to passive paths (ghost LUNs)
> > because
> > > scsi_dh isn't yet loaded.  Whereby causing the storage backend to
> > > trespass unnecessarily.  This bouncing (and corresponding IO errors)
> > are
> > > avoided if the appropriate scsi_dh module is always loaded before the
> > > storage driver (e.g. lpfc or qla2xxx).
> > 
> > I have reviewed the scsi_dh match() changes (those from Hannes that are
> > already upstream and the 4 patches from Babu to complete match() for
> > other device handlers and the scsi_dh cleanup).
> > 
> > Hannes, in your cover-letter from the original scsi_dh_alua match
> > patchset, here: http://www.spinics.net/lists/linux-scsi/msg54281.html,
> > you said:
> > 
> > "In contrast to what we've discussed at LinuxTag I have not tried
> > to attach the alua device handler directly from scsi_scan.
> > Reason is that I need to issue SCSI commands during activation,
> > which means I would have to attach it from near the end of
> > scsi_add_lun(), at which point the device_handler would be attached
> > via the current method anyway. So I fail to see the gain here."
> > 
> > I haven't picked through the scsi_dh/scsi code enough to know what "the
> > current method" is (but I'm reviewing the code now).  That said, a
> > quick
> > recap of what you feel the relevant highlights are would be
> > appreciated.
> > 
> > But I thought the issue we discussed at LinuxTag was: how do we
> > autoload
> > the scsi_dh module(s) so that the device handler is even available for
> > attachment?
> > 
> > 
> > Babu, you said that your patchset to implement match() for rdac
> > resolved
> > the problem of the device handler not attaching properly:
> > http://www.redhat.com/archives/dm-devel/2011-November/msg00032.html
> > 
> > But that is only the case if scsi_dh_rdac has already been loaded early
> > by the initramfs right?
> 
> That is correct. I had included  the handler in initramfs.

Including it in initramfs isn't enough.  The initramfs needs to actively
load the scsi_dh module(s) very early to avoid errors resulting from
passive paths.

> > Given the updated scsi_dh match code: should it be safe for the
> > initramfs to just load all scsi_dh modules (and ALUA will be preferred
> > if TPGS is set)?
> 
> Yes, it would be great..

OK.  But ideally we'd have more intelligent autoload of the scsi_dh
modules.  Though it is definitely a nice impovement, thanks to the
match() work, that we can now load all the scsi_dh modules and not worry
about the wrong handler getting attached.

Makes for incremental improvement where tools like dracut can just
blindly load all of the scsi_dh modules that were included in the
initramfs.

> > Does it make sense to re-visit Peter Jones' modalias code to autoload
> > scsi_dh in kernel rather than relying on adhoc initramfs code to know
> > to
> > load the modules?
> 
> I don’t have complete understanding that. Can't comment.

I was referring to work Peter did that enables the ability to autoload
scsi_dh modules via modalias, see this patchset (which would obviously
need to be refreshed): http://pjones.fedorapeople.org/tpgs/

So the kernel would know how to load the appropriate scsi_dh and the
various distro initramfs wouldn't need to worry about blindly loading
all the scsi_dh modules.

Leading to a leaner kernel/boot for systems that don't need scsi_dh.

Mike
--
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