From: Tejun Heo <tj@kernel.org>
To: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Kay Sievers <kay@vrfy.org>, linux-scsi@vger.kernel.org
Subject: Re: Spurious DISK_EVENT_MEDIA_CHANGE on USB DVD hotplug?
Date: Mon, 14 Aug 2017 11:30:30 -0700 [thread overview]
Message-ID: <20170814183030.GA4087514@devbig577.frc2.facebook.com> (raw)
In-Reply-To: <c22ebffe-cf80-c3ef-2bd9-6b3b66271226@redhat.com>
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
next prev parent reply other threads:[~2017-08-14 18:30 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-10 14:45 Spurious DISK_EVENT_MEDIA_CHANGE on USB DVD hotplug? Joe Lawrence
2017-08-14 18:30 ` Tejun Heo [this message]
2017-08-17 13:44 ` Joe Lawrence
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170814183030.GA4087514@devbig577.frc2.facebook.com \
--to=tj@kernel.org \
--cc=joe.lawrence@redhat.com \
--cc=kay@vrfy.org \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox