2004-04-12 14:13:04

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 0/9

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

--
Kevin Corry
[email protected]
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.


2004-04-12 14:18:19

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 1/9

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)

2004-04-12 14:20:13

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 4/9

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

2004-04-12 14:19:58

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 3/9

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

2004-04-12 14:23:16

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 2/9

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)

2004-04-12 14:23:16

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 9/9

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

2004-04-12 14:26:33

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 5/9

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

2004-04-12 14:26:39

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 8/9

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

2004-04-12 14:26:39

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 7/9

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;

2004-04-12 14:26:38

by Kevin Corry

[permalink] [raw]
Subject: [PATCH] Device-Mapper 6/9

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