public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Chandra Seetharaman <sekharan@us.ibm.com>
To: Hannes Reinecke <hare@suse.de>
Cc: michaelc@cs.wisc.edu, dm-devel@redhat.com, linux-scsi@vger.kernel.org
Subject: Re: [PATCH] RFC: have dm-mpath use already attached scsi_dh
Date: Wed, 22 Apr 2009 10:32:24 -0700	[thread overview]
Message-ID: <1240421544.19442.6.camel@chandra-ubuntu> (raw)
In-Reply-To: <49EEE071.9060902@suse.de>

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);

  parent reply	other threads:[~2009-04-22 17:32 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-22  4:33 [PATCH] RFC: have dm-mpath use already attached scsi_dh michaelc
2009-04-22  9:16 ` Hannes Reinecke
2009-04-22 13:52   ` Mike Christie
2009-04-22 14:15     ` Hannes Reinecke
2009-04-22 17:32   ` Chandra Seetharaman [this message]
2009-04-22 17:39     ` Alasdair G Kergon

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=1240421544.19442.6.camel@chandra-ubuntu \
    --to=sekharan@us.ibm.com \
    --cc=dm-devel@redhat.com \
    --cc=hare@suse.de \
    --cc=linux-scsi@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=sekharan@linux.vnet.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