Proposed fix for 'hal sometimes creates volumes for raw device and partitions'

Kay Sievers kay.sievers at vrfy.org
Tue Mar 21 06:49:08 PST 2006


On Wed, Mar 08, 2006 at 04:12:29PM +0100, Kay Sievers wrote:
> On Wed, Mar 08, 2006 at 11:40:25AM +0100, Martin Pitt wrote:
> > I finally gave a stab to the bug that hal creates a volume for the raw
> > device (/dev/sda) even if there are partitions on it, so that you end
> > up with getting both /dev/sda and /dev/sda1 automounted. [1] [2]
> > 
> > I noticed that probe-{storage,volume} never really calls
> > volume_id_probe_msdos_part_table(). It just calls
> > volume_id_probe_all() and even expects that this might return
> > VOLUME_ID_PARTITIONTABLE, but volume_id_probe_all() does not probe for
> > partition tables.
> > 
> > So I made volume_id_probe_all() call volume_id_probe_msdos_part_table()
> > before trying to probe file systems, and voila, it works just great.
> > It even seems to be the intended semantics.
> > 
> > Does anyone see any problem with this approach?
> 
> Yeah! The only sane way is to have the kernel to tell you that it has
> finished creating partitions, means "scanning the partition table" and not
> create any volumes in HAL until that.

The attached patch to the kernel delays all events belonging to a block
device until the partition table is completely scanned and all devices
(disk and partitions) are already created by the kernel. The disk device
in sysfs will have an attribute "partitioned", which will tell if there
are child devices to expect.

That way we can just skip probing for a filesystem on the disk device
if the event for the disk tells us that we can expect partition events.

Would be nice, if someone can play around with this and see if that can
work to properly solve that old and nasty problem.

Thanks,
Kay
-------------- next part --------------
diff --git a/block/genhd.c b/block/genhd.c
index db57546..0adc9e9 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -436,6 +436,10 @@ static ssize_t disk_range_read(struct ge
 {
 	return sprintf(page, "%d\n", disk->minors);
 }
+static ssize_t disk_partitioned_read(struct gendisk * disk, char *page)
+{
+	return sprintf(page, "%d\n", disk->has_partitions);
+}
 static ssize_t disk_removable_read(struct gendisk * disk, char *page)
 {
 	return sprintf(page, "%d\n",
@@ -481,6 +485,10 @@ static struct disk_attribute disk_attr_r
 	.attr = {.name = "range", .mode = S_IRUGO },
 	.show	= disk_range_read
 };
+static struct disk_attribute disk_attr_partitioned = {
+	.attr = {.name = "partitioned", .mode = S_IRUGO },
+	.show	= disk_partitioned_read
+};
 static struct disk_attribute disk_attr_removable = {
 	.attr = {.name = "removable", .mode = S_IRUGO },
 	.show	= disk_removable_read
@@ -498,6 +506,7 @@ static struct attribute * default_attrs[
 	&disk_attr_uevent.attr,
 	&disk_attr_dev.attr,
 	&disk_attr_range.attr,
+	&disk_attr_partitioned.attr,
 	&disk_attr_removable.attr,
 	&disk_attr_size.attr,
 	&disk_attr_stat.attr,
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index f924f45..7b47d88 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -310,7 +310,9 @@ void delete_partition(struct gendisk *di
 	p->ios[0] = p->ios[1] = 0;
 	p->sectors[0] = p->sectors[1] = 0;
 	devfs_remove("%s/part%d", disk->devfs_name, part);
-	kobject_unregister(&p->kobj);
+	kobject_uevent(&p->kobj, KOBJ_REMOVE);
+	kobject_del(&p->kobj);
+	kobject_put(&p->kobj);
 }
 
 void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len)
@@ -336,8 +338,12 @@ void add_partition(struct gendisk *disk,
 		snprintf(p->kobj.name,KOBJ_NAME_LEN,"%s%d",disk->kobj.name,part);
 	p->kobj.parent = &disk->kobj;
 	p->kobj.ktype = &ktype_part;
-	kobject_register(&p->kobj);
+	kobject_init(&p->kobj);
+	kobject_add(&p->kobj);
+	if (!disk->part_uevent_supress)
+		kobject_uevent(&p->kobj, KOBJ_ADD);
 	disk->part[part-1] = p;
+	disk->has_partitions = 1;
 }
 
 static char *make_block_name(struct gendisk *disk)
@@ -373,6 +379,8 @@ void register_disk(struct gendisk *disk)
 {
 	struct block_device *bdev;
 	char *s;
+	int i;
+	struct hd_struct *p;
 	int err;
 
 	strlcpy(disk->kobj.name,disk->disk_name,KOBJ_NAME_LEN);
@@ -382,14 +390,13 @@ void register_disk(struct gendisk *disk)
 		*s = '!';
 	if ((err = kobject_add(&disk->kobj)))
 		return;
-	disk_sysfs_symlinks(disk);
-	kobject_uevent(&disk->kobj, KOBJ_ADD);
 
+	disk_sysfs_symlinks(disk);
 	/* No minors to use for partitions */
 	if (disk->minors == 1) {
 		if (disk->devfs_name[0] != '\0')
 			devfs_add_disk(disk);
-		return;
+		goto exit;
 	}
 
 	/* always add handle for the whole disk */
@@ -397,16 +404,31 @@ void register_disk(struct gendisk *disk)
 
 	/* No such device (e.g., media were just removed) */
 	if (!get_capacity(disk))
-		return;
+		goto exit;
 
 	bdev = bdget_disk(disk, 0);
 	if (!bdev)
-		return;
+		goto exit;
 
+	/* scan partition table, but supress uevents */
 	bdev->bd_invalidated = 1;
+	disk->part_uevent_supress = 1;
 	if (blkdev_get(bdev, FMODE_READ, 0) < 0)
-		return;
+		goto exit;
 	blkdev_put(bdev);
+	disk->part_uevent_supress = 0;
+
+exit:
+	kobject_uevent(&disk->kobj, KOBJ_ADD);
+
+	/* partition events are supressed while scanning the partition table */
+	if (disk->has_partitions)
+		for (i = 1; i < disk->minors; i++) {
+			p = disk->part[i-1];
+			if (!p || !p->nr_sects)
+				continue;
+			kobject_uevent(&p->kobj, KOBJ_ADD);
+		}
 }
 
 int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
@@ -420,6 +442,7 @@ int rescan_partitions(struct gendisk *di
 	if (res)
 		return res;
 	bdev->bd_invalidated = 0;
+	disk->has_partitions = 0;
 	for (p = 1; p < disk->minors; p++)
 		delete_partition(disk, p);
 	if (disk->fops->revalidate_disk)
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index eef5ccd..8f0d86d 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -104,6 +104,8 @@ struct gendisk {
                                          * disks that can't be partitioned. */
 	char disk_name[32];		/* name of major driver */
 	struct hd_struct **part;	/* [indexed by minor] */
+	int part_uevent_supress;
+	int has_partitions;
 	struct block_device_operations *fops;
 	struct request_queue *queue;
 	void *private_data;


More information about the hal mailing list