[PATCH] block: Fix register_disk() when name has '/' in it

[PATCH] block: Fix register_disk() when name has '/' in it

[Roland Dreier]

Commit 3ada8b7e ("block: struct device - replace bus_id with dev_name(),
dev_set_name()") deleted the code in register_disk() that changed a '/'
to a '!' in the device name when registering a disk.  This leads to
amusing problems with disks that have '/' in their names -- for example
a failure to boot with the root partition on a cciss device, even though
the kernel says it knows about the root device:

    VFS: Cannot open root device "cciss/c0d0p6" or unknown-block(0,0)
    Please append a correct "root=" boot option; here are the available partitions:
    6800        71652960 cciss/c0d0 driver: cciss
      6802               1 cciss/c0d0p2
      6805         2931831 cciss/c0d0p5
      6806        34354908 cciss/c0d0p6
    6810        71652960 cciss/c0d1 driver: cciss

Fix this by bringing back the code to change '/' to '!' in disk names
when registering a disk.

Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
 fs/partitions/check.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 6d72024..474d73e 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -448,11 +448,19 @@ void register_disk(struct gendisk *disk)
 	struct block_device *bdev;
 	struct disk_part_iter piter;
 	struct hd_struct *part;
+	char dname[DISK_NAME_LEN];
+	char *s;
 	int err;
 
 	ddev->parent = disk->driverfs_dev;
 
-	dev_set_name(ddev, disk->disk_name);
+	/* ewww... some of these buggers have / in the name... */
+	strcpy(dname, disk->disk_name);
+	s = strchr(dname, '/');
+	if (s)
+		*s = '!';
+
+	dev_set_name(ddev, dname);
 
 	/* delay uevents, until we scanned partition table */
 	ddev->uevent_suppress = 1;

Re: [PATCH] block: Fix register_disk() when name has '/' in it

[Kay Sievers]

On Sat, Jan 10, 2009 at 06:57, Roland Dreier <rdreier@cisco.com> wrote:
> Commit 3ada8b7e ("block: struct device - replace bus_id with dev_name(),
> dev_set_name()") deleted the code in register_disk() that changed a '/'
> to a '!' in the device name when registering a disk.

Hmm, this is done in the core, for all devices since a while:
  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=9f255651fb41c111ee35a2ae632df8ce9bd61def

We should find what is going wrong here, instead of putting that code back.

Kay

Re: [PATCH] block: Fix register_disk() when name has '/' in it

[Roland Dreier]

 > > Commit 3ada8b7e ("block: struct device - replace bus_id with dev_name(),
 > > dev_set_name()") deleted the code in register_disk() that changed a '/'
 > > to a '!' in the device name when registering a disk.
 > 
 > Hmm, this is done in the core, for all devices since a while:
 >   http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=9f255651fb41c111ee35a2ae632df8ce9bd61def
 > 
 > We should find what is going wrong here, instead of putting that code back.

The commit you point to is for kobject_set_name() ... but dev_set_name()
is still just setting dev->bus_id (at least in Linus's current tree).

If you want I can send a patch putting the conversion into
dev_set_name() for now instead.

 - R.

Re: [PATCH] block: Fix register_disk() when name has '/' in it

[Kay Sievers]

On Sat, Jan 10, 2009 at 07:11, Roland Dreier <rdreier@cisco.com> wrote:
>  > > Commit 3ada8b7e ("block: struct device - replace bus_id with dev_name(),
>  > > dev_set_name()") deleted the code in register_disk() that changed a '/'
>  > > to a '!' in the device name when registering a disk.
>  >
>  > Hmm, this is done in the core, for all devices since a while:
>  >   http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=9f255651fb41c111ee35a2ae632df8ce9bd61def
>  >
>  > We should find what is going wrong here, instead of putting that code back.
>
> The commit you point to is for kobject_set_name() ... but dev_set_name()
> is still just setting dev->bus_id (at least in Linus's current tree).

Ah, right, it will only work after dev_name() returns the kobject
name, which it already does with the patches in our queue, so we
missed that. Sorry.

> If you want I can send a patch putting the conversion into
> dev_set_name() for now instead.

Sounds good, we can remove that with the final conversion and the
removal of the bus_id field.

Thanks,
Kay

[PATCH] driver core: Convert '/' to '!' in dev_set_name()

[Roland Dreier]

Commit 3ada8b7e ("block: struct device - replace bus_id with dev_name(),
dev_set_name()") deleted the code in register_disk() that changed a '/'
to a '!' in the device name when registering a disk, but dev_set_name()
does not perform this conversion.

This leads to amusing problems with disks that have '/' in their names:
for example a failure to boot with the root partition on a cciss device,
even though the kernel says it knows about the root device:

    VFS: Cannot open root device "cciss/c0d0p6" or unknown-block(0,0)
    Please append a correct "root=" boot option; here are the available partitions:
    6800        71652960 cciss/c0d0 driver: cciss
      6802               1 cciss/c0d0p2
      6805         2931831 cciss/c0d0p5
      6806        34354908 cciss/c0d0p6
    6810        71652960 cciss/c0d1 driver: cciss

Fix this by adding code to change '/' to '!' in dev_set_name() to handle
this until dev_set_name() is converted to use kobject_set_name().

Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
OK, this is tested to work on my system as well.

 drivers/base/core.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 8079afc..55e5309 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -777,10 +777,16 @@ static void device_remove_class_symlinks(struct device *dev)
 int dev_set_name(struct device *dev, const char *fmt, ...)
 {
 	va_list vargs;
+	char *s;
 
 	va_start(vargs, fmt);
 	vsnprintf(dev->bus_id, sizeof(dev->bus_id), fmt, vargs);
 	va_end(vargs);
+
+	/* ewww... some of these buggers have / in the name... */
+	while ((s = strchr(dev->bus_id, '/')))
+		*s = '!';
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(dev_set_name);

Re: [PATCH] driver core: Convert '/' to '!' in dev_set_name()

[Kay Sievers]

On Sat, Jan 10, 2009 at 07:27, Roland Dreier <rdreier@cisco.com> wrote:
> Commit 3ada8b7e ("block: struct device - replace bus_id with dev_name(),
> dev_set_name()") deleted the code in register_disk() that changed a '/'
> to a '!' in the device name when registering a disk, but dev_set_name()
> does not perform this conversion.
>
> This leads to amusing problems with disks that have '/' in their names:
> for example a failure to boot with the root partition on a cciss device,
> even though the kernel says it knows about the root device:
>
>    VFS: Cannot open root device "cciss/c0d0p6" or unknown-block(0,0)
>    Please append a correct "root=" boot option; here are the available partitions:
>    6800        71652960 cciss/c0d0 driver: cciss
>      6802               1 cciss/c0d0p2
>      6805         2931831 cciss/c0d0p5
>      6806        34354908 cciss/c0d0p6
>    6810        71652960 cciss/c0d1 driver: cciss
>
> Fix this by adding code to change '/' to '!' in dev_set_name() to handle
> this until dev_set_name() is converted to use kobject_set_name().

Great! Thanks a lot for fixing this,
Kay

Re: [PATCH] driver core: Convert '/' to '!' in dev_set_name()

[Roland Dreier]

 > Great! Thanks a lot for fixing this,

I assume you will make sure this gets upstream (since the problem causes
some boxes with root on cciss to panic on boot, and possibly breaks
other stuff)?

Re: [PATCH] driver core: Convert '/' to '!' in dev_set_name()

[Greg KH]

On Sat, Jan 10, 2009 at 10:52:15PM -0800, Roland Dreier wrote:
>  > Great! Thanks a lot for fixing this,
> 
> I assume you will make sure this gets upstream (since the problem causes
> some boxes with root on cciss to panic on boot, and possibly breaks
> other stuff)?

Yes, I'll queue it up on Monday.

thanks,

greg k-h

Re: [PATCH] driver core: Convert '/' to '!' in dev_set_name()

[Roland Dreier]

Greg, I still don't see this upstream; I think it should be merged
expeditiously, since it makes some boxes panic on boot.  (One can
generally recover by using dev=<major:minor> instead of dev=/dev/foo/bar
but I still think having tester's boxes panic on boot because of a known
fixed bug is not ideal)

Re: [PATCH] driver core: Convert '/' to '!' in dev_set_name()

[Greg KH]

On Thu, Jan 15, 2009 at 02:49:26PM -0800, Roland Dreier wrote:
> Greg, I still don't see this upstream; I think it should be merged
> expeditiously, since it makes some boxes panic on boot.  (One can
> generally recover by using dev=<major:minor> instead of dev=/dev/foo/bar
> but I still think having tester's boxes panic on boot because of a known
> fixed bug is not ideal)

Sorry, it's in my to-apply queue, it was behind these huge -stable
releases that i just got out today.

Should get to it tonight or tomorrow, don't worry, it's not forgotten.

thanks,

greg "my inbox is a mess" k-h