By simply reading the code history, it is trivial to verify that this

description is false:
Commit 285e9670 depends on a341cd0f, so by definition it is 285e9670 --

or rather the incomplete update of your original patch that resulted in

285e9670 -- that is broken.
Since commit 285e9670 is broken, you fixed the wrong thing.
Furthermore, you broke a userspace interface that was introduced by

a341cd0f, by removing the event filter controlled by userspace.
Did anyone bother to read any code at all?
When 285e9670 was updated for the scsi_evt_* interface, it should have

initialized the supported_events mask.
And that is the fix -- initialize supported_events according to sr/sd's

needs, and revert this change (4d1566ed2100d074ccc654e5cf2e44cdea3a01d0)

as obviously broken.
	Jeff
--

Yes, I did.  The userspace interface has never worked as a writeable

attribute because it's read only (as Kay has pointed out several times).

Based on that, I just #if'd out the filter code to get polled media

events working again.
That would break HAL ... it's using the presence of the media_event flag

in the variable to indicate it doesn't need to poll.  We need two flags,
James
--

It has a show attribute and was obviously intended as a writable 
Oh for pete's sake.  How hard is it to create a second bit _and_ keep

the existing AN featureset working?
_YOU_ are the one that broke this stuff in the first place, can you

please take two seconds to fix it without breaking other stuff?
	Jeff
--

It's all the fault of those annoying computer systems that do what we

tell them rather than what we intend.  In this case sysfs won't allow
The existing feature set currently works, even with the event filter

removed, so it's the fastest and smallest bug fix to get polling to

work.  I'm just pointing out that your suggested fix wouldn't work ... I

think at -rc6 the more complex one involving two bits is really a
It's fixed, and nothing else is additionally broken by the current fix.
James
--

Other than the newly-inconsistent, exported-to-userspace interface, ITYM.
	Jeff
--

How is it newly inconsistent?  It can still be used in the manner you

intended it, namely to tell if the CD supports AN or not.  When we add

the new bit, we'll have to add a new file for it anyway.
James
--

Previously, the events could be expected to be sent (or sent) in strict

accordance with supported_events.
Now, you are essentially sending out-of-band events, because the

receiver requested supported_events -- but is getting much more than that!
You broke a fundamental assumption of the interface -- that

supported_events accurately and fully represented all events sent via

the new API.
	Jeff
--

we don't publish supported_events; we publish a single file called

media_change.  Right at the moment its meaning is I have a 1 if AN is

supported and 0 if it's not.
Before the fix, its meaning was  1 if AN is supported and 0 if it's not.
The file name now looks wrong ... it should probably be an_media_change

or something if we have to introduce a new file called poll_media_chage
Anyway, realistically, since no CD or DVD on the market today seems to

support the AHCI AN method, this argument is really moot ... we just

need polling to work.
James
--

No, it's not.  This information is published to userspace EVEN WHEN AN

SUPPORT IS ABSENT.  As any sane interface would do -- report a zero value.
Thus, the interface is useful even in the absence of AN.
Anyway, to recap...
before your "fix":
	userspace interface always reflected list of events sent

	via my new API
after your "fix":
	events may or may not be reflected in userspace interface,

	who knows?
Is that distinction so difficult to see?
This interface, like it or not, is in 2.6.24, which means its published

and "out there."
This is a clear regression from 2.6.24.
supported_events' value was accurate in 2.6.24.  Now it is not.
	Jeff
--

Right, it still does this.  Currently hal is taking 0 in the
Yes, that's why we're having the argument ... it's the use of the event
It didn't show a list to userspace ... it's a single file with a 0 or 1
It's still a single file with a 0 or 1 value.  0 means doesn't support
Well, if they have the same userspace effect, and display the same

information to userspace, it's a bit hard to see how a user would
The current published API is the media_events file.  HAL is using that

to indicate support for AN.  This is why we can't simply change it to 1

wholesale because we'll confuse HAL (HAL still has to send polling

events if AN isn't supported).
So, the best fix for 2.6.25 at the current -rc6 is to keep the meaning

