[dm-devel] Re: [PATCH 6/7] scsi_dh: Update RDAC device handler
Hannes Reinecke
hare at suse.de
Thu May 15 09:02:25 UTC 2008
Hi Chandra,
Chandra Seetharaman wrote:
> On Wed, 2008-05-14 at 16:43 +0200, Hannes Reinecke wrote:
>> This patch updates the RDAC device handler to
>> refuse to attach to devices not supporting the
>> RDAC vpd pages.
>>
>> Signed-off-by: Hannes Reinecke <hare at suse.de>
>> ---
>> drivers/scsi/device_handler/scsi_dh_rdac.c | 84 +++++++++++++++++----------
>> 1 files changed, 53 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
>> index e61cde6..dd9f515 100644
>> --- a/drivers/scsi/device_handler/scsi_dh_rdac.c
>> +++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
[ .. ]
>> /*
>> * If in AVT mode or if the path already owns the LUN,
>> * return RDAC_OWNED;
>> */
>
> With the code change below the comment above is incorrect, please
> remove.
OK.
>> @@ -478,24 +491,9 @@ static int rdac_activate(struct scsi_device *sdev)
>> struct rdac_dh_data *h = get_rdac_data(sdev);
>> int err = SCSI_DH_OK;
>>
>> - if (h->lun == UNINITIALIZED_LUN) {
>> - err = get_lun(sdev);
>> - if (err != SCSI_DH_OK)
>> - goto done;
>> - }
>> -
>> err = check_ownership(sdev);
>> - switch (err) {
>> - case RDAC_UNOWNED:
>> - break;
>> - case RDAC_OWNED:
>> - err = SCSI_DH_OK;
>> - goto done;
>> - case RDAC_FAILED:
>> - default:
>> - err = SCSI_DH_IO;
>
> What does this change yield ? (under check_ownership)
>
We're now setting the lun state explicitly, so there's
no need to return different error codes.
>> + if (err != SCSI_DH_OK)
>> goto done;
>> - }
>>
>> if (!h->ctlr) {
>> err = initialize_controller(sdev);
>> @@ -508,8 +506,9 @@ static int rdac_activate(struct scsi_device *sdev)
>> if (err != SCSI_DH_OK)
>> goto done;
>> }
>> -
>> - err = send_mode_select(sdev);
>> + if (h->lun_state != RDAC_LUN_AVT &&
>> + !(h->lun_state & RDAC_LUN_OWNED))
>
> This can be simplified by (h->lun_state == RDAC_LUN_UNOWNED) ?
Indeed,
>> + err = send_mode_select(sdev);
>> done:
>> return err;
>> }
>> @@ -606,6 +605,7 @@ static int rdac_bus_attach(struct scsi_device *sdev)
>> struct scsi_dh_data *scsi_dh_data;
>> struct rdac_dh_data *h;
>> unsigned long flags;
>> + int err;
>>
>> scsi_dh_data = kzalloc(sizeof(struct scsi_device_handler *)
>> + sizeof(*h) , GFP_KERNEL);
>> @@ -622,11 +622,33 @@ static int rdac_bus_attach(struct scsi_device *sdev)
>> spin_lock_irqsave(sdev->request_queue->queue_lock, flags);
>> sdev->scsi_dh_data = scsi_dh_data;
>> spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags);
>
> need an initialization for lun_state.
>
No. lun_state is initialized to '0', ie RDAC_LUN_UNOWNED.
I don't think we need a separate 'uninitialized' state here.
>> +
>> + err = get_lun(sdev);
>> + if (err != SCSI_DH_OK)
>> + goto failed;
>> +
>> + err = check_ownership(sdev);
>> + if (err != SCSI_DH_OK)
>> + goto failed;
>> +
>> + sdev_printk(KERN_NOTICE, sdev,
>> + "%s: LUN %d (state %d)\n",
>> + RDAC_NAME, h->lun, h->lun_state);
>
> instead of printing lun_state as %d it would be more readable it is a
> string.
>
Ok.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare at suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
More information about the dm-devel
mailing list