[PATCH] Device-Mapper 0/9

[PATCH] Device-Mapper 0/9

[Kevin Corry]

Device-Mapper bug-fixes from the -udm tree.

-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/


Revision 1:
  Fix 64/32 bit ioctl problems.

Revision 2:
  Check the uptodate flag in sub-bios to see if there was an error.
  [Mike Christie]

Revision 3:
  Handle interrupts within suspend.

Revision 4:
  dm.c: Use wake_up() rather than wake_up_interruptible() with the
  eventq.

Revision 5:
  Log an error if the target has unknown target type, or zero length.

Revision 6:
  Correctly align the dm_target_spec structures during retrieve_status().
  
Revision 7:
  Clarify the comment regarding the "next" field in struct dm_target_spec. The
  "next" field has different behavior if you're performing a DM_TABLE_STATUS
  command than it does if you're performing a DM_TABLE_LOAD command.
  
  See populate_table() and retrieve_status() in drivers/md/dm-ioctl.c for more
  details on how this field is used.

Revision 8:
  dm-ioctl.c::retrieve_status(): Prevent overrunning the ioctl buffer by
  making sure we don't call the target status routine with a buffer size limit
  of zero. [Kevin Corry, Alasdair Kergon]

Revision 9:
  Striped: Use an EMIT macro in the status function.

[PATCH] Device-Mapper 1/9

[Kevin Corry]

Fix 64/32 bit ioctl problems.

--- diff/include/linux/compat_ioctl.h	2004-04-03 21:38:19.000000000 -0600
+++ source/include/linux/compat_ioctl.h	2004-04-09 09:41:45.000000000 -0500
@@ -123,6 +123,19 @@
 COMPATIBLE_IOCTL(STOP_ARRAY_RO)
 COMPATIBLE_IOCTL(RESTART_ARRAY_RW)
 /* DM */
+COMPATIBLE_IOCTL(DM_VERSION_32)
+COMPATIBLE_IOCTL(DM_LIST_DEVICES_32)
+COMPATIBLE_IOCTL(DM_DEV_CREATE_32)
+COMPATIBLE_IOCTL(DM_DEV_REMOVE_32)
+COMPATIBLE_IOCTL(DM_DEV_RENAME_32)
+COMPATIBLE_IOCTL(DM_DEV_SUSPEND_32)
+COMPATIBLE_IOCTL(DM_DEV_STATUS_32)
+COMPATIBLE_IOCTL(DM_DEV_WAIT_32)
+COMPATIBLE_IOCTL(DM_TABLE_LOAD_32)
+COMPATIBLE_IOCTL(DM_TABLE_CLEAR_32)
+COMPATIBLE_IOCTL(DM_TABLE_DEPS_32)
+COMPATIBLE_IOCTL(DM_TABLE_STATUS_32)
+COMPATIBLE_IOCTL(DM_LIST_VERSIONS_32)
 COMPATIBLE_IOCTL(DM_VERSION)
 COMPATIBLE_IOCTL(DM_LIST_DEVICES)
 COMPATIBLE_IOCTL(DM_DEV_CREATE)
--- diff/include/linux/dm-ioctl.h	2004-04-03 21:36:13.000000000 -0600
+++ source/include/linux/dm-ioctl.h	2004-04-09 09:41:45.000000000 -0500
@@ -200,6 +200,34 @@
 	DM_LIST_VERSIONS_CMD,
 };
 
