On Thu, Apr 07, 2022 at 08:47:13PM +0300, Maxim Devaev wrote:
> В Thu, 7 Apr 2022 12:06:01 -0400
> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> > On Wed, Apr 06, 2022 at 09:36:34PM +0300, Maxim Devaev wrote:
> > > В Wed, 6 Apr 2022 13:51:40 -0400
> > > Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > > On Wed, Apr 06, 2022 at 07:52:34PM +0300, Maxim Devaev wrote:
> > > > > > It's not clear to me how breaking I/O operations allows you to do a
> > > > > > "force eject". It seems that what you would need is something like
> > > > > > fsg_store_file() that omits the curlun->prevent_medium_removal check.
> > > > > > Interrupting a lengthy I/O operation doesn't really have anything to do
> > > > > > with this.
> > > > >
> > > > > Perhaps I chose the wrong path, it's just how my userspace code works now.
> > > > > If the drive is connected to a Linux host, then in order to clear
> > > > > the "file" and extract the image, I sent a SIGUSR1 signal to the "file-storage"
> > > > > thread. This interrupted long IO operations, reset curlun->prevent_medium_removal
> > > > > and I got the ability to extract.
> > > >
> > > > Oh, I see. That's kind of an unintended side effect of not calling
> > > > raise_exception().
> > > >
> > > > And while it does interrupt long I/O operations, it does so in
> > > > non-sanctioned way. To the host it will appear as though the gadget's
> > > > firmware has crashed, since the gadget will stop sending or receiving
> > > > data. Eventually the host will time out and reset the gadget.
> > > >
> > > > Maybe that's the sort of thing you want, but I rather doubt it.
> > >
> > > It's hard to say how it actually should work in case of force removing.
> > > At least the currect approach with SIGUSR1 is really working on thousands
> > > systems and with Linux, Mac and Windows. I believe that the criterion
> > > of the experiment is quite important here. I know of several other utilities
> > > that use SIGUSR1 for similar purposes.
> >
> > This merely means that the current unintended behavior of userspace USR1
> > signals must not be changed. But it doesn't mean you have to continue
> > to rely on that behavior; you can implement something better.
>
> So I suggest break_io :) I haven't come up with anything better.
But breaking an I/O doesn't do all that you want. That is, interrupting an
I/O request (causing an executing command to terminate early) doesn't in
itself change the prevent/allow status. Those are two separate operations.
The fact that sending a USR1 signal does both is merely a coincidence.
Furthermore, it's not clear just what you mean when you say KVM needs to
"turn it off immediately". How soon is "immediately"? Even a USR1 signal
doesn't work instantaneously. You may find that a forced eject without an
I/O interruption works quickly enough.
> > > > > Will masking the curlun->prevent_medium_removal flag be enough?
> > > >
> > > > I think so. But it will be blocked to some extent by long-running I/O
> > > > operations, because those operations acquire the filesem rw-semaphore
> > > > for reading.
> > > >
> > > > More precisely, each individual command holds the rw-semaphore. But the
> > > > semaphore is dropped between commands, and a long-running I/O operation
> > > > typically consists of many separate commands. So the blocking may be
> > > > acceptable.
> > >
> > > It is very important for KVM-over-IP to be able to command "turn it off immediately".
> >
> > Why is this? A lot of actual devices (DVD drives, for instance) don't
> > give you the ability to eject the media when the host has prevented it.
> > Why should f-mass-storage be different?
>
> The DVD drive has the ability to physically eject the disc.
You mean by sticking an unfolded paperclip into the manual-eject hole?
> It's not too good
> for the drive itself, but it's just there. We can also urgently remove
> the USB flash drive.
Removing a USB flash drive is not a media-eject operation; it's a
disconnect operation. (That is, it removes the entire device, not just the
media.) By contrast, taking an SD card out from a USB card reader _is_ an
example of a media ejection. But card readers do not claim to support the
prevent/allow mechanism.
> At least there is one situation where the behavior of f_mass_storage differs
> from the behavior of a real drive. What happens when you click on the physical
> "eject" button?
If the host has prevented ejection, nothing happens. Otherwise the disc
gets ejected.
> Yes, the OS can block this, but the problem is that we don't have
> an "eject" here.
What do you mean? Writing an empty string to the sysfs "file" attribute
is the virtual analog of pressing the eject button.
> If I connect the gadget to the Linux host and don't even mount
> the image, Linux won't let me change the image in the "file", since the gadget
> will be constantly busy with some IO.
What I/O are you referring to? Why would a Linux host want to do constant
I/O to an unmounted device?
Besides, constant I/O shouldn't prevent you from ejecting or changing the
backing storage. The eject or change can take place between I/O requests.
> But I believe creating a virtual "eject" button is a separate task that
> does not depend on "break_io".
Do you mean "eject" or "forced eject"? I agree that a virtual "forced
eject" is separate from "break_io", and it's probably a lot closer to what
you really want.
> > > In this context, I would prefer "break_io" rather than "allow_force_remove".
> >
> > Okay. But what about the 30-second host timeout I mentioned above?
> > Does this actually happen with your approach? It seems like the kind of
> > thing you don't want in a "turn it off immediately" situation. (I
> > haven't tried doing this myself -- maybe I should.)
>
> Neither I nor my users noticed any problems related to this. After extracting
> the image using SIGUSR1/"file", I can just assign a new "file"image
> and everything will work.
I will try it for myself and see what happens.
> > > > > > You should not call send_sig_info() directly; instead call
> > > > > > raise_exception(). It already does the work you need (including some
> > > > > > things you left out).
> > > > >
> > > > > raise_exception() assumes the setting of a new state, and I did not want to do this,
> > > > > since the same does not happen when throwing a signal from userspace.
> > > >
> > > > Userspace isn't supposed to send the USR1 signal, only the INT, TERM, or
> > > > KILL signals. USR1 is supposed to be reserved for the driver's internal
> > > > use. Unfortunately, AFAIK there's no way to allow the driver to send a
> > > > signal to itself without also allowing the signal to be sent by
> > > > userspace. :-(
> > >
> > > It's funny that you actually helped me solve my problem thanks to this undocumented
> > > behavior. If it were not for the ability to send a signal, I would not be able to make
> > > the necessary code, and my software would always be waiting for the completion of IO.
> > >
> > > So here I am grateful to you - I didn't have to patch the kernel a few years ago,
> > > and now I just want to turn it into a clear feature :)
> > >
> > > Given the needs of the userspace code, maybe the suggested "break_io"
> > > would be the best choice?
> >
> > It sounds like what you really want is a combination of both "interrupt
> > I/O" and "forced eject".
>
> Indeed. But I didn't want to introduce some complex entities into the "file" attribute
> or make magic prefixes for the image name or something. So I suggested
> "echo > break_io && echo > file". This will not break the current behavior of the drive.
Does the host continue to issue "constant" I/O after the broken command? If
so, wouldn't that prevent your forced ejection from happening "immediately"?
If not, why not?
> > > > And sending the signal _does_ set a new state, whether you intended to
> > > > or not. Although in this case, the new state is always the same as the
> > > > old state, i.e., FSG_STATE_NORMAL.
> > >
> > > So I could call raise_exception(fsg->common, FSG_STATE_NORMAL) instead of sending
> > > the signal from break_io handler. There will be a slight difference
> > > in exception_req_tag and exception_arg, but it does not seem to cause any side effects.
> > > Please correct me if I'm wrong.
> >
> > In fact, the best approach would be to introduce a new state (let's call
> > it FSG_STATE_FORCED_EJECT) with priority just above
> > FSG_STATE_ABORT_BULK_OUT. You would call raise_exception with
> > FSG_STATE_FORCED_EJECT, not FSG_STATE_NORMAL. handle_exceptions() would
> > treat this state partially like ABORT_BULK_OUT in that it would avoid
> > resetting all the LUN data values and would call send_status_common() if
> > a command had been underway. But in addition it would do the forced
> > eject.
>
> Do you mean something like this?
>
> if (old_state != FSG_STATE_ABORT_BULK_OUT) {
> for (i = 0; i < ARRAY_SIZE(common->luns); ++i) {
> curlun = common->luns[i];
> if (!curlun)
> continue;
> curlun->prevent_medium_removal = 0;
> if (old_state != FSG_STATE_FORCED_EJECT) {
> curlun->sense_data = SS_NO_SENSE;
> curlun->unit_attention_data = SS_NO_SENSE;
> curlun->sense_data_info = 0;
> curlun->info_valid = 0;
> }
> }
> }
Sort of.
> > Also, the sysfs routine should be careful to see whether the command
> > currently being executed is for the LUN being ejected. I guess you
> > have never tried issuing your USR1 signal to a mass-storage gadget
> > running more than one LUN. If you did, you would find that it clears
> > the prevent_medium_removal flag for all of them, not just the one that
> > you wanted.
>
> I haven't tried it, but I figured it out along the way when I discovered
> the SIGUSR1 feature. I perceive it as something that should work that way.
> Like, we hit the whole device.
But that's not how real devices work. If you have a multi-LUN DVD drive,
for instance, sticking an unfolded paperclip into one of the manual-eject
holes will eject only one of the discs, not all of them.
Alan Stern
