[PATCH] Emulate BLKRRPART on device-mapper

[PATCH] Emulate BLKRRPART on device-mapper

[Nikanth Karthikesan]

From: Hannes Reinecke <hare@suse.de>
Subject: Emulate BLKRRPART on device-mapper

Partitions on device-mapper devices are managed by kpartx (if at
all). So if we were just to send out a 'change' event if someone
called BLKRRPART on these devices, kpartx will be triggered via udev
and can manage the partitions accordingly.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>

---

Index: linux-2.6-dm/drivers/md/dm.c
===================================================================
--- linux-2.6-dm.orig/drivers/md/dm.c
+++ linux-2.6-dm/drivers/md/dm.c
@@ -394,19 +394,25 @@ static int dm_blk_ioctl(struct block_dev
 	if (!map || !dm_table_get_size(map))
 		goto out;
 
-	/* We only support devices that have a single target */
-	if (dm_table_get_num_targets(map) != 1)
-		goto out;
-
-	tgt = dm_table_get_target(map, 0);
-
 	if (dm_suspended(md)) {
 		r = -EAGAIN;
 		goto out;
 	}
 
-	if (tgt->type->ioctl)
-		r = tgt->type->ioctl(tgt, cmd, arg);
+	if (cmd == BLKRRPART) {
+		/* Emulate Re-read partitions table */
+		kobject_uevent(&disk_to_dev(md->disk)->kobj, KOBJ_CHANGE);
+		r = 0;
+	} else {
+		/* We only support devices that have a single target */
+		if (dm_table_get_num_targets(map) != 1)
+			goto out;
+
+		tgt = dm_table_get_target(map, 0);
+
+		if (tgt->type->ioctl)
+			r = tgt->type->ioctl(tgt, cmd, arg);
+	}
 
 out:
 	dm_table_put(map);

Re: [PATCH] Emulate BLKRRPART on device-mapper

[Alasdair G Kergon]

On Wed, Jul 08, 2009 at 02:14:50PM +0530, Nikanth Karthikesan wrote:
> From: Hannes Reinecke <hare@suse.de>
> Subject: Emulate BLKRRPART on device-mapper
> 
> Partitions on device-mapper devices are managed by kpartx (if at
> all). So if we were just to send out a 'change' event if someone
> called BLKRRPART on these devices, kpartx will be triggered via udev
> and can manage the partitions accordingly.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>
 
Please could I have a 'Tested-by' for this one?

I agree with the idea, but I need some evidence that this specific patch
works correctly.  (The block layer is supposed to handle that class
of ioctls itself and I didn't think dm saw them.)

Alasdair

[PATCH][RFC] Emulate BLKRRPART on device-mapper

[Milan Broz]

Alasdair G Kergon wrote:
> On Wed, Jul 08, 2009 at 02:14:50PM +0530, Nikanth Karthikesan wrote:
>   
>> From: Hannes Reinecke <hare@suse.de>
>> Subject: Emulate BLKRRPART on device-mapper
>>
>> Partitions on device-mapper devices are managed by kpartx (if at
>> all). So if we were just to send out a 'change' event if someone
>> called BLKRRPART on these devices, kpartx will be triggered via udev
>> and can manage the partitions accordingly.
>>
>>     
> Please could I have a 'Tested-by' for this one?
>   
I am afraid that this patch cannot work, BLRRPART never reach this code.
I tried another idea - or is there better way how to achieve that?

Milan
---

From: Milan Broz <mbroz@redhat.com>

Add genhd flag requesting notification of partition changes only.

This patch provides notification mechanism which allows handle partition
code in userspace.

If the BLKRRPART ioctl arrives and GENHD_FL_PARTITION_CHANGE_NOTIFY
is set, just send uevent and ignore in-kernel partitioning code.

This is useful e.g. for device-mapper devices, which can use kpartx
or similar tool in udev rules.

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 block/ioctl.c         |    3 ++-
 drivers/md/dm.c       |    1 +
 fs/partitions/check.c |    6 ++++++
 include/linux/genhd.h |    8 ++++++++
 4 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 500e4c7..bce793f 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -101,7 +101,8 @@ static int blkdev_reread_part(struct block_device *bdev)
 	struct gendisk *disk = bdev->bd_disk;
 	int res;
 
-	if (!disk_partitionable(disk) || bdev != bdev->bd_contains)
+	if (!disk_userspace_partitions(disk) &&
+	    (!disk_partitionable(disk) || bdev != bdev->bd_contains))
 		return -EINVAL;
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 9acd54a..1186ce1 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1791,6 +1791,7 @@ static struct mapped_device *alloc_dev(int minor)
 	md->disk->queue = md->queue;
 	md->disk->private_data = md;
 	sprintf(md->disk->disk_name, "dm-%d", minor);
+	md->disk->flags |= GENHD_FL_PARTITION_CHANGE_NOTIFY;
 	add_disk(md->disk);
 	format_dev_t(md->name, MKDEV(_major, minor));
 
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index ea4e6cb..bb42c44 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -521,6 +521,12 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
 	struct parsed_partitions *state;
 	int p, highest, res;
 
+	/* partitions handled in userspace, just send change event */
+	if (disk_userspace_partitions(disk)) {
+		kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
+		return 0;
+	}
+
 	if (bdev->bd_part_count)
 		return -EBUSY;
 	res = invalidate_partition(disk, 0);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 45fc320..a241bd6 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -116,6 +116,9 @@ struct hd_struct {
 #define GENHD_FL_EXT_DEVT			64 /* allow extended devt */
 #define GENHD_FL_NATIVE_CAPACITY		128
 
+/* notify udev instead of use in-kernel partitioning */
+#define GENHD_FL_PARTITION_CHANGE_NOTIFY	256
+
 #define BLK_SCSI_MAX_CMDS	(256)
 #define BLK_SCSI_CMD_PER_LONG	(BLK_SCSI_MAX_CMDS / (sizeof(long) * 8))
 
@@ -180,6 +183,11 @@ static inline struct gendisk *part_to_disk(struct hd_struct *part)
 	return NULL;
 }
 
+static inline bool disk_userspace_partitions(struct gendisk *disk)
+{
+	return (disk->flags & GENHD_FL_PARTITION_CHANGE_NOTIFY) ? 1 : 0;
+}
+
 static inline int disk_max_parts(struct gendisk *disk)
 {
 	if (disk->flags & GENHD_FL_EXT_DEVT)

Re: [PATCH][RFC] Emulate BLKRRPART on device-mapper

[Hannes Reinecke]

Milan Broz wrote:
> Alasdair G Kergon wrote:
>> On Wed, Jul 08, 2009 at 02:14:50PM +0530, Nikanth Karthikesan wrote:
>>   
>>> From: Hannes Reinecke <hare@suse.de>
>>> Subject: Emulate BLKRRPART on device-mapper
>>>
>>> Partitions on device-mapper devices are managed by kpartx (if at
>>> all). So if we were just to send out a 'change' event if someone
>>> called BLKRRPART on these devices, kpartx will be triggered via udev
>>> and can manage the partitions accordingly.
>>>
>>>     
>> Please could I have a 'Tested-by' for this one?
>>   
> I am afraid that this patch cannot work, BLRRPART never reach this code.
> I tried another idea - or is there better way how to achieve that?
> 
Hey, that is cool. And what's more, that's exactly what I need for another
pet-project of mine, switching off the in-kernel partitioning code altogether.

But there are some comments, see below.

> Milan
> ---
> 
> From: Milan Broz <mbroz@redhat.com>
> 
> Add genhd flag requesting notification of partition changes only.
> 
> This patch provides notification mechanism which allows handle partition
> code in userspace.
> 
> If the BLKRRPART ioctl arrives and GENHD_FL_PARTITION_CHANGE_NOTIFY
> is set, just send uevent and ignore in-kernel partitioning code.
> 
> This is useful e.g. for device-mapper devices, which can use kpartx
> or similar tool in udev rules.
> 
> Signed-off-by: Milan Broz <mbroz@redhat.com>
> ---
>  block/ioctl.c         |    3 ++-
>  drivers/md/dm.c       |    1 +
>  fs/partitions/check.c |    6 ++++++
>  include/linux/genhd.h |    8 ++++++++
>  4 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 500e4c7..bce793f 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -101,7 +101,8 @@ static int blkdev_reread_part(struct block_device *bdev)
>  	struct gendisk *disk = bdev->bd_disk;
>  	int res;
>  
> -	if (!disk_partitionable(disk) || bdev != bdev->bd_contains)
> +	if (!disk_userspace_partitions(disk) &&
> +	    (!disk_partitionable(disk) || bdev != bdev->bd_contains))
>  		return -EINVAL;
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EACCES;
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 9acd54a..1186ce1 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1791,6 +1791,7 @@ static struct mapped_device *alloc_dev(int minor)
>  	md->disk->queue = md->queue;
>  	md->disk->private_data = md;
>  	sprintf(md->disk->disk_name, "dm-%d", minor);
> +	md->disk->flags |= GENHD_FL_PARTITION_CHANGE_NOTIFY;
>  	add_disk(md->disk);
>  	format_dev_t(md->name, MKDEV(_major, minor));
>  
> diff --git a/fs/partitions/check.c b/fs/partitions/check.c
> index ea4e6cb..bb42c44 100644
> --- a/fs/partitions/check.c
> +++ b/fs/partitions/check.c
> @@ -521,6 +521,12 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
>  	struct parsed_partitions *state;
>  	int p, highest, res;
>  
> +	/* partitions handled in userspace, just send change event */
> +	if (disk_userspace_partitions(disk)) {
> +		kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
> +		return 0;
> +	}
> +
Wrong. If you do it here, you'll never be able to catch any size changes
of the disk. You'll have to move it to after the 'bdev->bd_invalidated = 0'
line.

>  	if (bdev->bd_part_count)
>  		return -EBUSY;
>  	res = invalidate_partition(disk, 0);
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 45fc320..a241bd6 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -116,6 +116,9 @@ struct hd_struct {
>  #define GENHD_FL_EXT_DEVT			64 /* allow extended devt */
>  #define GENHD_FL_NATIVE_CAPACITY		128
>  
> +/* notify udev instead of use in-kernel partitioning */
> +#define GENHD_FL_PARTITION_CHANGE_NOTIFY	256
> +
I would suggest renaming it to GENHD_FL_USERSPACE_PARTITIONS, as this is
more in line with the function of the flag.
Plus I have a patch making use of it :-)

I'll send an adapted patchset.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

Re: [PATCH][RFC] Emulate BLKRRPART on device-mapper

[Milan Broz]

Hannes Reinecke wrote:
>> +	/* partitions handled in userspace, just send change event */
>> +	if (disk_userspace_partitions(disk)) {
>> +		kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
>> +		return 0;
>> +	}
>> +
> Wrong. If you do it here, you'll never be able to catch any size changes
> of the disk. You'll have to move it to after the 'bdev->bd_invalidated = 0'
> line.

Yes, for non-DM devices which want to use that flag.

For device-mapper device, size can change only by mapping table change
which always generate change uevent.

> I would suggest renaming it to GENHD_FL_USERSPACE_PARTITIONS, as this is
> more in line with the function of the flag.

no problem here:-)

Milan

Re: [PATCH][RFC] Emulate BLKRRPART on device-mapper

[Hannes Reinecke]

Milan Broz wrote:
> Hannes Reinecke wrote:
>>> +	/* partitions handled in userspace, just send change event */
>>> +	if (disk_userspace_partitions(disk)) {
>>> +		kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
>>> +		return 0;
>>> +	}
>>> +
>> Wrong. If you do it here, you'll never be able to catch any size changes
>> of the disk. You'll have to move it to after the 'bdev->bd_invalidated = 0'
>> line.
> 
> Yes, for non-DM devices which want to use that flag.
> 
> For device-mapper device, size can change only by mapping table change
> which always generate change uevent.
> 
Okay. But the whole point of my objections is to make the flag useable
for the general populace, so that my second patch can take advantage
of it.

>> I would suggest renaming it to GENHD_FL_USERSPACE_PARTITIONS, as this is
>> more in line with the function of the flag.
> 
> no problem here:-)
Hmm. I've send an updated patch, but it seems to be stuck in the mailqueue
somewhere.
The second patch went through, though ... curious.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)