Ubuntu

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

Reported by Chris Gagnon on 2011-02-01
34
This bug affects 6 people
Affects Status Importance Assigned to Milestone
udisks
Fix Released
High
udisks (Ubuntu)
Medium
Martin Pitt

Bug Description

Binary package hint: udisks

Steps to reproduce:
1. log in to Gnome desktop
2. Insert CD or DVD in drive and wait for icon to appear on desktop
3. Press the physical eject button on the DVD/CD drive(do NOT click the software eject by right clicking the icon)

Expected results:
CD or DVD is ejected and desktop (and places menu) Icon disappear for the CD or DVD

Actual results:
CD or DVD is ejected and the desktop icon remains on the desktop (The DVD or CD remains in the 'Places' menu as well)

ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: udisks 1.0.1+git20100614-3
ProcVersionSignature: Ubuntu 2.6.35-24.42sutton04-generic-pae 2.6.35.8
Uname: Linux 2.6.35-24-generic-pae i686
Architecture: i386
Date: Tue Feb 1 17:46:13 2011
DistributionChannelDescriptor:
 # This is a distribution channel descriptor
 # For more information see http://wiki.ubuntu.com/DistributionChannelDescriptor
 canonical-oem-sutton-edge-amd-20110124-2
ExecutablePath: /usr/lib/udisks/udisks-daemon
InstallationMedia: Ubuntu 10.10 "Maverick" - Build i386 LIVE Binary 20110124-21:10
MachineType: LENOVO 0199RXU
ProcCmdLine: BOOT_IMAGE=/boot/vmlinuz-2.6.35-24-generic-pae root=UUID=5209bc7a-fb56-400e-b6fd-1e3db99384cf ro quiet splash acpi_skip_timer_override
ProcEnviron:

SourcePackage: udisks
dmi.bios.date: 12/07/2010
dmi.bios.vendor: LENOVO
dmi.bios.version: 82ET71WW (2.11 )
dmi.board.name: 0199RXU
dmi.board.vendor: LENOVO
dmi.board.version: Not Available
dmi.chassis.asset.tag: No Asset Information
dmi.chassis.type: 10
dmi.chassis.vendor: LENOVO
dmi.chassis.version: Not Available
dmi.modalias: dmi:bvnLENOVO:bvr82ET71WW(2.11):bd12/07/2010:svnLENOVO:pn0199RXU:pvrThinkPadEdge:rvnLENOVO:rn0199RXU:rvrNotAvailable:cvnLENOVO:ct10:cvrNotAvailable:
dmi.product.name: 0199RXU
dmi.product.version: ThinkPad Edge
dmi.sys.vendor: LENOVO

Related branches

Chris Gagnon (chris.gagnon) wrote :
description: updated
Changed in udisks (Ubuntu):
status: New → Confirmed
PCMan (pcman-tw) wrote :
Download full text (3.6 KiB)

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 ...

Read more...

Changed in udisks:
importance: Unknown → High
status: Unknown → Confirmed
tags: added: patch
PCMan (pcman-tw) wrote :

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

Kees Cook (kees) wrote :

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!

Changed in udisks (Ubuntu):
status: Confirmed → Incomplete
PCMan (pcman-tw) wrote :

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.

Julien Lavergne (gilir) wrote :

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

PCMan (pcman-tw) wrote :

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);
                        }
                 }

PCMan (pcman-tw) wrote :

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.

Eric Drechsel (ericdrex) wrote :

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...

Jani Monoses (jani) wrote :

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

Martin Pitt (pitti) wrote :

@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.

Changed in udisks (Ubuntu):
status: Incomplete → In Progress
assignee: nobody → Martin Pitt (pitti)
importance: Undecided → Medium
Martin Pitt (pitti) wrote :

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.

Changed in udisks:
status: Confirmed → In Progress
Martin Pitt (pitti) wrote :

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!

Launchpad Janitor (janitor) wrote :

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

Changed in udisks (Ubuntu):
status: In Progress → Fix Released
Changed in udisks:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.