Bug 38535

Summary: A DeviceAutoMountHint attribute should be added to udisks
Product: udisks Reporter: ayan george <ayan>
Component: detectionAssignee: David Zeuthen (not reading bugmail) <zeuthen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Patch 1/4 - src/device-private.c
Patch 2/4 - src/device-private.h
Patch 3/4 - data/org.freedesktop.UDisks.Device.xml
Patch 4/4 - src/device.c

Description ayan george 2011-06-21 11:32:24 UTC 
We need a way to explicitly add a hint that a device should be automounted.

This can be done by having udisks expose the udev value UDISKS_AUTOMOUNT_HINT  as the DeviceAutoMountHint DBus property.

The possible values for DeviceAutoMountHint are either blank, "never", or "always".

gvfs-gdu-volume-monitor should, in turn, use this value to determine how the should_automount property is set.

The purpose would be to be able to insist that a device gets automounted and opened by Nautilus regardless of which bus it is on in a semantically clear way.
Comment 1 ayan george 2011-06-21 18:11:51 UTC 
Created attachment 48260 [details] [review]
Patch 1/4 - src/device-private.c
Comment 2 ayan george 2011-06-21 18:12:30 UTC 
Created attachment 48261 [details] [review]
Patch 2/4 - src/device-private.h
Comment 3 ayan george 2011-06-21 18:13:16 UTC 
Created attachment 48262 [details] [review]
Patch 3/4 - data/org.freedesktop.UDisks.Device.xml
Comment 4 ayan george 2011-06-21 18:13:53 UTC 
Created attachment 48263 [details] [review]
Patch 4/4 - src/device.c
Comment 5 David Zeuthen (not reading bugmail) 2011-06-23 10:10:39 UTC 
Note: it doesn't make any sense to split this into four patches since

 - it breaks compilation if patches are in the wrong order
   (and in this case they are)
 - it breaks bisectability
 - one logical change is one commit, nothing more, nothing less
Comment 6 David Zeuthen (not reading bugmail) 2011-06-23 10:11:52 UTC 
(In reply to comment #3)
> Created an attachment (id=48262) [details]
> Patch 3/4 - data/org.freedesktop.UDisks.Device.xml

+            An empty string is interpreted to mean "never".

We definitely don't want this - I'm replacing it with

  An empty string is interpreted to mean that there is no
  hint - the desktop auto-mounter should make its own
  decision of whether to auto-mount the device.
Comment 7 David Zeuthen (not reading bugmail) 2011-06-23 10:16:34 UTC 
The patches don't include additions to the udisks(7) man page. I'm adding this

      <varlistentry>
        <term><option>UDISKS_AUTOMOUNT_HINT</option></term>
        <listitem><para>
          A variable to influence whether a device should be automounted.
          Possible values include "always" (to hint that a device should
          always be automounted) and "never" (to hint that a device should
          never be automounted). Note that this is only a hint - the
          auto-mounter might not honor it.
        </para></listitem>
      </varlistentry>
Comment 8 David Zeuthen (not reading bugmail) 2011-06-23 10:22:05 UTC 
(In reply to comment #1)
> Created an attachment (id=48260) [details]
> Patch 1/4 - src/device-private.c

+  if (G_UNLIKELY (g_strcmp0(device->priv->device_auto_mount_hint,value)))

 - missing space between g_strcmp0 and opening parenthesis
 - missing space after comma
 - improper use of int as a boolean - you want to use != 0 explicitly

I've changed it to this

  if (G_UNLIKELY (g_strcmp0 (device->priv->device_auto_mount_hint, value) != 0))
Comment 9 David Zeuthen (not reading bugmail) 2011-06-23 10:25:31 UTC 
(In reply to comment #4)
> Created an attachment (id=48263) [details]
> Patch 4/4 - src/device.c

+  const char *auto_mount_hint = "";

We generally don't mix initializers and declarations. I've moved this down to before the if. I've also changed this to gchar (the rest of the code isn't consistent with char vs gchar yet so I don't blame you for using char).

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.