cd-tray immediately closed after it has been opened

cd-tray immediately closed after it has been opened

[Christian Krause]

Hi,

A very annoying bug has been introduced in udev release 121:

In some distribution it is not possible to eject a CD or DVD anymore, 
because the cd tray is immediately closed after it has been opened:

http://bugs.gentoo.org/show_bug.cgi?id#0886
https://bugzilla.redhat.com/show_bug.cgi?idE3095


We have tracked down the problem:

udev 120: works
udev > 121: problem occurs

The following checkin introduced the odd behaviour:
41677cf51fb2c14aa512ecf9410e43eb35560408
"persistent device naming: also read unpartitioned media"
with the following change in 60-persistent-storage.rules:
----------------------------------------------------------
+# import filesystem metadata
+IMPORT{program}="vol_id --export $tempnode"
----------------------------------------------------------

The following happens:

- the CD is ejected (by user)
- a change event is generated:
UDEV  [1217184249.757495] change   
/devices/pci0000:00/0000:00:1f.2/host2/target2:0:0/2:0:0:0/block/sr0 (block)
- the rule above triggers the execution of "vol_id --export ...sr0"
- vol_id opens the device with open(2), which causes the cd tray to be 
closed

Before this checkin, vol_id was only called for partitioned devices and 
so the CD tray stayed open.


I'm not an expert in writing udev rules, but I'm using now the following 
workaround:

diff --git a/rules/rules.d/60-persistent-storage.rules 
b/rules/rules.d/60-persistent-storage.rules
index 5ae0c7f..95eb1fd 100644
--- a/rules/rules.d/60-persistent-storage.rules
+++ b/rules/rules.d/60-persistent-storage.rules
@@ -52,6 +52,7 @@ ENV{DEVTYPE}="partition", ENV{ID_PATH}="?*", 
SYMLINK+="disk/by-path/$env{ID_PA

 # skip unpartitioned removable media devices from drivers which do not 
send "change" events
 ENV{DEVTYPE}="disk", KERNEL!="sd*|sr*", ATTR{removable}="1", 
GOTO="persistent_storage_end"
+ENV{DEVTYPE}="disk", KERNEL="sr*", ATTR{removable}="1", 
GOTO="persistent_storage_end"

 # import filesystem metadata
 IMPORT{program}="vol_id --export $tempnode"


If this patch is used, the vol_id call is omitted for sr* devices.

Sure, a drawback is, that you loose the "disk/by-label" link for CDs, 
but IMHO these features are mainly targetted for USB hard drives or 
something like this.


Best regards,
Christian Krause <chkr@plauener.de>
Ronald Wahl <rwahl@gmx.de>

Re: cd-tray immediately closed after it has been opened

[David Zeuthen]

On Sun, 2008-07-27 at 21:19 +0200, Christian Krause wrote:
> Before this checkin, vol_id was only called for partitioned devices and 
> so the CD tray stayed open.

Probably vol_id needs to use O_NONBLOCK when opening sr devices. Does
that fix it?

    David

Re: cd-tray immediately closed after it has been opened

[Christian Krause]

David Zeuthen wrote:
> On Sun, 2008-07-27 at 21:19 +0200, Christian Krause wrote:
>   
>> Before this checkin, vol_id was only called for partitioned devices and 
>> so the CD tray stayed open.
>>     
>
> Probably vol_id needs to use O_NONBLOCK when opening sr devices. Does
> that fix it?
>   
Yes, indeed using a patch like this:

diff --git a/extras/volume_id/vol_id.c b/extras/volume_id/vol_id.c
index 5c4e05d..3dc9f95 100644
--- a/extras/volume_id/vol_id.c
+++ b/extras/volume_id/vol_id.c
@@ -224,7 +224,7 @@ int main(int argc, char *argv[])
                goto exit;
        }
 
-       fd = open(node, O_RDONLY);
+       fd = open(node, O_RDONLY | O_NONBLOCK);
        if (fd < 0) {
                fprintf(stderr, "%s: error opening volume\n", node);
                rc = 2;

fixes the problem, too. But it looks like that this is discouraged:

http://gitweb.freedesktop.org/?p=hal.git;a=commit;h-ab6d1b27a160895e99f3a7111a78aff68baff3

;-)


Best regards,
Christian

Re: cd-tray immediately closed after it has been opened

[David Zeuthen]

On Mon, 2008-07-28 at 10:48 +0200, Christian Krause wrote:
> fixes the problem, too. But it looks like that this is discouraged:
> 
> http://gitweb.freedesktop.org/?p=hal.git;a=commit;h-ab6d1b27a160895e99f3a7111a78aff68baff3

I remember something funky about that; so we probably need to special
case vol_id to do something similar, e.g. first check for media and if
there is media make vol_id open it without O_NONBLOCK. And probably only
do the media check for optical drives.

     David

Re: cd-tray immediately closed after it has been opened

[Kay Sievers]

