Hello, Joe. On Thu, Aug 10, 2017 at 10:45:54AM -0400, Joe Lawrence wrote: > In the case of my USB DVD -> laptop example, there is no media in my > device, however I still see the DISK_EVENT_MEDIA_CHANGE event. This is > a bit confusing, and I was wondering if there was an explanation for > the following: > > drivers/scsi/sr.c :: sr_probe() > > disk->events = DISK_EVENT_MEDIA_CHANGE | DISK_EVENT_EJECT_REQUEST; > ... > cd->media_present = 1; > > DISK_EVENT_MEDIA_CHANGE events will pass through to userspace and > for some reason cd->media_present defaults to 1? More on that below...
I don't have any concrete ideas but I think the only thing it's trying to do is to always generate at least one changed event no matter what. ... > sr_check_events() compares the previous (in this case, default) > media_present value with what the TUR returns. If it has changed, then > turn on the DISK_EVENT_MEDIA_CHANGE event bit. > > In my laptop USB DVD case, !scsi_status_is_good and sshdr.asc == 0x3a, > so last_present (1) and cd->media_present (0) mis-compare and the change > event is set. That does not seem intuitive to me, is this a bug? It's not incorrect. We can try to change the behavior to avoid double notifications but given that this has been like this for a really long time and that it isn't technically incorrect, I'm not sure changing it is a good idea. It might as well break other things. > Bringing this back to the reported BMC case, which presumably does have > "media" present in the virtual device... is it reasonable to expect a > DISK_EVENT_MEDIA_CHANGE even for a new device that contains media? (I > haven't verified, but in this case GET_EVENT_STATUS_NOTIFICATION might > be enough to set media present.) Yeah, I think so. > If there is documentation that explains DISK_EVENT_MEDIA_CHANGE conditions > somewhere, feel free to point me there. AFAIK, there isn't any. The only thing it tries to do is generating at least one event after media change. Given that media is reported present after the last notification, I think userspace should be able to do the right thing (and must have been doing the right thing until recently). Thanks. -- tejun
