GNOME Bugzilla – Bug 678595
PATCH: Add an automount-inhibit flag
Last modified: 2012-08-03 17:08:37 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.
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...
(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.
Ping, any progress on this? I would like to move forward with bug 678597 but that depends on this patch getting merged.
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?
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.
Review of attachment 218001 [details] [review]: Ok.
(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 on attachment 218001 [details] [review] PATCH: Add an automount-inhibit flag Pushed as 70725a0c33
Can we also add this to GtkApplication please?
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.
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.
(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.