+/*
+ * The dm_ioctl struct passed into the ioctl is just the header
+ * on a larger chunk of memory.  On x86-64 and other
+ * architectures the dm-ioctl struct will be padded to an 8 byte
+ * boundary so the size will be different, which would change the
+ * ioctl code - yes I really messed up.  This hack forces these
+ * architectures to have the correct ioctl code.
+ */
+#ifdef CONFIG_COMPAT
+typedef char ioctl_struct[308];
+#define DM_VERSION_32       _IOWR(DM_IOCTL, DM_VERSION_CMD, ioctl_struct)
+#define DM_REMOVE_ALL_32    _IOWR(DM_IOCTL, DM_REMOVE_ALL_CMD, ioctl_struct)
+#define DM_LIST_DEVICES_32  _IOWR(DM_IOCTL, DM_LIST_DEVICES_CMD, ioctl_struct)
+
+#define DM_DEV_CREATE_32    _IOWR(DM_IOCTL, DM_DEV_CREATE_CMD, ioctl_struct)
+#define DM_DEV_REMOVE_32    _IOWR(DM_IOCTL, DM_DEV_REMOVE_CMD, ioctl_struct)
+#define DM_DEV_RENAME_32    _IOWR(DM_IOCTL, DM_DEV_RENAME_CMD, ioctl_struct)
+#define DM_DEV_SUSPEND_32   _IOWR(DM_IOCTL, DM_DEV_SUSPEND_CMD, ioctl_struct)
+#define DM_DEV_STATUS_32    _IOWR(DM_IOCTL, DM_DEV_STATUS_CMD, ioctl_struct)
+#define DM_DEV_WAIT_32      _IOWR(DM_IOCTL, DM_DEV_WAIT_CMD, ioctl_struct)
+
+#define DM_TABLE_LOAD_32    _IOWR(DM_IOCTL, DM_TABLE_LOAD_CMD, ioctl_struct)
+#define DM_TABLE_CLEAR_32   _IOWR(DM_IOCTL, DM_TABLE_CLEAR_CMD, ioctl_struct)
+#define DM_TABLE_DEPS_32    _IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, ioctl_struct)
+#define DM_TABLE_STATUS_32  _IOWR(DM_IOCTL, DM_TABLE_STATUS_CMD, ioctl_struct)
+#define DM_LIST_VERSIONS_32 _IOWR(DM_IOCTL, DM_LIST_VERSIONS_CMD, ioctl_struct)
+#endif
+
 #define DM_IOCTL 0xfd
 
 #define DM_VERSION       _IOWR(DM_IOCTL, DM_VERSION_CMD, struct dm_ioctl)

[PATCH] Device-Mapper 2/9

[Kevin Corry]

Check the uptodate flag in sub-bios to see if there was an error.
[Mike Christie]

--- diff/drivers/md/dm.c	2004-04-09 09:40:14.000000000 -0500
+++ source/drivers/md/dm.c	2004-04-09 09:41:53.000000000 -0500
@@ -294,6 +294,9 @@
 	if (bio->bi_size)
 		return 1;
 
+	if (!bio_flagged(bio, BIO_UPTODATE) && !error)
+		error = -EIO;
+
 	if (endio) {
 		r = endio(tio->ti, bio, error, &tio->info);
 		if (r < 0)

[PATCH] Device-Mapper 3/9

[Kevin Corry]

Handle interrupts within suspend.

--- diff/drivers/md/dm.c	2004-04-09 09:41:53.000000000 -0500
+++ source/drivers/md/dm.c	2004-04-09 09:42:02.000000000 -0500
@@ -925,7 +925,7 @@
 	while (1) {
 		set_current_state(TASK_INTERRUPTIBLE);
 
-		if (!atomic_read(&md->pending))
+		if (!atomic_read(&md->pending) || signal_pending(current))
 			break;
 
 		io_schedule();
@@ -934,6 +934,14 @@
 
 	down_write(&md->lock);
 	remove_wait_queue(&md->wait, &wait);
+
+	/* were we interrupted ? */
+	if (atomic_read(&md->pending)) {
+		clear_bit(DMF_BLOCK_IO, &md->flags);
+		up_write(&md->lock);
+		return -EINTR;
+	}
+
 	set_bit(DMF_SUSPENDED, &md->flags);
 
 	map = dm_get_table(md);

[PATCH] Device-Mapper 4/9

[Kevin Corry]

dm.c: Use wake_up() rather than wake_up_interruptible() with the
eventq.

--- diff/drivers/md/dm.c	2004-04-09 09:42:02.000000000 -0500
+++ source/drivers/md/dm.c	2004-04-09 09:42:12.000000000 -0500
@@ -748,7 +748,7 @@
 
 	down_write(&md->lock);
 	md->event_nr++;
-	wake_up_interruptible(&md->eventq);
+	wake_up(&md->eventq);
 	up_write(&md->lock);
 }
 

[PATCH] Device-Mapper 5/9

[Kevin Corry]

Log an error if the target has unknown target type, or zero length.

--- diff/drivers/md/dm-table.c	2004-04-09 09:40:14.000000000 -0500
+++ source/drivers/md/dm-table.c	2004-04-09 09:42:20.000000000 -0500
@@ -663,12 +663,14 @@
 
 	if (!len) {
 		tgt->error = "zero-length target";
+		DMERR(": %s\n", tgt->error);
 		return -EINVAL;
 	}
 
 	tgt->type = dm_get_target_type(type);
 	if (!tgt->type) {
 		tgt->error = "unknown target type";
+		DMERR(": %s\n", tgt->error);
 		return -EINVAL;
 	}
 
@@ -705,7 +707,7 @@
 	return 0;
 
  bad:
-	printk(KERN_ERR DM_NAME ": %s\n", tgt->error);
+	DMERR(": %s\n", tgt->error);
 	dm_put_target_type(tgt->type);
 	return r;
 }

[PATCH] Device-Mapper 6/9

[Kevin Corry]

Correctly align the dm_target_spec structures during retrieve_status().

