Add MOUNT_STATUS ioctl to cdrom device
PATCH 1/1 Add MOUNT_STATUS ioctl to cdrom device
Hi,
This patch add a new ioctl MOUNT_STATUS to the 2.4 kernel's cdrom device. It'll be used as an API to query the mount status of a cdrom :-
CDROM_MOUNT_STATUS
Return :-
0 = not mount.
1 = mounted, but not in-use. It is ok to umount.
2 = busy. Umount will result in getting EBUSY.
<0 = error.
This same functionality can be done in user-space, like the way 'fuser' do, but doing so in kernel-space is simpler and faster. The ioctl can (will) be used by automount daemon to query the device to see if it is in use or not and lock/unlock the drive as needed. This will make the cdrom "eject" button work as it should be (as auto-umount button). Novice GUI users usually wonder why they can't eject cds even when the discs are not in use.
An experimental implementation has been done and working on my machine. I'm completely new to kernel hacking so please make comments about this patch and whether it's justified or plain stupid to add another ioctl for this feature cause I really have no idea.
drivers/cdrom/cdrom.c +22 -0
@@ -264,6 +264,8 @@#include <linux/sysctl.h> #include <linux/proc_fs.h> #include <linux/init.h> +#include <linux/namespace.h> +#include <linux/mount.h> #include <asm/fcntl.h> #include <asm/segment.h>
@@ -1622,6 +1624,26 @@return cdo->lock_door (cdi, arg); } +case CDROM_MOUNT_STATUS : { +struct super_block *sb =get_super (dev); +if (sb ==NULL )return -EINVAL ; +down_read (¤t->namespace->sem); +struct vfsmount *mnt =NULL ; +struct list_head *p; +list_for_each (p, ¤t->namespace->list) { + struct vfsmount *m = list_entry(p, struct vfsmount, mnt_list); +if (sb == m->mnt_sb) { + mnt = m;break ; + } + } +up_read (¤t->namespace->sem); +drop_super (sb); +int mstat =0 ;/* 0 not mounted, 1 umount ok, 2 umount EBUSY */ +if (mnt) mstat =1 + (atomic_read (&mnt->mnt_count) >1 ); +cdinfo (CD_DO_IOCTL ,"mount status(%s) = %d\n" , mnt->mnt_devname, mstat); +return mstat; + } +case CDROM_DEBUG : {if (!capable (CAP_SYS_ADMIN ))return -EACCES ;
include/linux/cdrom.h +1 -0
@@ -127,6 +127,7 @@#define CDROM_LOCKDOOR 0x5329/* lock or unlock door */ #define CDROM_DEBUG 0x5330/* Turn debug messages on/off */ #define CDROM_GET_CAPABILITY 0x5331/* get capabilities */ +#define CDROM_MOUNT_STATUS 0x5332/* is it mounted? can umount? */ /* Note that scsi/scsi_ioctl.h also uses 0x5382 - 0x5386. * Future CDROM ioctls should be kept below 0x537F
Signature · 5 lines
Samphan Raruenrom, The Open Source Project, National Electronics and Computer Technology Center, National Science and Technology Development Agency, Thailand.
drivers/cdrom/cdrom.c line 268
Quoted 5 lines · Samphan Raruenrom
#include <linux/sysctl.h> #include <linux/proc_fs.h> #include <linux/init.h>drivers/cdrom/cdrom.c +2
-0+#include <linux/namespace.h> +#include <linux/mount.h>
your mailer is screwed.
Quoted 9 lines
drivers/cdrom/cdrom.c +8
-0+case CDROM_MOUNT_STATUS : { +struct super_block *sb =get_super (dev); +if (sb ==NULL )return -EINVAL ; +down_read (¤t->namespace->sem); +struct vfsmount *mnt =NULL ; +struct list_head *p; +list_for_each (p, ¤t->namespace->list) { + struct vfsmount *m = list_entry(p, struct vfsmount, mnt_list);
drivers/cdrom/cdrom.c line 1644
Quoted 10 lines
drivers/cdrom/cdrom.c +10
-0+if (sb == m->mnt_sb) { + mnt = m;break ; + } + } +up_read (¤t->namespace->sem); +drop_super (sb); +int mstat =0 ;/* 0 not mounted, 1 umount ok, 2 umount EBUSY */ +if (mnt) mstat =1 + (atomic_read (&mnt->mnt_count) >1 ); +cdinfo (CD_DO_IOCTL ,"mount status(%s) = %d\n" , mnt->mnt_devname, mstat); +return mstat;
WTF? This is not only a layering violation but also totally racy.
Rejected.
Quoted 11 lines · Christoph Hellwig
Quoted 11 lines
drivers/cdrom/cdrom.c +11
-0+case CDROM_MOUNT_STATUS : { +struct super_block *sb =get_super (dev); +if (sb ==NULL )return -EINVAL ; +down_read (¤t->namespace->sem); +struct vfsmount *mnt =NULL ; +struct list_head *p; +list_for_each (p, ¤t->namespace->list) { + struct vfsmount *m = list_entry(p, struct vfsmount, mnt_list); +if (sb == m->mnt_sb) { + mnt = m;break ; + }
Quoted 8 lines
drivers/cdrom/cdrom.c line 1644
Quoted 7 lines
drivers/cdrom/cdrom.c +7
-0+ } +up_read (¤t->namespace->sem); +drop_super (sb); +int mstat =0 ;/* 0 not mounted, 1 umount ok, 2 umount EBUSY */ +if (mnt) mstat =1 + (atomic_read (&mnt->mnt_count) >1 ); +cdinfo (CD_DO_IOCTL ,"mount status(%s) = %d\n" , mnt->mnt_devname, mstat); +return mstat;WTF? This is not only a layering violation but also totally racy.
I'm sorry for issueing this layering violation. I read a guideline that it's easier to submit a patch to add device driver ioctl than inventing something new. It really doesn't belong here.
Could you guide me where else can I place this functionality?
(my random idea)
- fcntl(open("/dev/cdrom", F_MNTSTAT)
- umount2("/dev/cdrom", MS_TEST) // not actually perform
- new system call! e.g. mntstat(open("/dev/cdrom"))
Signature · 5 lines
Samphan Raruenrom, The Open Source Project, National Electronics and Computer Technology Center, National Science and Technology Development Agency, Thailand.
Quoted 3 lines · Samphan Raruenrom
I'm sorry for issueing this layering violation. I read a guideline that it's easier to submit a patch to add device driver ioctl than inventing something new. It really doesn't belong here.
That guideline is wrong. Adding a driver ioctl is slmost always wrong.
Quoted 6 lines
Could you guide me where else can I place this functionality?
(my random idea)
- fcntl(open("/dev/cdrom",F_MNTSTAT)
-umount2("/dev/cdrom", MS_TEST)// not actually perform
- new system call! e.g. mntstat(open("/dev/cdrom"))
In userspace. Or you could tell me what you want to actually archive - your call by itself doesn't make any sense.
Quoted 6 lines · Christoph Hellwig
Quoted 4 lines
(my random idea)
- fcntl(open("/dev/cdrom",F_MNTSTAT)
-umount2("/dev/cdrom", MS_TEST)// not actually perform
- new system call! e.g. mntstat(open("/dev/cdrom"))In userspace. Or you could tell me what you want to actually archive - your call by itself doesn't make any sense.
To get the same result in userspace, it means scanning /proc like fuser, right? Do you think it is ok to do that? Because the daemon (patched magicdev) poll the cdrom device very often (every 2 sec.)
That's why I thought that faster is better (less load to the system) and push the job to kernel-space.
This is what I really want to achive - "enable the eject button, by locking the drive only when the device is really in use, unlock otherwise".
I implement this as a modified version of GNOME magicdev.
http://bugzilla.gnome.org/show_bug.cgi?id=119892
The only visible feature of this new magicdev is that now GNOME users can eject there CDs (the discs' icon will disappear). The eject button now act as 'umount' command.
One new requirement from this new magicdev is the question "will umount failed?". I have no preference on any way to implement it. Should there be the right way to do it, I'll do so. I can think of many way to implement it (including adding a new lazy-lock mode to cdrom device) but since I have no kernel hacking experience, I need everyone advices. Novice users need this 'eject' button after all.
Signature · 5 lines
Samphan Raruenrom, The Open Source Project, National Electronics and Computer Technology Center, National Science and Technology Development Agency, Thailand.
Quoted 11 lines · Samphan Raruenrom
The only visible feature of this new magicdev is that now GNOME users can eject there CDs (the discs' icon will disappear). The eject button now act as 'umount' command.
One new requirement from this new magicdev is the question "will umount failed?". I have no preference on any way to implement it. Should there be the right way to do it, I'll do so. I can think of many way to implement it (including adding a new lazy-lock mode to cdrom device) but since I have no kernel hacking experience, I need everyone advices. Novice users need this 'eject' button after all.
This doesn't make sense at all. Just try the unmount and tell the user if it failed - you can't say whether it will fail before trying.
Quoted 13 lines · Christoph Hellwig
Quoted 11 lines · Samphan Raruenrom
The only visible feature of this new magicdev is that now GNOME users can eject there CDs (the discs' icon will disappear). The eject button now act as 'umount' command.
One new requirement from this new magicdev is the question "will umount failed?". I have no preference on any way to implement it. Should there be the right way to do it, I'll do so. I can think of many way to implement it (including adding a new lazy-lock mode to cdrom device) but since I have no kernel hacking experience, I need everyone advices. Novice users need this 'eject' button after all.
Quoted 3 lines
This doesn't make sense at all. Just try the unmount and tell the user if it failed - you can't say whether it will fail before trying.
Exactly. You poll media events from the drive, and upon an eject request you try and umount it. If it suceeds, you eject the tray. It's pretty simple, really.
This mount status patch is seriously broken and misguided.
Signature · 1 line
Jens Axboe
Quoted 12 lines · Jens Axboe
Quoted 11 lines · Christoph Hellwig
Quoted 10 lines · Samphan Raruenrom
The only visible feature of this new magicdev is that now GNOME users can eject there CDs (the discs' icon will disappear). The eject button now act as 'umount' command. One new requirement from this new magicdev is the question "will umount failed?". I have no preference on any way to implement it. Should there be the right way to do it, I'll do so. I can think of many way to implement it (including adding a new lazy-lock mode to cdrom device) but since I have no kernel hacking experience, I need everyone advices. Novice users need this 'eject' button after all.
Quoted 3 lines
Quoted 3 lines
This doesn't make sense at all. Just try the unmount and tell the user if it failed - you can't say whether it will fail before trying.
Yes, you can! Reading the code, if vfsmount.mnt_count > 1 then umount on that device will fail. Similar function is used by autofs, i.e. may_umount() in fs/namespace.c. I know the patch is not beautiful but it work.
Hmm. You seem to advice me to detect the 'eject' button then issue a umount? I don't know how and I guess it is impossible. But maybe I'm wrong.
Quoted 2 lines
Exactly. You poll media events from the drive, and upon an eject request you try and umount it. If it suceeds, you eject the tray.
No, it seems impossible to sense the eject request (right?). This is what I really did with the patched kernel and patched magicdev. When, for example:-
- user insert CD
- magicdev sense the CD, mount /dev/cdrom -> eject is ok
- user play an mp3 file in the disc
- magicdev found MOUNT_STATUS=BUSY, lock the drive -> eject disabled
- user stop using any file on the disc
- magicdev found MOUNT_STATUS=MOUNTED, unlock the drive -> eject is ok
- user push the eject button, the disc pop out because it's not locked.
- magicdev sense empty drive, umount /dev/cdrom
Signature · 5 lines
Samphan Raruenrom, The Open Source Project, National Electronics and Computer Technology Center, National Science and Technology Development Agency, Thailand.
Quoted 4 lines · Samphan Raruenrom
Quoted 2 lines
Exactly. You poll media events from the drive, and upon an eject request you try and umount it. If it suceeds, you eject the tray.
No, it seems impossible to sense the eject request (right?). This
No it isn't, in fact there are several ways to do it. Just by searching this list you should be able to find them.
Quoted 1 line
is what I really did with the patched kernel and patched magicdev.
magicdev is a piece of crap.
Quoted 10 lines
When, for example:-
- user insert CD
- magicdev sense the CD, mount/dev/cdrom-> eject is ok
- user play an mp3 file in the disc
- magicdev found MOUNT_STATUS=BUSY, lock the drive -> eject disabled
- user stop using any file on the disc
- magicdev found MOUNT_STATUS=MOUNTED, unlock the drive -> eject is ok
- user push the eject button, the disc pop out because it's not locked.
- magicdev sense empty drive, umount/dev/cdrom
That's not the way to do it. For drives that support media status event reporting you don't lock the drive, you just intercept the eject request. And at that time you check if you can umount /cdrom (or whatever) - if that suceeds, you send an eject request to the drive. This will work with any drive made in the last few years. You can also check out MS media status notification, I think you can even find that document on microsoft.com.
I think you need to spend a little more time thinking/researching this problem. At least it really looks like you are going about it all wrong.
Signature · 1 line
Jens Axboe
Quoted 6 lines · Jens Axboe
Quoted 3 lines · Samphan Raruenrom
Quoted 2 lines
Exactly. You poll media events from the drive, and upon an eject request you try and umount it. If it suceeds, you eject the tray.
No, it seems impossible to sense the eject request (right?). This
No it isn't, in fact there are several ways to do it. Just by searching this list you should be able to find them.
YES!! http://www.ussg.iu.edu/hypermail/linux/kernel/0202.0/att-0603/01-cd_poll.cget_media_event() = 1 -> eject
Thanks :-) I think you can't imagine how happy I am now. Thank you again.
Quoted 2 lines
Quoted 1 line
is what I really did with the patched kernel and patched magicdev.
magicdev is a piece of crap.
Why? I read all its code. Because of 2 sec. polling?
Quoted 2 lines
I think you need to spend a little more time thinking/researching this problem. At least it really looks like you are going about it all wrong.
You've just put me on the right track. Thank you very much. I really appreciate your and every others insightful comments.
Quoted 4 lines · Samphan Raruenrom
Quoted 3 lines · Jens Axboe
Quoted 2 lines · Samphan Raruenrom
Quoted 2 lines
Exactly. You poll media events from the drive, and upon an eject request you try and umount it. If it suceeds, you eject the tray.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
No, it seems impossible to sense the eject request (right?). This
No it isn't, in fact there are several ways to do it. Just by searching this list you should be able to find them.
YES!!
http://www.ussg.iu.edu/hypermail/linux/kernel/0202.0/att-0603/01-cd_poll.cget_media_event()= 1 -> eject
Thanks :-) I think you can't imagine how happy I am now. Thank you again.
I'm surprised you didn't find these things up front.
Quoted 4 lines
Quoted 2 lines
Quoted 1 line
is what I really did with the patched kernel and patched magicdev.
magicdev is a piece of crap.
Why? I read all its code. Because of 2 sec. polling?
Because it relies on unreliable mechanisms instead of using the proper support. That makes it 100% crap in my eyes, unusable.
Quoted 5 lines
Quoted 2 lines
I think you need to spend a little more time thinking/researching this problem. At least it really looks like you are going about it all wrong.
You've just put me on the right track. Thank you very much. I really appreciate your and every others insightful comments.
No problem.
Signature · 1 line
Jens Axboe
Quoted 6 lines · Samphan Raruenrom
Quoted 3 lines
Quoted 3 lines
This doesn't make sense at all. Just try the unmount and tell the user if it failed - you can't say whether it will fail before trying.
Yes, you can! Reading the code, if vfsmount.mnt_count > 1 then umount on that device will fail.
if you are doing the unmount currently and nothing changes because you hold the nessecary locks, yes. But as soon as you drop the locks this is void. There's no way to find out whether you can unmount a filesystem except trying it.
Quoted 3 lines
Hmm. You seem to advice me to detect the 'eject' button then issue a umount? I don't know how and I guess it is impossible. But maybe I'm wrong.
See Jens' post.
Quoted 10 lines · Christoph Hellwig
Quoted 5 lines · Samphan Raruenrom
Quoted 3 lines
Quoted 3 lines
This doesn't make sense at all. Just try the unmount and tell the user if it failed - you can't say whether it will fail before trying.
Yes, you can! Reading the code, if vfsmount.mnt_count > 1 then umount on that device will fail.
if you are doing the unmount currently and nothing changes because you hold the nessecary locks, yes. But as soon as you drop the locks this is void. There's no way to find out whether you can unmount a filesystem except trying it.
_^_ I understand it now. Thank you for your patience in explaining this to me. I learn quite a lot of things in <4 hours! Thank you everyone. I'll try to make a good use of what I learn here, e.g. to write another GNOME automounter that use the new "event status notification" and also let users eject their CDs -
"without polling" - possible?
It must be real cool to write such daemon (polling make any hacker feel guilty).
Signature · 5 lines
Samphan Raruenrom, The Open Source Project, National Electronics and Computer Technology Center, National Science and Technology Development Agency, Thailand.
Quoted 5 lines · Samphan Raruenrom
GNOME automounter that use the new "event status notification" and also let users eject their CDs -
"without polling" - possible?
It must be real cool to write such daemon (polling make any hacker feel guilty).
I don't want to burst your bubble, but you still have to poll when using event status notification. You didn't note that the name of my program was cd_poll.c? ;-)
But the poll is much quicker and more reliable.
Signature · 1 line
Jens Axboe
Quoted 6 lines · Samphan Raruenrom
Hi,
This patch add a new ioctl
MOUNT_STATUSto the 2.4 kernel's cdrom device. It'll be used as an API to query the mount status of a cdrom :-
Quoted 6 lines
CDROM_MOUNT_STATUS
Return :-
0 = not mount.
1 = mounted, but not in-use. It is ok to umount.
2 = busy. Umount will result in getting EBUSY.
<0 = error.
Huh? And what, pray tell, makes cdrom special? Not to mention
the use of ioctl, the inherent raciness of the interface and the fact that yes,
Quoted 1 line
This same functionality can be done in user-space,
... which should be the end of it.
Quoted 9 lines
drivers/cdrom/cdrom.c +8
-0+case CDROM_MOUNT_STATUS : { +struct super_block *sb =get_super (dev); +if (sb ==NULL )return -EINVAL ; +down_read (¤t->namespace->sem); +struct vfsmount *mnt =NULL ; +struct list_head *p; +list_for_each (p, ¤t->namespace->list) { + struct vfsmount *m = list_entry(p, struct vfsmount,
mnt_list);
drivers/cdrom/cdrom.c line 1639
Quoted 5 lines
drivers/cdrom/cdrom.c +5
-0+if (sb == m->mnt_sb) { + mnt = m;break ; + } + } +up_read (¤t->namespace->sem);
And what about other namespaces?
drivers/cdrom/cdrom.c line 1642
Quoted 4 lines
(quoted diff) +2
-0+ drop_super(sb); + int mstat = 0; /* 0 not mounted, 1 umount ok, 2 umount EBUSY*/
drivers/cdrom/cdrom.c +1
-0+if (mnt) mstat =1 + (atomic_read (&mnt->mnt_count) >1 );
Or the possibility that
* mnt might've been freed by that point.
* we might have the damn thing mounted in several places, some
busy, some not.
* cdrom had been used not by mount.
* cdrom had been mounted just as we had decided to tell that it's
not busy.