On Mon, Jul 27 2009 at 10:23am -0400,
Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
> On Mon, 27 Jul 2009, Mike Snitzer wrote:
>
> > On Mon, Jul 27 2009 at 9:21am -0400,
> > Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
> >
> > > Hi
> > >
> > > This patch fixes a bug introduced in 2.6.31-rc1. Targets that define
> > > iterate_devices and don't contain any devices (such as dm-loop in fsio)
> > > don't work.
> > >
> > > Mikulas
> > >
> > > ---
> > >
> > > Fix reverse logic in device_area_is_valid
> > >
> > > ->iterate_devices method calls the callback for every underlying device
> > > in the target. If the callback returns non-zero, iterate_devices exits
> > > and returns this value. If the callback returns zero for all the devices,
> > > iterate_devices returns zero.
> > >
> > > The logic to check for invalid device areas was reversed.
> > > device_area_is_valid returned 0 on error and 1 on success.
> > >
> > > Thus:
> > > - error was returned only if all the devices vere errorneous. If some of them
> > > returned 1 (success), dm_calculate_queue_limits understood this as success.
> > > - if the target had no device (the example is dm-loop target in FSIO mode),
> > > iterate_devices returned error straight away and dm_calculate_queue_limits
> > > understood this as an error.
> > >
> > > This patch reverses the logic so that 0 means success and 1 means error.
> > >
> > > Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> >
> > Nice catch, interesting that this was somehow missed. All targets do
> > treat a non-zero return from .iterate_devices as an error.
> >
> > We should probably rename device_area_is_valid to device_area_invalid.
> >
> > Mike
>
> Yes, you can rename it to that.
>
> Mikulas
---
From: Mikulas Patocka <mpatocka@xxxxxxxxxx>
Rename device_area_is_valid and reverse logic accordingly
->iterate_devices method calls the callback for every underlying device
in the target. If the callback returns non-zero iterate_devices exits
immediately and the caller should understand that an error occurred.
The logic to check for valid device areas was reversed relative to
proper use with iterate_devices. device_area_is_valid returned 0 on
error and 1 on success. Thus error (from iterate_devices) was returned
only if all the devices were invalid.
This patch renames device_area_is_valid to device_area_is_invalid and
reverses the logic so that 0 means valid and 1 means invalid (to match
iterate_devices).
Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
---
drivers/md/dm-table.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index d952b34..aa60526 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -343,10 +343,10 @@ static void close_dev(struct dm_dev_internal *d, struct mapped_device *md)
}
/*
- * If possible, this checks an area of a destination device is valid.
+ * If possible, this checks an area of a destination device is invalid.
*/
-static int device_area_is_valid(struct dm_target *ti, struct dm_dev *dev,
- sector_t start, sector_t len, void *data)
+static int device_area_is_invalid(struct dm_target *ti, struct dm_dev *dev,
+ sector_t start, sector_t len, void *data)
{
struct queue_limits *limits = data;
struct block_device *bdev = dev->bdev;
@@ -357,16 +357,16 @@ static int device_area_is_valid(struct dm_target *ti, struct dm_dev *dev,
char b[BDEVNAME_SIZE];
if (!dev_size)
- return 1;
+ return 0;
if ((start >= dev_size) || (start + len > dev_size)) {
DMWARN("%s: %s too small for target",
dm_device_name(ti->table->md), bdevname(bdev, b));
- return 0;
+ return 1;
}
if (logical_block_size_sectors <= 1)
- return 1;
+ return 0;
if (start & (logical_block_size_sectors - 1)) {
DMWARN("%s: start=%llu not aligned to h/w "
@@ -374,7 +374,7 @@ static int device_area_is_valid(struct dm_target *ti, struct dm_dev *dev,
dm_device_name(ti->table->md),
(unsigned long long)start,
limits->logical_block_size, bdevname(bdev, b));
- return 0;
+ return 1;
}
if (len & (logical_block_size_sectors - 1)) {
@@ -383,10 +383,10 @@ static int device_area_is_valid(struct dm_target *ti, struct dm_dev *dev,
dm_device_name(ti->table->md),
(unsigned long long)len,
limits->logical_block_size, bdevname(bdev, b));
- return 0;
+ return 1;
}
- return 1;
+ return 0;
}
/*
@@ -1000,8 +1000,8 @@ int dm_calculate_queue_limits(struct dm_table *table,
* Check each device area is consistent with the target's
* overall queue limits.
*/
- if (!ti->type->iterate_devices(ti, device_area_is_valid,
- &ti_limits))
+ if (ti->type->iterate_devices(ti, device_area_is_invalid,
+ &ti_limits))
return -EINVAL;
combine_limits:
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
