[RESEND PATCH 0/3] multipath features for 3.6

[RESEND PATCH 0/3] multipath features for 3.6

[Mike Snitzer]

Hi James,

This scsi_dh patch is a prerequisite for the corresponding dm-multipath patch.
This scsi_dh patch has seen wide review and testing -- please apply it
for the 3.6 merge.  Alasdair will stage the dm-multipath changes for 3.6.

Thanks,
Mike

Hannes Reinecke (1):
  dm mpath: add ability to disable partition creation

Mike Snitzer (2):
  scsi_dh: add scsi_dh_attached_handler_name
  dm mpath: add 'default_hw_handler' feature

 drivers/md/dm-mpath.c                 |   37 ++++++++++++++++++++++++++++---
 drivers/scsi/device_handler/scsi_dh.c |   38 +++++++++++++++++++++++++++++++-
 include/scsi/scsi_dh.h                |    6 +++++
 3 files changed, 75 insertions(+), 6 deletions(-)

-- 
1.7.4.4

[RESEND PATCH 1/3] scsi_dh: add scsi_dh_attached_handler_name

[Mike Snitzer]

Introduce scsi_dh_attached_handler_name() to retrieve the name of the
scsi_dh that is attached to the scsi_device associated with the provided
request queue.  Returns NULL if a scsi_dh is not attached.

Also, fix scsi_dh_{attach,detach} function header comments to document
@q rather than @sdev.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Tested-by: Babu Moger <babu.moger@netapp.com>
Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>
Acked-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh.c |   38 +++++++++++++++++++++++++++++++-
 include/scsi/scsi_dh.h                |    6 +++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 48e46f5..33e422e 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -468,7 +468,8 @@ EXPORT_SYMBOL_GPL(scsi_dh_handler_exist);
 
 /*
  * scsi_dh_attach - Attach device handler
- * @sdev - sdev the handler should be attached to
+ * @q - Request queue that is associated with the scsi_device
+ *      the handler should be attached to
  * @name - name of the handler to attach
  */
 int scsi_dh_attach(struct request_queue *q, const char *name)
@@ -498,7 +499,8 @@ EXPORT_SYMBOL_GPL(scsi_dh_attach);
 
 /*
  * scsi_dh_detach - Detach device handler
- * @sdev - sdev the handler should be detached from
+ * @q - Request queue that is associated with the scsi_device
+ *      the handler should be detached from
  *
  * This function will detach the device handler only
  * if the sdev is not part of the internal list, ie
@@ -527,6 +529,38 @@ void scsi_dh_detach(struct request_queue *q)
 }
 EXPORT_SYMBOL_GPL(scsi_dh_detach);
 
+/*
+ * scsi_dh_attached_handler_name - Get attached device handler's name
+ * @q - Request queue that is associated with the scsi_device
+ *      that may have a device handler attached
+ * @gfp - the GFP mask used in the kmalloc() call when allocating memory
+ *
+ * Returns name of attached handler, NULL if no handler is attached.
+ * Caller must take care to free the returned string.
+ */
+const char *scsi_dh_attached_handler_name(struct request_queue *q, gfp_t gfp)
+{
+	unsigned long flags;
+	struct scsi_device *sdev;
+	const char *handler_name = NULL;
+
+	spin_lock_irqsave(q->queue_lock, flags);
+	sdev = q->queuedata;
+	if (!sdev || !get_device(&sdev->sdev_gendev))
+		sdev = NULL;
+	spin_unlock_irqrestore(q->queue_lock, flags);
+
+	if (!sdev)
+		return NULL;
+
+	if (sdev->scsi_dh_data)
+		handler_name = kstrdup(sdev->scsi_dh_data->scsi_dh->name, gfp);
+
+	put_device(&sdev->sdev_gendev);
+	return handler_name;
+}
+EXPORT_SYMBOL_GPL(scsi_dh_attached_handler_name);
+
 static struct notifier_block scsi_dh_nb = {
 	.notifier_call = scsi_dh_notifier
 };
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index e3f2db2..620c723 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -60,6 +60,7 @@ extern int scsi_dh_activate(struct request_queue *, activate_complete, void *);
 extern int scsi_dh_handler_exist(const char *);
 extern int scsi_dh_attach(struct request_queue *, const char *);
 extern void scsi_dh_detach(struct request_queue *);
