pressing physical eject button on CD/DVD drive causes icon to remain on desktop after eject

I found the possible cause of the bug and reported this upstream.
https://bugs.freedesktop.org/show_bug.cgi?id=34710

Quote the bug report on fd.o here so more people can see it.

UDisks failed to perform ForceUnmount when I press the physical eject button on
my CDROM.
So I downloaded udisks source code from git and traced it for a while and then
I found the problem.

Here are the related properties of my device.
--------------------------------------------------------------------------------------------------------------
  device-file: /dev/sr0
  removable: 1
  has media: 1 (detected at Fri Feb 25 19:16:23 2011)
    detects change: 1
    detection by polling: 1 <--- I have no SATA AN, so polling is
performed.
    detection inhibitable: 1
    detection inhibited: 0 <-- polling of the device is not inhibited.
  is read only: 0
  is mounted: 1
  drive:
    vendor: HL-DT-ST
    model: HL-DT-ST DVDRAM GMA-4082N
    revision: CX08
    serial: M0573JA3106
    detachable: 0
    ejectable: 1
    media: optical_cd
    interface: scsi
-------------------------------------------------------------------------------------------------------------
I tested udisks on a clean system in a clean xsession + terminal emulator so
there shouldn't be other programs interfering with udisks.

In src/poller.c, poller_poll_device() :

      fd = open (device_file, O_RDONLY | O_NONBLOCK | O_EXCL);
      if (fd != -1)
          close (fd);

When I inserted a CD, this call to open() succeeded and returned a valid fd. I
enabled POLL_SHOW_DEBUG, and according to the debug message, the device was
polled every 2 seconds as expected.

However, after the device is mounted, either by udisks --mount or by calling
sudo mount myself, open(device_file) always returns -1 here.
Then, I press the physical eject button on my CDROM to get its tray opened. The
polling code should let udisks detect the media removal and then invokes
ForceUnmount, but this is not the case. The call to open() kept returning -1
every 2 seconds and udisks didn't detect the media change at all. Checking
errno, "Device is busy" is reported.

Then I remove the O_EXCL flag and perform the whole test again. This time,
after the CD is removed by pressing physical eject button, the open() call
succeeded and returned a valid fd. Then, udisks detects the media change and
gracefully perform ForceUnmount for it. Everything works fine as expected.

In addition, when "Device is busy", I tried fuser and lsof, but none of them
demonstrates other process using the device. However, if I unmount the device
either by calling sudo umount or udisks --unmount, this error is gone and
open() can return a valid fd. Then everything works.

Removing O_EXCL from this open() call seems to fix the issue, but this of
course is not the correct way to fix it.

Another interesting thing I found is, if I call udisks --poll-for-media change
manually after forced removal of CD with physical eject ...

I made a new patch according to the suggestions from Kay Sievers.

Thanks for all the work on this!

I think this might have already been solved here: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=550316

Can you recheck the behavior with that patch applied? If it's still a problem, perhaps update the patch to work with the existing polling patch?

Thanks!

No, these two patches handles totally different problems.
The patch you mentitioned focused on supporting the legacy IDE driver via ioctl() while my patch fixed the O_EXECL part which is not done in previous patches. In previous patch, there is a line which reads // TODO: check if media is mounted.
Basically my patch does this TODO stuff, checking if media is mounted.
Please review it again if possible. Thanks.

I updated the patch with the current udisks package in natty to apply cleany (see the result attached). But it failled to build :

   CC udisks_daemon-poller.o
