Bug 38535 - A DeviceAutoMountHint attribute should be added to udisks
Summary: A DeviceAutoMountHint attribute should be added to udisks
Status: RESOLVED FIXED
Alias: None
Product: udisks
Classification: Unclassified
Component: detection (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: medium normal
Assignee: David Zeuthen (not reading bugmail)
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-21 11:32 UTC by ayan george
Modified: 2011-06-23 10:30 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Patch 1/4 - src/device-private.c (1.00 KB, patch)
2011-06-21 18:11 UTC, ayan george
Details | Splinter Review
Patch 2/4 - src/device-private.h (1.20 KB, patch)
2011-06-21 18:12 UTC, ayan george
Details | Splinter Review
Patch 3/4 - data/org.freedesktop.UDisks.Device.xml (1.56 KB, patch)
2011-06-21 18:13 UTC, ayan george
Details | Splinter Review
Patch 4/4 - src/device.c (3.03 KB, patch)
2011-06-21 18:13 UTC, ayan george
Details | Splinter Review

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.