| Message ID | 49EEE071.9060902@suse.de (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Delegated to: | Alasdair Kergon |
| Headers | show |
Hannes Reinecke wrote: > Hi Mike, > > michaelc@cs.wisc.edu wrote: >> From: Mike Christie <michaelc@cs.wisc.edu> >> >> If you have a mixed environment of clarriions, where some >> support ALAU and some support PNR, what do you put in >> your multipath.conf? With this patch you do not have to worry about >> it. If those modules are loaded before dm-mpath, then they >> will have attached to the correct devices based on inquiry, alua commands >> and parsing of data buffers (for example in scsi_dh_emc's alua check). >> There is no need for the user to set that info in the multipath.conf. >> And in general since all scsi_dh_modules will attach to the devices >> they work for, we do not need to have users specific this. >> > No. The problem here is the hardware table from scsi_dh is compiled > in and cannot be changed from userland. The multipath.conf OTOH > is purely user-defined and, what's more, the user might have a valid > reason for modifying it. > (EG EMC Clariion can well be run in PNR mode even though ALUA is > active, or the user might want to try ALUA on any as-of-yet unknown > devices) Ah. I misread the code and misunderstood the compat mode. I thought scsi_dh_emc was failing the attach when ALUA support was detected. > > So _not_ allowing multipath to override the device handler setting > will just add to the confusion and makes error tracking even more > difficult. > > So I would prefer the attached patch, it even save to touch > device handler code at all. > Thanks. I think this will work for me. Are you going to push this for 2.6.30? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Hi Mike, Mike Christie wrote: > Hannes Reinecke wrote: >> Hi Mike, >> >> michaelc@cs.wisc.edu wrote: >>> From: Mike Christie <michaelc@cs.wisc.edu> >>> >>> If you have a mixed environment of clarriions, where some >>> support ALAU and some support PNR, what do you put in >>> your multipath.conf? With this patch you do not have to worry about >>> it. If those modules are loaded before dm-mpath, then they >>> will have attached to the correct devices based on inquiry, alua >>> commands >>> and parsing of data buffers (for example in scsi_dh_emc's alua check). >>> There is no need for the user to set that info in the multipath.conf. >>> And in general since all scsi_dh_modules will attach to the devices >>> they work for, we do not need to have users specific this. >>> >> No. The problem here is the hardware table from scsi_dh is compiled >> in and cannot be changed from userland. The multipath.conf OTOH >> is purely user-defined and, what's more, the user might have a valid >> reason for modifying it. >> (EG EMC Clariion can well be run in PNR mode even though ALUA is >> active, or the user might want to try ALUA on any as-of-yet unknown >> devices) > > Ah. I misread the code and misunderstood the compat mode. I thought > scsi_dh_emc was failing the attach when ALUA support was detected. > >> >> So _not_ allowing multipath to override the device handler setting >> will just add to the confusion and makes error tracking even more >> difficult. >> >> So I would prefer the attached patch, it even save to touch >> device handler code at all. >> > > Thanks. I think this will work for me. > > Are you going to push this for 2.6.30? Well, yes, I could. However, I'm still waiting for agk to push the request-based multipath patches; I've got quite some updates here for multipathing which are done with request-based multipath in place; disentangling them will be quite time consuming (and pointless). But this one, sure. Cheers, Hannes
I agree with Hannes, that we should allow multipath to override the default attachment, mainly to give control to the user. But, I have a small issue with the attached patch. See below. On Wed, 2009-04-22 at 11:16 +0200, Hannes Reinecke wrote: > Hi Mike, > <snip> > diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c > index 095f77b..46d01d9 100644 > --- a/drivers/md/dm-mpath.c > +++ b/drivers/md/dm-mpath.c > @@ -592,12 +592,25 @@ static struct pgpath *parse_path(struct arg_set > *as, struct path_selector *ps, > } > > if (m->hw_handler_name) { > - r = scsi_dh_attach(bdev_get_queue(p->path.dev->bdev), > - m->hw_handler_name); > + struct request_queue *q = bdev_get_queue(p->path.dev->bdev); > + > + r = scsi_dh_attach(q, m->hw_handler_name); > + if (r == -EBUSY) { > + /* > + * Already attached to different hw_handler, > + * try to reattach with correct one. > + */ > + scsi_dh_detach(q); > + r = scsi_dh_attach(q, m->hw_handler_name); > + } > if (r < 0) { > + ti->error = "error attaching hardware handler"; > dm_put_device(ti, p->path.dev); > goto bad; > } > + } else { > + /* Play safe and detach hardware handler */ > + scsi_dh_detach(bdev_get_queue(p->path.dev->bdev)); > } I prefer not to detach a previously attached hardware handler, if multipath doesn't have any handler associated with the device. Leaving it will not do any hard to multipath as the multipath layer would not call scsi_dh_activate for the device. Device handler would just handle the sense code as defined in the kernel. > > r = ps->type->add_path(ps, &p->path, as->argc, as->argv, > &ti->error); -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Wed, Apr 22, 2009 at 10:32:24AM -0700, Chandra Seetharaman wrote: > I agree with Hannes, that we should allow multipath to override the > default attachment, mainly to give control to the user. > But, I have a small issue with the attached patch. See below. > I prefer not to detach a previously attached hardware handler, if > multipath doesn't have any handler associated with the device. Leaving > it will not do any hard to multipath as the multipath layer would not > call scsi_dh_activate for the device. On balance, I'm inclined to agree - too many layers of config overlapping here. If you agree, would one of you like to submit the patch to dm-devel properly, with appropriate signoffs and a descriptive patch header? Alasdair
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 095f77b..46d01d9 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -592,12 +592,25 @@ static struct pgpath *parse_path(struct arg_set *as, struct path_selector *ps, } if (m->hw_handler_name) { - r = scsi_dh_attach(bdev_get_queue(p->path.dev->bdev), - m->hw_handler_name); + struct request_queue *q = bdev_get_queue(p->path.dev->bdev); + + r = scsi_dh_attach(q, m->hw_handler_name); + if (r == -EBUSY) { + /* + * Already attached to different hw_handler, + * try to reattach with correct one. + */ + scsi_dh_detach(q); + r = scsi_dh_attach(q, m->hw_handler_name); + } if (r < 0) { + ti->error = "error attaching hardware handler"; dm_put_device(ti, p->path.dev); goto bad; } + } else { + /* Play safe and detach hardware handler */ + scsi_dh_detach(bdev_get_queue(p->path.dev->bdev)); } r = ps->type->add_path(ps, &p->path, as->argc, as->argv, &ti->error);