of the media_change file the same (0 for no AN, 1 for AN) and let HAL

take the polled events via udev, which basically means it's preserving

the behaviour and isn't a regression.
For 2.6.26 we can add a new media_events_polled (or some other name)

file, fix the sysfs ro attribute and make them true writeable filters so

some raving user can turn off polled events if they want and everyone

will be happy.
James
--

So version 3 of the interface will be the first stable and usable one...

sigh :/
It's just disheartening that the userspace filtering stuff (including

interface) was disabled rather than fixed, given that that change came

first and arguably the follow-on change (285e9670) was an abuse of the

API that was never corrected -- which seems to be tacitly acknowledged

since everyone seems to agree more than one flag is needed.
	Jeff
--

Well, it's open source ... this interface has had a better passage than
I think the current hack is the simplest way to fix up 2.6.25, given the

lateness in the -rc cycle ... we can do a far better job in 2.6.26 and

still be backwards compatible with the 2.6.25 hack.  In fact, I

committed to Kay that we would ... although if you want to do it

instead, that would be easier for me.
Thanks,
James
--

It worked fine with Kristen's patches, and that's where it is coming
You mean adding a new flag? As every device will "support" these events.
Thanks,

Kay
--

Neither her patches nor yours went upstream verbatim at version one.

You need to look at what happens upstream, not what happened in your 
I mean the attribute with both 'show' (read) and 'store' (write) 
Does every device attached to every controller wish to fire these events?
If so, then that wants new flag, yes.
	Jeff
--

That's not a bug.
For starters, we have transport classes that provide generic store

methods but can't pass the information on to drivers.  For these, we set

the attribute to read only even if there is a store method.  Even if

that weren't the case, how do you know which of UGO wants the write

setting?  Finally, if you look at the sysfs code, the place where mode

settings of the sysfs files are handled doesn't see the show/store

methods at all (these are handled at a different layering within the

generic driver model).
James
--

You give it to root, and let userspace change it after that, like we

always do.
	Jeff
--

Yes, a new flag sounds good. Every device that detects a media change

should send such event, so userspace can possibly update it's

representation of the volume.

That way, userspace can switch to a single generic event, regardless if

the device is periodically polled, or supports AN.
Thanks,

Kay
--

Patches, please? I can do the revert, but the patch to supported_events

and the testing, please?
		Linus

--

James was the one who wrote the incomplete/broken changes,
	commit 285e9670d91cdeb6b6693729950339cb45410fdc

	Author: Kay Sievers <kay.sievers@vrfy.org>

	Date:   Tue Aug 14 14:10:39 2007 +0200
	    [SCSI] sr,sd: send media state change modification events

[...]

	[jejb: modified for new event API]
so I'm hoping he will fix it up.
I'll send a patch if he doesn't, though.
	Jeff
--

I tested this and discussed it with Kristen before it went in.  The

current fix you sucked in with scsi-rc-fixes basically makes the whole

lot work for 2.6.25 (both AN media change events and traditional polled

ones).  The thing that doesn't work is the ability to turn off AN events

from user space, but that feature never worked anyway.
I promised Kay I'd work on a better solution for 2.6.26 which will

include separating up the input events into polled and AN for a CD but

emitting the same media change event to udev at the top (so you can rely

on either polled or AN for media change and select which or both if you

desire).
James
--

Not true.  Change the perm with chmod...
But furthermore, change 4d1566ed renders the userspace interface

/inconsistent/ even for the read-only case that we all acknowledge

_does_ work.
IOW, after your "fix", the userspace interface gives an incorrect

picture of what events are/are not masked.
If you don't have the time to complete work on
	commit 285e9670d91cdeb6b6693729950339cb45410fdc

	Author: Kay Sievers <kay.sievers@vrfy.org>

	Date:   Tue Aug 14 14:10:39 2007 +0200
	    [SCSI] sr,sd: send media state change modification events
then revert _that_.
Don't break my patch, just because yours (which came _after_ mine)

doesn't work.
	Jeff
--