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 <[email protected]>
---
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;
On Sat, Jan 10, 2009 at 06:57, Roland Dreier <[email protected]> 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
> > 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.
On Sat, Jan 10, 2009 at 07:11, Roland Dreier <[email protected]> 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
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 <[email protected]>
---
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);
On Sat, Jan 10, 2009 at 07:27, Roland Dreier <[email protected]> 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
> 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)?
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
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)
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