+extern const char *scsi_dh_attached_handler_name(struct request_queue *, gfp_t);
 extern int scsi_dh_set_params(struct request_queue *, const char *);
 #else
 static inline int scsi_dh_activate(struct request_queue *req,
@@ -80,6 +81,11 @@ static inline void scsi_dh_detach(struct request_queue *q)
 {
 	return;
 }
+static inline const char *scsi_dh_attached_handler_name(struct request_queue *q,
+							gfp_t gfp)
+{
+	return NULL;
+}
 static inline int scsi_dh_set_params(struct request_queue *req, const char *params)
 {
 	return -SCSI_DH_NOSYS;
-- 
1.7.4.4

Re: [RESEND PATCH 1/3] scsi_dh: add scsi_dh_attached_handler_name

[Mike Snitzer]

On Tue, Jun 26 2012 at  2:32pm -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> Introduce scsi_dh_attached_handler_name() to retrieve the name of the
> scsi_dh that is attached to the scsi_device associated with the provided
> request queue.  Returns NULL if a scsi_dh is not attached.
> 
> Also, fix scsi_dh_{attach,detach} function header comments to document
> @q rather than @sdev.
> 
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> Tested-by: Babu Moger <babu.moger@netapp.com>
> Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Acked-by: Hannes Reinecke <hare@suse.de>

Hi James,

Any insight on staging this scsi_dh patch for 3.6?

Or what about adding your Acked-by and allowing Alasdair to submit it
for 3.6 with the corresponding dm-mpath patch?

However you'd like to skin it is fine by me.  I'd just _really_
appreciate some feedback.

Thanks,
Mike

p.s. I'm nothing if not persistent ;)

[RESEND PATCH 2/3] dm mpath: add 'default_hw_handler' feature

[Mike Snitzer]

When specifying the feature 'default_hw_handler' multipath will use
the currently attached hardware handler instead of trying to attach the
one specified during table load.  If no hardware handler is attached the
specified hardware handler will be used.

Leverages scsi_dh_attach's ability to increment the scsi_dh's reference
count if the same scsi_dh name is provided when attaching -- currently
attached scsi_dh name is determined with scsi_dh_attached_handler_name.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Tested-by: Babu Moger <babu.moger@netapp.com>
Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 drivers/md/dm-mpath.c |   28 ++++++++++++++++++++++++----
 1 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 638dae0..4dd3f59 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -85,6 +85,7 @@ struct multipath {
 	unsigned queue_io:1;		/* Must we queue all I/O? */
 	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
 	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
+	unsigned use_default_hw_handler:1; /* Use attached device handler */
 
 	unsigned pg_init_retries;	/* Number of times to retry pg_init */
 	unsigned pg_init_count;		/* Number of times pg_init called */
@@ -568,6 +569,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 	int r;
 	struct pgpath *p;
 	struct multipath *m = ti->private;
+	struct request_queue *q = NULL;
 
 	/* we need at least a path arg */
 	if (as->argc < 1) {
@@ -586,9 +588,19 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 		goto bad;
 	}
 
-	if (m->hw_handler_name) {
-		struct request_queue *q = bdev_get_queue(p->path.dev->bdev);
+	if (m->use_default_hw_handler || m->hw_handler_name)
+		q = bdev_get_queue(p->path.dev->bdev);
+
+	if (m->use_default_hw_handler) {
+		const char *attached_handler_name =
+			scsi_dh_attached_handler_name(q, GFP_KERNEL);
+		if (attached_handler_name) {
+			kfree(m->hw_handler_name);
+			m->hw_handler_name = attached_handler_name;
+		}
+	}
 
+	if (m->hw_handler_name) {
 		r = scsi_dh_attach(q, m->hw_handler_name);
 		if (r == -EBUSY) {
 			/*
@@ -760,7 +772,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 	const char *arg_name;
 
 	static struct dm_arg _args[] = {
-		{0, 5, "invalid number of feature args"},
+		{0, 6, "invalid number of feature args"},
 		{1, 50, "pg_init_retries must be between 1 and 50"},
 		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
 	};
@@ -781,6 +793,11 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 			continue;
 		}
 
+		if (!strcasecmp(arg_name, "default_hw_handler")) {
+			m->use_default_hw_handler = 1;
+			continue;
+		}
+
 		if (!strcasecmp(arg_name, "pg_init_retries") &&
 		    (argc >= 1)) {
 			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
@@ -1364,13 +1381,16 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 	else {
 		DMEMIT("%u ", m->queue_if_no_path +
 			      (m->pg_init_retries > 0) * 2 +
-			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2);
+			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
+			      m->use_default_hw_handler);
 		if (m->queue_if_no_path)
 			DMEMIT("queue_if_no_path ");
 		if (m->pg_init_retries)
 			DMEMIT("pg_init_retries %u ", m->pg_init_retries);
 		if (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT)
 			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
+		if (m->use_default_hw_handler)
+			DMEMIT("default_hw_handler ");
 	}
 
 	if (!m->hw_handler_name || type == STATUSTYPE_INFO)
-- 
1.7.4.4

Re: [dm-devel] [RESEND PATCH 2/3] dm mpath: add 'default_hw_handler' feature

[Hannes Reinecke]

On 06/26/2012 08:32 PM, Mike Snitzer wrote:
> When specifying the feature 'default_hw_handler' multipath will use
> the currently attached hardware handler instead of trying to attach the
> one specified during table load.  If no hardware handler is attached the
> specified hardware handler will be used.
> 
> Leverages scsi_dh_attach's ability to increment the scsi_dh's reference
> count if the same scsi_dh name is provided when attaching -- currently
> attached scsi_dh name is determined with scsi_dh_attached_handler_name.
> 
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> Tested-by: Babu Moger <babu.moger@netapp.com>
> Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>

Looks good.

Acked-by: Hannes Reinecke <hare@suse.de>
-- 
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] [RESEND PATCH 2/3] dm mpath: add 'default_hw_handler' feature

[Alasdair G Kergon]

On Wed, Jun 27, 2012 at 08:16:55AM +0200, Hannes Reinecke wrote:
> On 06/26/2012 08:32 PM, Mike Snitzer wrote:
> > When specifying the feature 'default_hw_handler' multipath will use
> > the currently attached hardware handler instead of trying to attach the
> > one specified during table load.  If no hardware handler is attached the
> > specified hardware handler will be used.
> > 
> > Leverages scsi_dh_attach's ability to increment the scsi_dh's reference
> > count if the same scsi_dh name is provided when attaching -- currently
> > attached scsi_dh name is determined with scsi_dh_attached_handler_name.
> > 
> > Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> > Tested-by: Babu Moger <babu.moger@netapp.com>
> > Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Looks good.
 
Maybe, but I'd like to see an inline explanation of what this confusing new
setting means and a better patch header that provides some motivation for this
change.

To my eyes, the word "default" is over-used here.  If I *don't* specify the new
"default" flag, surely I'll get default behaviour, won't I, by definition?  And
if I do specify it, I'm asking for default behaviour too, so isn't it
redundant?

Alasdair

Re: [RESEND PATCH 2/3] dm mpath: add 'default_hw_handler' feature

[Mike Snitzer]

On Wed, Jul 11 2012 at  6:27am -0400,
Alasdair G Kergon <agk@redhat.com> wrote:

> On Wed, Jun 27, 2012 at 08:16:55AM +0200, Hannes Reinecke wrote:
> > On 06/26/2012 08:32 PM, Mike Snitzer wrote:
> > > When specifying the feature 'default_hw_handler' multipath will use
> > > the currently attached hardware handler instead of trying to attach the
> > > one specified during table load.  If no hardware handler is attached the
> > > specified hardware handler will be used.
> > > 
> > > Leverages scsi_dh_attach's ability to increment the scsi_dh's reference
> > > count if the same scsi_dh name is provided when attaching -- currently
> > > attached scsi_dh name is determined with scsi_dh_attached_handler_name.
> > > 
> > > Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> > > Tested-by: Babu Moger <babu.moger@netapp.com>
> > > Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > Looks good.
>  
> Maybe, but I'd like to see an inline explanation of what this confusing new
> setting means and a better patch header that provides some motivation for this
> change.
> 
> To my eyes, the word "default" is over-used here.  If I *don't* specify the new
> "default" flag, surely I'll get default behaviour, won't I, by definition?  And
> if I do specify it, I'm asking for default behaviour too, so isn't it
> redundant?

"default" is in reference to the hardware handler that will get attached
by the scsi_dh .match().  It has nothing to do with the userspace
multipath-tool's desired handler.

[RESEND PATCH 3/3] dm mpath: add ability to disable partition creation

[Mike Snitzer]

From: Hannes Reinecke <hare@suse.de>

When multipath devices are being used as disks for VM Guests any
partition scanning / setup should be done within the VM Guest, not from
host.  So we need to a mechanism to switch off partition scanning and
creation via kpartx.

The new 'no_partitions' feature serves as a notifier to kpartx to _not_
create partitions on these multipath devices.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 drivers/md/dm-mpath.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 4dd3f59..115ad9e 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -86,6 +86,7 @@ struct multipath {
 	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
 	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
 	unsigned use_default_hw_handler:1; /* Use attached device handler */
+	unsigned no_partitions:1;	/* Avoid partition scanning */
 
 	unsigned pg_init_retries;	/* Number of times to retry pg_init */
 	unsigned pg_init_count;		/* Number of times pg_init called */
@@ -772,7 +773,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 	const char *arg_name;
 
 	static struct dm_arg _args[] = {
-		{0, 6, "invalid number of feature args"},
+		{0, 7, "invalid number of feature args"},
 		{1, 50, "pg_init_retries must be between 1 and 50"},
 		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
 	};
@@ -798,6 +799,11 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 			continue;
 		}
 
+		if (!strcasecmp(arg_name, "no_partitions")) {
+			m->no_partitions = 1;
+			continue;
+		}
+
 		if (!strcasecmp(arg_name, "pg_init_retries") &&
 		    (argc >= 1)) {
 			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
@@ -1382,7 +1388,8 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 		DMEMIT("%u ", m->queue_if_no_path +
 			      (m->pg_init_retries > 0) * 2 +
 			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
-			      m->use_default_hw_handler);
+			      m->use_default_hw_handler +
+			      m->no_partitions);
 		if (m->queue_if_no_path)
 			DMEMIT("queue_if_no_path ");
 		if (m->pg_init_retries)
@@ -1391,6 +1398,8 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
 		if (m->use_default_hw_handler)
 			DMEMIT("default_hw_handler ");
+		if (m->no_partitions)
+			DMEMIT("no_partitions ");
 	}
 
 	if (!m->hw_handler_name || type == STATUSTYPE_INFO)
-- 
1.7.4.4

Re: [dm-devel] [RESEND PATCH 3/3] dm mpath: add ability to disable partition creation

[Alasdair G Kergon]

On Tue, Jun 26, 2012 at 02:32:05PM -0400, Mike Snitzer wrote:
> The new 'no_partitions' feature serves as a notifier to kpartx to _not_
> create partitions on these multipath devices.
 
This isn't really multipath-specific so doesn't belong in the target.
It could go into dm core, but we already have flags attached to
udev cookies that can turn udev rules on and off and thereby could
allow userspace multipath to control whether or not kpartx creates
partitions on any particular device.

But first I'd like us to explore creating a config file for kpartx and
controlling the behaviour from there.  Activation could then be
triggered by target type, device name, scsi WWID, dm UUID etc. according
to rules in that file.

I think this approach of handling each layer independently gives us the
best chance of ending up with integrated management of arbitrary device
stacks.

Alasdair

Re: [dm-devel] [RESEND PATCH 3/3] dm mpath: add ability to disable partition creation

[Hannes Reinecke]

On 07/11/2012 05:18 PM, Alasdair G Kergon wrote:
> On Tue, Jun 26, 2012 at 02:32:05PM -0400, Mike Snitzer wrote:
>> The new 'no_partitions' feature serves as a notifier to kpartx to _not_
>> create partitions on these multipath devices.
>  
> This isn't really multipath-specific so doesn't belong in the target.
> It could go into dm core, but we already have flags attached to
> udev cookies that can turn udev rules on and off and thereby could
> allow userspace multipath to control whether or not kpartx creates
> partitions on any particular device.
> 
> But first I'd like us to explore creating a config file for kpartx and
> controlling the behaviour from there.  Activation could then be
> triggered by target type, device name, scsi WWID, dm UUID etc. according
> to rules in that file.
> 
But that would mean one would have to maintain _two_ configuration
files, one for multipath and one for kpartx.

Also kpartx initially doesn't have any clue about device names, SCSI
WWIDs etc. It just takes a device-mapper device and acts upon it.
The only information it's got is the device-mapper name (which could
be anything) and the device-mapper UUID if present.

Same goes for udev rules; one would have to traverse the device
stack backwards to retrieve any information about the hardware.

So when moving the configuration into kpartx we would lose quite
some flexibility. And reliability, methinks.

Flexilibity is a valid goal, but not when it can achieved only by
adding complexity to other pieces.

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: [RESEND PATCH 3/3] dm mpath: add ability to disable partition creation

[Benjamin Marzinski]

On Wed, Jul 25, 2012 at 02:02:15PM +0200, Hannes Reinecke wrote:
> On 07/11/2012 05:18 PM, Alasdair G Kergon wrote:
> > On Tue, Jun 26, 2012 at 02:32:05PM -0400, Mike Snitzer wrote:
> >> The new 'no_partitions' feature serves as a notifier to kpartx to _not_
> >> create partitions on these multipath devices.
> >  
> > This isn't really multipath-specific so doesn't belong in the target.
> > It could go into dm core, but we already have flags attached to
> > udev cookies that can turn udev rules on and off and thereby could
> > allow userspace multipath to control whether or not kpartx creates
> > partitions on any particular device.
> > 
> > But first I'd like us to explore creating a config file for kpartx and
> > controlling the behaviour from there.  Activation could then be
> > triggered by target type, device name, scsi WWID, dm UUID etc. according
> > to rules in that file.
> > 
> But that would mean one would have to maintain _two_ configuration
> files, one for multipath and one for kpartx.
> 
> Also kpartx initially doesn't have any clue about device names, SCSI
> WWIDs etc. It just takes a device-mapper device and acts upon it.
> The only information it's got is the device-mapper name (which could
> be anything) and the device-mapper UUID if present.
> 
> Same goes for udev rules; one would have to traverse the device
> stack backwards to retrieve any information about the hardware.
> 
> So when moving the configuration into kpartx we would lose quite
> some flexibility. And reliability, methinks.
> 
> Flexilibity is a valid goal, but not when it can achieved only by
> adding complexity to other pieces.

Kpartx does already grab the dm device name, uuid and target type
(although it doesn't currently use the target type for anything). So,
aside from adding code to read a config file, we don't really have to
add any complexity to be able to have kpartx configure a device based on
dm name (which admittedly probably isn't that useful), dm UUID, or
target type.  We could also have it configure based on the blkid
results, which should be present when it's called by udev. When
called manually, kpartx could use libblkid.

We do lose the ability to have kpartx easily configure a device based on
hardware type.  However, that's not really essential.  Multipath needs
it because it can't even finish setting up a device until it knows the
hardware type.  For kpartx, it could make configuration simpler in cases
where all of the LUNs from a particular hardware type needed to be
configured the same way.  This may be handy, but its not necessary, and
you could achieve the same results using UUIDs. What we gain is the
ability to set this property for non-multipath devices, which currently
isn't possible.  Also, blkid based (fs type, fs uuid, and fs label)
configuration seems to me like it would be the most useful. 

Now I'm not arguing that adding configuration file handling isn't adding
complexity, and adding libblkid calls would add more. But since
multipath always sets the UUID, I don't see this losing any reliability,
and I does gain the fexibility to work with any dm target type. I
don't feel strongly about this one way or the other, but do see the
benefits of having kpartx own this configuration setting.

-Ben

> 
> 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] [RESEND PATCH 3/3] dm mpath: add ability to disable partition creation

[Hannes Reinecke]

On 07/26/2012 07:03 PM, Benjamin Marzinski wrote:
> On Wed, Jul 25, 2012 at 02:02:15PM +0200, Hannes Reinecke wrote:
[ .. ]
>> But that would mean one would have to maintain _two_ configuration
>> files, one for multipath and one for kpartx.
>>
>> Also kpartx initially doesn't have any clue about device names, SCSI
>> WWIDs etc. It just takes a device-mapper device and acts upon it.
>> The only information it's got is the device-mapper name (which could
>> be anything) and the device-mapper UUID if present.
>>
>> Same goes for udev rules; one would have to traverse the device
>> stack backwards to retrieve any information about the hardware.
>>
>> So when moving the configuration into kpartx we would lose quite
>> some flexibility. And reliability, methinks.
>>
>> Flexilibity is a valid goal, but not when it can achieved only by
>> adding complexity to other pieces.
> 
> Kpartx does already grab the dm device name, uuid and target type
> (although it doesn't currently use the target type for anything). So,
> aside from adding code to read a config file, we don't really have to
> add any complexity to be able to have kpartx configure a device based on
> dm name (which admittedly probably isn't that useful), dm UUID, or
> target type.  We could also have it configure based on the blkid
> results, which should be present when it's called by udev. When
> called manually, kpartx could use libblkid.
> 
Hmpf. Feels like a waste; multipath already has this information ...
Anyway.

> We do lose the ability to have kpartx easily configure a device based on
> hardware type.  However, that's not really essential.  Multipath needs
> it because it can't even finish setting up a device until it knows the
> hardware type.  For kpartx, it could make configuration simpler in cases
> where all of the LUNs from a particular hardware type needed to be
> configured the same way.  This may be handy, but its not necessary, and
> you could achieve the same results using UUIDs. What we gain is the
> ability to set this property for non-multipath devices, which currently
> isn't possible.  Also, blkid based (fs type, fs uuid, and fs label)
> configuration seems to me like it would be the most useful. 
> 
Oh. Right.
Yes, indeed, other target types beside from multipath will benefit
from this approach, too.

> Now I'm not arguing that adding configuration file handling isn't adding
> complexity, and adding libblkid calls would add more. But since
> multipath always sets the UUID, I don't see this losing any reliability,
> and I does gain the fexibility to work with any dm target type. I
> don't feel strongly about this one way or the other, but do see the
> benefits of having kpartx own this configuration setting.
> 
The main problem I have with the kpartx approach is that none of the
information in the device-mapper device is well-defined.
The name certainly isn't, and even the UUID is a case of "let's hope
no-one else is daft enough using this".

Did we ever get around formalizing how the UUID should look like?
If so, and if we could enforce this policy, then we can be sure the
UUID is actually meaningful.

But if we can't then kpartx would have to crawl back to the parent
device to grab some information from there.
blkid notwithstanding.
And that's just horrible.

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