Messages in this thread | | | From | Heinz Mauelshagen <> | Subject | Re: defvs patch v84 for linux 2.2.0-pre9 bugfix | Date | Sun, 24 Jan 1999 5:09:10 MET |
| |
> > Oops. I see you posted separately to me and linux-kernel. I'm > forwarding my reply for the benefit of kernel readers. > > Heinz Mauelshagen writes: > > > > I found a little bug, which seems to cause non standard block devices > > > > beeing _not_ mountable any more. > > > > In detail, my logical volume manager block devices don't work. > > > > The block device specials are created by lvm user commands. > > > > > > > > I think any software creating block specials should fail with the > > > > v84 code in super.c, where your patch looks like: > > > > > > > > @@ -1067,8 +1079,9 @@ > > > > if (MAJOR(dev) >= MAX_BLKDEV) > > > > goto dput_and_out; > > > > > > > > - retval = -ENOTBLK; > > > > - dummy.f_op = get_blkfops(MAJOR(dev)); > > > > + retval = devfs_fill_file (inode, &dummy, NULL); > > > > + if ( !retval && !S_ISBLK (inode->i_mode) ) retval = -ENOTBLK; > > > > + if (retval < 0) dummy.f_op = get_blkfops(MAJOR(dev)); > > > > if (!dummy.f_op) > > > > goto dput_and_out; > > > > > > Can you please explain why you think my patch is not working? > > > > Please see below. > > > > > > > > Also, please send me the output of ls -lF on the device node you are > > > trying to mount. > > > > brw-r----- 1 root root 58, 1 Jan 24 03:48 /dev/vg00/u1 > > Has this been created from user space?
Yes, it has.
> > > > I do have one theory why my patch is failing. See the line: > > > if ( !retval && !S_ISBLK (inode->i_mode) ) retval = -ENOTBLK; > > > ^^ > > > If the device is non-standard (i.e. the device node was created with > > > mknod(2) and not internally by the driver calling devfs_register()), > > > *and* the previous contents of the inode were for a block device, then > > > the condition fails. This means that reval will not be set to -ENOTBLK > > > and the fops are subsequently not filled. Hence you can't mount. > > > This is a braino on my part. > > > > > > I suggest changing the "&&" to a "||". This should fix your problem > > > and also provides the desired behaviour. Please let me know if this > > > works for you. > > > > That's o.k. for me, but why do you test for block device again anyway? > > It's already tested a couple of lines above in linux/fs/super.c based on > > the actual dentry (line 1343). > > Ah, I had overlook that. Thanks for pointing that out. That whole line > does no good and only does harm. > > > > > Patch against stock linux-2.2.0-pre9/fs/super.c to fix the problem > > > > follows: > > > > > > In future, could you please provide patches against kernel+devfs, > > > rather than providing a replacement devfs patch? This makes it easier > > > for me to understand what you're doing and also makes it easier to > > > integrate a patch. > > > > I thought i had done this 8*( > > No, you provided a patch against stock linux-2.2.0-pre9 (as you > said). What it easier for me is a patch against stock > linux-2.2.0pre9+devfs-patch-v86. >
Understood.
> > > > @@ -1067,8 +1079,9 @@ > > > > if (MAJOR(dev) >= MAX_BLKDEV) > > > > goto dput_and_out; > > > > > > > > - retval = -ENOTBLK; > > > > - dummy.f_op = get_blkfops(MAJOR(dev)); > > > > + if ( !( retval = devfs_fill_file (inode, &dummy, NULL))) > > > > + retval = -ENOTBLK; > > > > + if ( retval < 0) dummy.f_op = get_blkfops(MAJOR(dev)); > > > > if (!dummy.f_op) > > > > goto dput_and_out; > > > > > > > > > > What you have done here is removed the check for a > > > block device returned from devfs. So now, the user could attempt to > > > mount a character device. I don't think this is a good fix to the > > > problem. See above for a suggested fix. > > > > No, i don't think so because it has already been tested before > > (see my arguments above). > > The code is more subtle than I first thought. Upon further > examination, I see that your patch will break mounting devices which > do not register themselves with register_blkdev() when the > "devfs=only" boot option is passed. For example, the SCSI disc driver > calls devfs_register_blkdev() which is a no-op when "devfs=only".
O.k., have to look deaper into that... Yes, you are right. My 2 hours of devfs studies and hacking are not enough 8*)))
> > I have come up with a different fix which I think is correct. I've > appended a fresh patch against stock 2.2.0-pre9. Please try this and > let me know how it goes. >
Will test it soon.
> > P.S.: i found another flaw. If someone does chmod a devfs block special > > sys_chmod/sys_fchmod in linux/fs/open.c updates the dentry but > > devfs internal cached mode for the inode never changes. > > If the block special contains a filesystem and you do mount/umount > > it, your changed permissions are gone afterwards. > > This is intentional. Permission changes are > filesystem-specific. Imagine you have a chroot() gaol and you want to > change some permissions there but you don't want to affect any other > mounted devfs'. The current behaviour supports this.
That's not my problem.
Different permissions in different mounted devfs _not_ affecting each other if changes are going on in one of them are o.k.
> > When you unmount a devfs, you lose all the permissions that were > cached. The current way to fix this is to manaully change the > permissions when you mount a devfs. >
If i do mount a filesystem using a block device special in one of the mounted devfs i _don't_ want the permissions be changed by that mount via the prefiously mentioned devfs_fill_file() scenario. The permissions should be the same as before the mount.
> In the long term, devfs will peek through to the underlying disc-based > filesystem and use the permissions specified there. If you change > permissions in devfs, the changes will be written back to the > underlying disc-based filesystem. >
Great!
Regards, Heinz
--
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Systemmanagement Entwicklungsbereich 2 Deutsche Telekom AG Entwicklungszentrum Darmstadt Heinz Mauelshagen Otto-Roehm-Strasse 71c Postfach 10 05 41 mge@ez-darmstadt.telekom.de 64205 Darmstadt Germany +49 6151 886-425 FAX-386 =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |