| Message ID | 4A65E912.6020706@redhat.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
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
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 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
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
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)