gcc -DHAVE_CONFIG_H -I. -I.. -I../src -I../src -DPACKAGE_LIBEXEC_DIR=\""/usr/lib/udisks"\" -DPACKAGE_SYSCONF_DIR=\""/etc"\" -DPACKAGE_DATA_DIR=\""/usr/share"\" -DPACKAGE_BIN_DIR=\""/usr/bin"\" -DPACKAGE_LOCALSTATE_DIR=\""/var"\" -DPACKAGE_LOCALE_DIR=\""/usr/share/locale"\" -DPACKAGE_LIB_DIR=\""/usr/lib"\" -D_POSIX_PTHREAD_SEMANTICS -D_REENTRANT -pthread -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/polkit-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/gudev-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/gio-unix-2.0/ -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I../src -DG_LOG_DOMAIN=\"udisks-daemon\" -g -O2 -Wall -Wchar-subscripts -Wmissing-declarations -Wnested-externs -Wpointer-arith -Wcast-align -Wsign-compare -Wformat -Wformat-security -c -o udisks_daemon-poller.o `test -f 'poller.c' || echo './'`poller.c
poller.c: In function 'poller_poll_device':
poller.c:242:27: error: 'device_file' undeclared (first use in this function)
poller.c:242:27: note: each undeclared identifier is reported only once for each function it appears in
poller.c: In function 'poller_check_ide_cdrom':
poller.c:208:17: warning: ignoring return value of 'write', declared with attribute warn_unused_result
make[6]: *** [udisks_daemon-poller.o] Error 1
make[6]: Leaving directory `/tmp/buildd/udisks-1.0.2/src'
make[5]: *** [all-recursive] Error 1
make[5]: Leaving directory `/tmp/buildd/udisks-1.0.2/src'
make[4]: *** [all] Error 2
make[4]: Leaving directory `/tmp/buildd/udisks-1.0.2/src'
make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory `/tmp/buildd/udisks-1.0.2'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/tmp/buildd/udisks-1.0.2'
dh_auto_build: make -j1 returned exit code 2
make[1]: *** [override_dh_auto_build] Error 2
make[1]: Leaving directory `/tmp/buildd/udisks-1.0.2'
make: *** [build] Error 2

Why upstream udisks authors refuse integrating theses important patches?
Without them, udisks never works with some devices.
When you open a decice with O_EXECL and it's mounted, you'll get EBUSY error.
So when we get EBUSY, we check if it's mounted. If it is, we open it without O_EXECL.
Otherwise, that means other programs, such as cd burner, are using the device and we shouldn't touch it.
That's basically why is_mounted() is called here.

Personally, I think the correct way to integrate these two patches is like this:

 In the original patch, you can find this:

                } else if (errno == EBUSY && device->is_ide_cdrom) {
                        // TODO: check if media is mounted
                        // in that case mounting with O_EXCL should be safe
                        fd = open (device->dev_path, O_RDONLY | O_NONBLOCK);
                        if (fd != -1) {
                                poller_check_ide_cdrom (fd, device);
                                close (fd);
                        }
                 }

change it to:

                } else if (errno == EBUSY) { // no need to check for ide_cd here
+ /* From hal/hald/linux/addons/addon-storage.c: */
+ /* this means the disc is mounted or some other app,
+ * like a cd burner, has already opened O_EXCL */
+
+ /* HOWEVER, when starting hald, a disc may be
+ * mounted; so check /etc/mtab to see if it
+ * actually is mounted. If it is we retry to open
+ * without O_EXCL
+ */
+ if (is_mounted (device->dev_path))
+ {
+ fd = open (device->dev_path, O_RDONLY | O_NONBLOCK);
+ }
                        if (fd != -1 && device->is_ide_cdrom) { // only IDE cd needs this.
                                poller_check_ide_cdrom (fd, device);
                                close (fd);
                        }
                 }

If someone knows the upstream udisks authors, talk to them please.
It's really bad that these important patches cannot be accepted since without them udisks never works correctly.
The upstream authors tend to think that people has the latest hardware + the most updated kernel.
This is just wrong in the real world.

This bug is giving my friend a very bad impression of Linux on her new Thinkpad. Can we have a hotfix plzzz? I guess I will try to build it myself...

How much work is adding in-kernel polling support to udisks now that natty's kernel supports that feature?

@Jani: The actual polling support is easy to add, I already have a patch for it. It just needs some more changes to also properly handle the eject button; I discussed the approach with the other upstreams, we found an agreement, and I'm working on it now.

Status: I have a set of patches which make this work nicely, sent to upstream. I'll wait for David to review, then push them, and see to making a new release.

My kernel-polling patches are still being reviewed upstream, and they look good so far. But I think at this point they are too intrusive and have missed the Natty line, so I'll fix PCMan's patch to actually build (which looks simple), and upload that as a workaround for natty.

Thanks PCMan!

This bug was fixed in the package udisks - 1.0.2-4ubuntu1

---------------
udisks (1.0.2-4ubuntu1) natty; urgency=low

  * Add 12-polling-failed-due-to-O_EXCL.patch: Work around buggy userspace
    CD-ROM polling. The proper patch is currently being discussed upstream,
    but too intrusive for Natty at this point. This correctly cleans up mounts
    after ejecting a CD. Thanks to Hong Jen Yee for this patch! (LP: #711549)
 -- Martin Pitt <email address hidden> Sun, 10 Apr 2011 16:01:50 +0200