After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 678595 - PATCH: Add an automount-inhibit flag
PATCH: Add an automount-inhibit flag
Status: RESOLVED FIXED
Product: gnome-session
Classification: Core
Component: gnome-session
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Session Maintainers
Session Maintainers
Depends on:
Blocks: 678597
 
 
Reported: 2012-06-22 06:50 UTC by Hans de Goede
Modified: 2012-08-03 17:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
PATCH: Add an automount-inhibit flag (2.91 KB, patch)
2012-06-22 06:50 UTC, Hans de Goede
reviewed Details | Review
PATCH: Add an automount-inhibit flag (2.93 KB, patch)
2012-07-04 10:13 UTC, Hans de Goede
committed Details | Review

Description Hans de Goede 2012-06-22 06:50:13 UTC
Created attachment 216994 [details] [review]
PATCH: Add an automount-inhibit flag

When connecting to virtual machines with usb-device redirection, such as Spice enabled vms, automount may get in the way. Specifically if auto-usbredir is enabled in the vm-viewer, then the usbredir code and the automount code race for who gets to the device first.

If the automount code wins the race this is a problem, since usbredir causes a device-disconnect (iow the usb mass storage driver sees an unplug), so in the end usbredir always wins, and we end up with a non clean potentially corrupt filesystem. Also see:
https://bugzilla.redhat.com/show_bug.cgi?id=812972

There for the need exists to be able to inhibit gnome-shell's automounting, since all other inhibits run through gnome-session, it seems consistent to store the inhibit flag for automounting in gnome-session too. This has the added advantage that gnome-session already has all the necessary logic to automatically uninhibit if a client goes away.

Another reason to do this through gnome-session is that all other inhibit flags are accessible through GtkApplication which relies on gnome-session,  so storing this in gnome-session allows to add it to GtkApplication too.

Note that gnome-session does not do anything with the flag, other then tracking it. The reason to still modify gnome-session is to reserve and document the flag.
Comment 1 Matthias Clasen 2012-06-22 11:46:46 UTC
Doing it this way, there's still a race though, no ?