--- diff/drivers/md/dm-ioctl.c	2004-04-03 21:36:53.000000000 -0600
+++ source/drivers/md/dm-ioctl.c	2004-04-09 09:42:29.000000000 -0500
@@ -828,7 +828,7 @@
 		outptr += strlen(outptr) + 1;
 		used = param->data_start + (outptr - outbuf);
 
-		align_ptr(outptr);
+		outptr = align_ptr(outptr);
 		spec->next = outptr - outbuf;
 	}
 

[PATCH] Device-Mapper 7/9

[Kevin Corry]

Clarify the comment regarding the "next" field in struct dm_target_spec. The
"next" field has different behavior if you're performing a DM_TABLE_STATUS
command than it does if you're performing a DM_TABLE_LOAD command.

See populate_table() and retrieve_status() in drivers/md/dm-ioctl.c for more
details on how this field is used.

--- diff/include/linux/dm-ioctl.h	2004-04-09 09:41:45.000000000 -0500
+++ source/include/linux/dm-ioctl.h	2004-04-09 09:42:36.000000000 -0500
@@ -129,8 +129,14 @@
 	int32_t status;		/* used when reading from kernel only */
 
 	/*
-	 * Offset in bytes (from the start of this struct) to
-	 * next target_spec.
+	 * Location of the next dm_target_spec.
+	 * - When specifying targets on a DM_TABLE_LOAD command, this value is
+	 *   the number of bytes from the start of the "current" dm_target_spec
+	 *   to the start of the "next" dm_target_spec.
+	 * - When retrieving targets on a DM_TABLE_STATUS command, this value
+	 *   is the number of bytes from the start of the first dm_target_spec
+	 *   (that follows the dm_ioctl struct) to the start of the "next"
+	 *   dm_target_spec.
 	 */
 	uint32_t next;
 

[PATCH] Device-Mapper 8/9

[Kevin Corry]

dm-ioctl.c::retrieve_status(): Prevent overrunning the ioctl buffer by making
sure we don't call the target status routine with a buffer size limit of zero.
[Kevin Corry, Alasdair Kergon]

--- diff/drivers/md/dm-ioctl.c	2004-04-09 09:42:29.000000000 -0500
+++ source/drivers/md/dm-ioctl.c	2004-04-09 09:42:44.000000000 -0500
@@ -800,7 +800,7 @@
 		struct dm_target *ti = dm_table_get_target(table, i);
 
 		remaining = len - (outptr - outbuf);
-		if (remaining < sizeof(struct dm_target_spec)) {
+		if (remaining <= sizeof(struct dm_target_spec)) {
 			param->flags |= DM_BUFFER_FULL_FLAG;
 			break;
 		}
@@ -815,6 +815,10 @@
 
 		outptr += sizeof(struct dm_target_spec);
 		remaining = len - (outptr - outbuf);
+		if (remaining <= 0) {
+			param->flags |= DM_BUFFER_FULL_FLAG;
+			break;
+		}
 
 		/* Get the status/table string from the target driver */
 		if (ti->type->status) {

[PATCH] Device-Mapper 9/9

[Kevin Corry]

Striped: Use an EMIT macro in the status function.

--- diff/drivers/md/dm-stripe.c	2004-04-03 21:37:06.000000000 -0600
+++ source/drivers/md/dm-stripe.c	2004-04-09 09:42:55.000000000 -0500
@@ -187,24 +187,24 @@
 			 status_type_t type, char *result, unsigned int maxlen)
 {
 	struct stripe_c *sc = (struct stripe_c *) ti->private;
-	int offset;
+	unsigned int sz = 0;
 	unsigned int i;
 	char buffer[32];
 
+#define EMIT(x...) sz += ((sz >= maxlen) ? \
+			  0 : scnprintf(result + sz, maxlen - sz, x))
+
 	switch (type) {
 	case STATUSTYPE_INFO:
 		result[0] = '\0';
 		break;
 
 	case STATUSTYPE_TABLE:
-		offset = scnprintf(result, maxlen, "%d " SECTOR_FORMAT,
-				  sc->stripes, sc->chunk_mask + 1);
+		EMIT("%d " SECTOR_FORMAT, sc->stripes, sc->chunk_mask + 1);
 		for (i = 0; i < sc->stripes; i++) {
 			format_dev_t(buffer, sc->stripe[i].dev->bdev->bd_dev);
-			offset +=
-			    scnprintf(result + offset, maxlen - offset,
-				     " %s " SECTOR_FORMAT, buffer,
-				     sc->stripe[i].physical_start);
+			EMIT(" %s " SECTOR_FORMAT, buffer,
+			     sc->stripe[i].physical_start);
 		}
 		break;
 	}