On Mon, Jul 28, 2008 at 10:48, Christian Krause <chkr@plauener.de> wrote:
> David Zeuthen wrote:
>>
>> On Sun, 2008-07-27 at 21:19 +0200, Christian Krause wrote:
>>
>>>
>>> Before this checkin, vol_id was only called for partitioned devices and
>>> so the CD tray stayed open.
>>>
>>
>> Probably vol_id needs to use O_NONBLOCK when opening sr devices. Does
>> that fix it?
>>
>
> Yes, indeed using a patch like this:
>
> diff --git a/extras/volume_id/vol_id.c b/extras/volume_id/vol_id.c
> index 5c4e05d..3dc9f95 100644
> --- a/extras/volume_id/vol_id.c
> +++ b/extras/volume_id/vol_id.c
> @@ -224,7 +224,7 @@ int main(int argc, char *argv[])
>               goto exit;
>       }
>
> -       fd = open(node, O_RDONLY);
> +       fd = open(node, O_RDONLY | O_NONBLOCK);
>       if (fd < 0) {
>               fprintf(stderr, "%s: error opening volume\n", node);
>               rc = 2;
>
> fixes the problem, too. But it looks like that this is discouraged:
>
> http://gitweb.freedesktop.org/?p=hal.git;a=commit;h-ab6d1b27a160895e99f3a7111a78aff68baff3

Just to check if its the kernel that closes the tray on open(), can you set:
  /proc/sys/dev/cdrom/autoclose
to "0" and try again?

Thanks,
Kay

Re: cd-tray immediately closed after it has been opened

[Christian Krause]

Hi Kay,

Kay Sievers wrote:
> On Mon, Jul 28, 2008 at 10:48, Christian Krause <chkr@plauener.de> wrote:
>   
>> David Zeuthen wrote:
>>     
>>> On Sun, 2008-07-27 at 21:19 +0200, Christian Krause wrote:
>>>
>>>       
>>>> Before this checkin, vol_id was only called for partitioned devices and
>>>> so the CD tray stayed open.
>>>>
>>>>         
>>> Probably vol_id needs to use O_NONBLOCK when opening sr devices. Does
>>> that fix it?
>>>
>>>       
>> Yes, indeed using a patch like this:
>>
>> diff --git a/extras/volume_id/vol_id.c b/extras/volume_id/vol_id.c
>> index 5c4e05d..3dc9f95 100644
>> --- a/extras/volume_id/vol_id.c
>> +++ b/extras/volume_id/vol_id.c
>> @@ -224,7 +224,7 @@ int main(int argc, char *argv[])
>>               goto exit;
>>       }
>>
>> -       fd = open(node, O_RDONLY);
>> +       fd = open(node, O_RDONLY | O_NONBLOCK);
>>       if (fd < 0) {
>>               fprintf(stderr, "%s: error opening volume\n", node);
>>               rc = 2;
>>
>> fixes the problem, too. But it looks like that this is discouraged:
>>
>> http://gitweb.freedesktop.org/?p=hal.git;a=commit;h-ab6d1b27a160895e99f3a7111a78aff68baff3
>>     
>
> Just to check if its the kernel that closes the tray on open(), can you set:
>   /proc/sys/dev/cdrom/autoclose
> to "0" and try again?
>   
I've tried it out with a small test program which just does an 
open("/dev/sr0", O_RDONLY)
and additionally with the complete udev/hal/... stack:

If autoclose = 0 then the tray is not closed otherwise it is (without 
O_NONBLOCK).
If O_NONBLOCK is used, the tray is not closed, even if autoclose = 0. So 
yes, it is the kernel which closes the tray.


Best regards,
Christian

Re: cd-tray immediately closed after it has been opened

[Kay Sievers]

On Thu, Jul 31, 2008 at 09:31, Christian Krause <chkr@plauener.de> wrote:
> Kay Sievers wrote:
>>
>> On Mon, Jul 28, 2008 at 10:48, Christian Krause <chkr@plauener.de> wrote:
>>
>>>
>>> David Zeuthen wrote:
>>>
>>>>
>>>> On Sun, 2008-07-27 at 21:19 +0200, Christian Krause wrote:
>>>>
>>>>
>>>>>
>>>>> Before this checkin, vol_id was only called for partitioned devices and
>>>>> so the CD tray stayed open.
>>>>>
>>>>>
>>>>
>>>> Probably vol_id needs to use O_NONBLOCK when opening sr devices. Does
>>>> that fix it?
>>>>
>>>>
>>>
>>> Yes, indeed using a patch like this:
>>>
>>> diff --git a/extras/volume_id/vol_id.c b/extras/volume_id/vol_id.c
>>> index 5c4e05d..3dc9f95 100644
>>> --- a/extras/volume_id/vol_id.c
>>> +++ b/extras/volume_id/vol_id.c
>>> @@ -224,7 +224,7 @@ int main(int argc, char *argv[])
>>>              goto exit;
>>>      }
>>>
>>> -       fd = open(node, O_RDONLY);
>>> +       fd = open(node, O_RDONLY | O_NONBLOCK);
>>>      if (fd < 0) {
>>>              fprintf(stderr, "%s: error opening volume\n", node);
>>>              rc = 2;
>>>
>>> fixes the problem, too. But it looks like that this is discouraged:
>>>
>>>
>>> http://gitweb.freedesktop.org/?p=hal.git;a=commit;h-ab6d1b27a160895e99f3a7111a78aff68baff3
>>>
>>
>> Just to check if its the kernel that closes the tray on open(), can you
>> set:
>>  /proc/sys/dev/cdrom/autoclose
>> to "0" and try again?
>>
>
> I've tried it out with a small test program which just does an
> open("/dev/sr0", O_RDONLY)
> and additionally with the complete udev/hal/... stack:
>
> If autoclose = 0 then the tray is not closed otherwise it is (without
> O_NONBLOCK).
> If O_NONBLOCK is used, the tray is not closed, even if autoclose = 0. So
> yes, it is the kernel which closes the tray.

Ah, great, thanks for the test. We might just be able to skip optical
drives if cdrom_id did not find a valid media. Does adding this rule
(ID_CDROM_MEDIA_TRACK_COUNT) fix the unwanted tray closing?

Thanks,
Kay

--- a/rules/rules.d/60-persistent-storage.rules
+++ b/rules/rules.d/60-persistent-storage.rules
@@ -52,6 +52,8

 # skip unpartitioned removable media devices from drivers which do
not send "change" events
 ENV{DEVTYPE}="disk", KERNEL!="sd*|sr*", ATTR{removable}="1",
GOTO="persistent_storage_end"
+# skip optical drives without media
+ENV{DEVTYPE}="disk", KERNEL="sr*",
ENV{ID_CDROM_MEDIA_TRACK_COUNT}!="?*", GOTO="persistent_storage_end"

Re: cd-tray immediately closed after it has been opened

[David Zeuthen]

On Thu, 2008-07-31 at 09:48 +0200, Kay Sievers wrote:
> Ah, great, thanks for the test. We might just be able to skip optical
> drives if cdrom_id did not find a valid media. Does adding this rule
> (ID_CDROM_MEDIA_TRACK_COUNT) fix the unwanted tray closing?

Your patch fixes the problem for me. Thanks!

     David

Re: cd-tray immediately closed after it has been opened

[Christian Krause]

Hi Kay,

Kay Sievers wrote:
> Ah, great, thanks for the test. We might just be able to skip optical
> drives if cdrom_id did not find a valid media. Does adding this rule
> (ID_CDROM_MEDIA_TRACK_COUNT) fix the unwanted tray closing?
>
> Thanks,
> Kay
>
> --- a/rules/rules.d/60-persistent-storage.rules
> +++ b/rules/rules.d/60-persistent-storage.rules
> @@ -52,6 +52,8
>
>  # skip unpartitioned removable media devices from drivers which do
> not send "change" events
>  ENV{DEVTYPE}="disk", KERNEL!="sd*|sr*", ATTR{removable}="1",
> GOTO="persistent_storage_end"
> +# skip optical drives without media
> +ENV{DEVTYPE}="disk", KERNEL="sr*",
> ENV{ID_CDROM_MEDIA_TRACK_COUNT}!="?*", GOTO="persistent_storage_end"
>
>   

Sorry for the late answer. I've tried out the patch and it solves the 
problem for me. Thanks!


Best regards,
Christian

Re: cd-tray immediately closed after it has been opened

[Kay Sievers]

On Sun, Aug 3, 2008 at 19:38, Christian Krause <chkr@plauener.de> wrote:
> Kay Sievers wrote:
>>
>> Ah, great, thanks for the test. We might just be able to skip optical
>> drives if cdrom_id did not find a valid media. Does adding this rule
>> (ID_CDROM_MEDIA_TRACK_COUNT) fix the unwanted tray closing?
>>
>> Thanks,
>> Kay
>>
>> --- a/rules/rules.d/60-persistent-storage.rules
>> +++ b/rules/rules.d/60-persistent-storage.rules
>> @@ -52,6 +52,8
>>
>>  # skip unpartitioned removable media devices from drivers which do
>> not send "change" events
>>  ENV{DEVTYPE}="disk", KERNEL!="sd*|sr*", ATTR{removable}="1",
>> GOTO="persistent_storage_end"
>> +# skip optical drives without media
>> +ENV{DEVTYPE}="disk", KERNEL="sr*",
>> ENV{ID_CDROM_MEDIA_TRACK_COUNT}!="?*", GOTO="persistent_storage_end"
>>
>
> Sorry for the late answer. I've tried out the patch and it solves the
> problem for me. Thanks!

It's part of udev 126. Thanks a lot for your help.

Kay