I mean, when you launch a session, automounting will not be inhibited, you need to start an application that takes an inhibitor. And that will still race with other things starting up and trying to automount...
Comment 2 Hans de Goede 2012-06-22 15:06:42 UTC
(In reply to comment #1)
> Doing it this way, there's still a race though, no ?
> 
> I mean, when you launch a session, automounting will not be inhibited, you need
> to start an application that takes an inhibitor. And that will still race with
> other things starting up and trying to automount...

The race is between usb device auto-redirection, which is managed by the spice-client and automount. The spice client first inhibits auto-mounting, and only then starts listening for new devices to autoredirect, 

IOW the process doing the inhibit is one of the 2 racing parties.
Comment 3 Hans de Goede 2012-07-02 09:51:14 UTC
Ping, any progress on this? I would like to move forward with bug 678597 but that depends on this patch getting merged.
Comment 4 Colin Walters 2012-07-02 20:30:57 UTC
Review of attachment 216994 [details] [review]:

We don't really have a clear line for what applications should be able to do, but it seems to me like there should be a better way to handle this.  I mean, is there a use case for an application suspending removable media automount *other* than virtual machine redirection?

I don't have a better idea though than this, but it sure is ugly.  For example, it will prohibit *all* automounting, but what if the virt stack gains support for only redirecting *specific* USB devices (determined by vendor ID or whatever)?  

Anyways...let's just add it.

::: gnome-session/gsm-inhibitor.h
@@ +55,3 @@
         GSM_INHIBITOR_FLAG_SUSPEND     = 1 << 2,
+        GSM_INHIBITOR_FLAG_IDLE        = 1 << 3,
+        GSM_INHIBITOR_FLAG_AUTOMOUNT   = 1 << 4,

Drop the final trailing comma; it's a GNU C extension, and I think Sun CC barfs on it.

::: gnome-session/org.gnome.SessionManager.xml
@@ +149,3 @@
+              <doc:item>
+                <doc:term>16</doc:term>
+                <doc:definition>Inhibit auto-mounting for the session</doc:definition>

Maybe add "removable media" there?
Comment 5 Hans de Goede 2012-07-04 10:13:03 UTC
Created attachment 218001 [details] [review]
PATCH: Add an automount-inhibit flag

(In reply to comment #4)
> ::: gnome-session/gsm-inhibitor.h
> @@ +55,3 @@
>          GSM_INHIBITOR_FLAG_SUSPEND     = 1 << 2,
> +        GSM_INHIBITOR_FLAG_IDLE        = 1 << 3,
> +        GSM_INHIBITOR_FLAG_AUTOMOUNT   = 1 << 4,
> 
> Drop the final trailing comma; it's a GNU C extension, and I think Sun CC barfs
> on it.
> 
> ::: gnome-session/org.gnome.SessionManager.xml
> @@ +149,3 @@
> +              <doc:item>
> +                <doc:term>16</doc:term>
> +                <doc:definition>Inhibit auto-mounting for the
> session</doc:definition>
> 
> Maybe add "removable media" there?

Here is a new version fixing both.
Comment 6 Colin Walters 2012-07-05 17:47:25 UTC
Review of attachment 218001 [details] [review]:

Ok.
Comment 7 Hans de Goede 2012-07-06 06:16:11 UTC
(In reply to comment #6)
> Review of attachment 218001 [details] [review]:

Thanks for the review, I don't have commit rights, can you commit this for me?

Thanks,

Hans
Comment 8 drago01 2012-07-06 07:14:52 UTC
Comment on attachment 218001 [details] [review]
PATCH: Add an automount-inhibit flag

Pushed as 70725a0c33
Comment 9 David Zeuthen (not reading bugmail) 2012-08-03 15:50:37 UTC
Can we also add this to GtkApplication please?
Comment 10 David Zeuthen (not reading bugmail) 2012-08-03 16:14:58 UTC
Btw, another possible class of API users include applications dealing with importing stuff from removable media such as CF cards. E.g. a photo management application may want to avoid

 1. The media being automounted
 2. "Open in Shotwell" notification

Such an application would simply

 a. take the automount inhibitor lock
 b. watch for "volume-added" events and call g_volume_mount() on the volume
    if g_volume_should_automount() is TRUE
 c. examine the media - if there is nothing of interest to the app, stop here
 d. otherwise, do its own in-app notification saying "importing from media"
 e. actually do the import (copy files)
 f. g_mount_eject() to eject the media
 g. tell the user itself "the media is now safe to remove"
 h. give up the inhibitor lock

See https://plus.google.com/u/0/110773474140772402317/posts/SAv66HXtnvZ for an example of a real-world app that wants something like this.
Comment 11 Ray Strode [halfline] 2012-08-03 16:51:16 UTC
That works if there's a separate import dialog because you can drop the lock when the dialog goes away, but I think importing would ideally happen automatically if a stick is inserted and the app is open.  That implies the need for more coordination than just a lock, though, since it probably shouldn't inhibit auto-mounting of non-photo removable media, and also there's the question of what to do if, e.g., Disks and Photos, or Photos and Shotwell are open at the same time.
Comment 12 David Zeuthen (not reading bugmail) 2012-08-03 17:08:37 UTC
(In reply to comment #11)
> That works if there's a separate import dialog because you can drop the lock
> when the dialog goes away, 

Right.

> but I think importing would ideally happen
> automatically if a stick is inserted and the app is open.  That implies the
> need for more coordination than just a lock, though, since it probably
> shouldn't inhibit auto-mounting of non-photo removable media, and also there's
> the question of what to do if, e.g., Disks and Photos, or Photos and Shotwell
> are open at the same time.

It's a bit different from what Hans originally wanted (avoid mounting the device at all)... I mean, for this, what really want a way to inhibit the autorun handling, not automounting.

We certainly could add arbitration API to GVolumeManager like this

 interface GAutorunHandler {
   bool should_inhibit_autorun(GMount mount);
 };

where your app implement this interface (it could be a GSignal to avoid subclassing). This allows your app to examine the mount and allow the default policy if it's not interested...

You'd then add these new GVolumeManager methods

 // would need both sync and async versions
 void register_autorun_handler(GAutorunHandler handler);
 void unregister_autorun_handler(GAutorunHandler handler);
 bool check_autorun_inhibited();

so each application can register a handler (first come, first served).

With this, the desktop automounter (e.g. gnome-shell) then simply calls GVolumeManager.check_autorun_inhibited() after it mounts a GVolume but before it does any autorun handling. If check_autorun_inhibited() returns TRUE, the shell doesn't present any notification.

To ensure that things work session-wide even with non-proxy GVolumeMonitor implementations, we'd only implement this on the GUnionVolumeMonitor type (which is what g_volume_monitor_get() returns) and which would use a simple session D Bus service to orchestrate multiple processes. Something like that, I think.