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@xxxxxxx> > --- > 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 > @@ -173,6 +173,11 @@ struct rdac_dh_data { > #define RDAC_STATE_ACTIVE 0 > #define RDAC_STATE_PASSIVE 1 > unsigned char state; > + > +#define RDAC_LUN_UNOWNED 0 > +#define RDAC_LUN_OWNED 1 > +#define RDAC_LUN_AVT 2 > + char lun_state; > unsigned char sense[SCSI_SENSE_BUFFERSIZE]; > union { > struct c2_inquiry c2; > @@ -214,7 +219,7 @@ static struct request *get_rdac_req(struct scsi_device *sdev, > return NULL; > } > > - memset(&rq->cmd, 0, BLK_MAX_CDB); > + memset(rq->cmd, 0, BLK_MAX_CDB); > rq->sense = h->sense; > memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE); > rq->sense_len = 0; > @@ -354,14 +359,16 @@ static int get_lun(struct scsi_device *sdev) > err = submit_inquiry(sdev, 0xC8, sizeof(struct c8_inquiry)); > if (err == SCSI_DH_OK) { > inqp = &h->inq.c8; > - h->lun = inqp->lun[7]; /* currently it uses only one byte */ > + if (inqp->page_code != 0xc8) > + return SCSI_DH_NOSYS; > + if (inqp->page_id[0] != 'e' || inqp->page_id[1] != 'd' || > + inqp->page_id[2] != 'i' || inqp->page_id[3] != 'd') > + return SCSI_DH_NOSYS; > + h->lun = scsilun_to_int((struct scsi_lun *)inqp->lun); > } > return err; > } > > -#define RDAC_OWNED 0 > -#define RDAC_UNOWNED 1 > -#define RDAC_FAILED 2 > static int check_ownership(struct scsi_device *sdev) > { > int err; > @@ -370,17 +377,23 @@ static int check_ownership(struct scsi_device *sdev) > > err = submit_inquiry(sdev, 0xC9, sizeof(struct c9_inquiry)); > if (err == SCSI_DH_OK) { > - err = RDAC_UNOWNED; > inqp = &h->inq.c9; > /* > * 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. > - if (((inqp->avte_cvp >> 7) == 0x1) || > - ((inqp->avte_cvp & 0x1) != 0)) > - err = RDAC_OWNED; > - } else > - err = RDAC_FAILED; > + if ((inqp->avte_cvp >> 7) == 0x1) { > + /* LUN in AVT mode */ > + sdev_printk(KERN_NOTICE, sdev, > + "%s: AVT mode detected\n", > + RDAC_NAME); > + h->lun_state = RDAC_LUN_AVT; > + } else if ((inqp->avte_cvp & 0x1) != 0) { > + /* LUN was owned by the controller */ > + h->lun_state = RDAC_LUN_OWNED; > + } > + } > + > return err; > } > > @@ -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) > + 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) ? > + 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. > + > + 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. > + > try_module_get(THIS_MODULE); > > - sdev_printk(KERN_NOTICE, sdev, "Attached %s\n", RDAC_NAME); > + sdev_printk(KERN_NOTICE, sdev, "%s: Attached\n", RDAC_NAME); > > return 0; > + > +failed: > + spin_lock_irqsave(sdev->request_queue->queue_lock, flags); > + sdev->scsi_dh_data = NULL; > + spin_unlock_irqrestore(sdev->request_queue->queue_lock, flags); > + kfree(scsi_dh_data); > + sdev_printk(KERN_ERR, sdev, "%s: not attached\n", > + RDAC_NAME); > + return -EINVAL; > } > > static void rdac_bus_detach( struct scsi_device *sdev ) > @@ -645,7 +667,7 @@ static void rdac_bus_detach( struct scsi_device *sdev ) > kref_put(&h->ctlr->kref, release_controller); > kfree(scsi_dh_data); > module_put(THIS_MODULE); > - sdev_printk(KERN_NOTICE, sdev, "Detached %s\n", RDAC_NAME); > + sdev_printk(KERN_NOTICE, sdev, "%s: Detached\n", RDAC_NAME); > } > > static int __init rdac_init(void) -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel