Attachment 45214 Details for Bug 34710 – [PATCH 2/4] Add "unmount" option to DriveEject

[patch] [PATCH 2/4] Add "unmount" option to DriveEject

0002-Add-unmount-option-to-DriveEject.patch (text/plain), 11.02 KB, created by Martin Pitt on 2011-04-04 03:18:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Pitt

Created: 2011-04-04 03:18:14 UTC

Size: 11.02 KB

patch

obsolete

>From a4176b7081a8fd438a83ec14d107ecf7ed89a484 Mon Sep 17 00:00:00 2001
>From: Martin Pitt <martin.pitt@ubuntu.com>
>Date: Mon, 4 Apr 2011 00:07:03 +0200
>Subject: [PATCH 2/4] Add "unmount" option to DriveEject
>
>DriveEject currently fails if there are mounted file systems on the drive. Add
>an option "unmount" which will unmount any mounted file system on the drive to
>be ejected first. The "eject" command conveniently already does this, so the
>only thing we need to do here is to skip the "is mounted" check in
>device_local_is_busy().
>
>This is a preparation for bug #34710: When pressing the Eject button, we do not
>want to (and neither need to) run FilesystemUnmount() first, wait for that to
>complete, and then run "eject".
>---
> data/org.freedesktop.UDisks.Device.xml |    4 +-
> src/device.c                           |   70 +++++++++++++++++++-------------
> 2 files changed, 44 insertions(+), 30 deletions(-)
>
>diff --git a/data/org.freedesktop.UDisks.Device.xml b/data/org.freedesktop.UDisks.Device.xml
>index 8b76ccb..991976d 100644
>--- a/data/org.freedesktop.UDisks.Device.xml
>+++ b/data/org.freedesktop.UDisks.Device.xml
>@@ -1112,13 +1112,13 @@
>     <method name="DriveEject">
>       <annotation name="org.freedesktop.DBus.GLib.Async" value=""/>
>       <arg name="options" direction="in" type="as">
>-        <doc:doc><doc:summary>Eject options. Currently no options are recognized.</doc:summary></doc:doc>
>+        <doc:doc><doc:summary>Eject options. Valid options currently include only 'unmount'.</doc:summary></doc:doc>
>       </arg>
> 
>       <doc:doc>
>         <doc:description>
>           <doc:para>
>-            Ejects media from the device.
>+            Ejects media from the device. If the <doc:term>unmount</doc:term> option is given, the device might have mounted file systems, which will be unmounted before ejection.
>           </doc:para>
>         </doc:description>
>         <doc:permission>
>diff --git a/src/device.c b/src/device.c
>index a928fb5..fef8ee8 100644
>--- a/src/device.c
>+++ b/src/device.c
>@@ -87,6 +87,7 @@ static void drain_pending_changes (Device *device,
> 
> static gboolean device_local_is_busy (Device *device,
>                                       gboolean check_partitions,
>+                                      gboolean check_mounted,
>                                       GError **error);
> 
> static gboolean device_local_partitions_are_busy (Device *device);
>@@ -4837,6 +4838,7 @@ update_info (Device *device)
>  * device_local_is_busy:
>  * @device: A #Device.
>  * @check_partitions: Whether to check if partitions is busy if @device is a partition table
>+ * @check_mounted: Whether to check if device has mounted file systems
>  * @error: Either %NULL or a #GError to set to #ERROR_BUSY and an appropriate
>  * message, e.g. "Device is busy" or "A partition on the device is busy" if the device is busy.
>  *
>@@ -4847,6 +4849,7 @@ update_info (Device *device)
> static gboolean
> device_local_is_busy (Device *device,
>                       gboolean check_partitions,
>+                      gboolean check_mounted,
>                       GError **error)
> {
>   gboolean ret;
>@@ -4861,7 +4864,7 @@ device_local_is_busy (Device *device,
>     }
> 
>   /* or if we're mounted */
>-  if (device->priv->device_is_mounted)
>+  if (check_mounted && device->priv->device_is_mounted)
>     {
>       g_set_error (error, ERROR, ERROR_BUSY, "%s is mounted", device->priv->device_file);
>       goto out;
>@@ -4944,7 +4947,7 @@ device_local_partitions_are_busy (Device *device)
>           == 0)
>         {
> 
>-          if (device_local_is_busy (d, FALSE, NULL))
>+          if (device_local_is_busy (d, FALSE, TRUE, NULL))
>             {
>               ret = TRUE;
>               break;
>@@ -4976,7 +4979,7 @@ device_local_logical_partitions_are_busy (Device *device)
>           == 0 && g_strcmp0 (d->priv->partition_scheme, "mbr") == 0 && d->priv->partition_number >= 5)
>         {
> 
>-          if (device_local_is_busy (d, FALSE, NULL))
>+          if (device_local_is_busy (d, FALSE, TRUE, NULL))
>             {
>               ret = TRUE;
>               break;
>@@ -6401,7 +6404,7 @@ device_filesystem_mount_authorized_cb (Daemon *daemon,
>         }
>     }
> 
>-  if (device_local_is_busy (device, FALSE, &error))
>+  if (device_local_is_busy (device, FALSE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -7047,6 +7050,7 @@ device_drive_eject_authorized_cb (Daemon *daemon,
>   int n;
>   char *argv[16];
>   GError *error;
>+  gboolean unmount = FALSE;
> 
>   error = NULL;
> 
>@@ -7062,20 +7066,30 @@ device_drive_eject_authorized_cb (Daemon *daemon,
>       goto out;
>     }
> 
>-  if (device_local_is_busy (device, TRUE, &error))
>+  for (n = 0; options[n] != NULL; n++)
>     {
>-      throw_error (context, ERROR_BUSY, error->message);
>-      g_error_free (error);
>-      goto out;
>+      const char *option = options[n];
>+      if (strcmp ("unmount", option) == 0)
>+        {
>+          unmount = TRUE;
>+        }
>+      else
>+        {
>+          throw_error (context, ERROR_INVALID_OPTION, "Unknown option %s", option);
>+          goto out;
>+	}
>     }
> 
>-  for (n = 0; options[n] != NULL; n++)
>+  /* If we specify the unmount option, don't check if the device is mounted */
>+  if (device_local_is_busy (device, TRUE, !unmount, &error))
>     {
>-      const char *option = options[n];
>-      throw_error (context, ERROR_INVALID_OPTION, "Unknown option %s", option);
>+      throw_error (context, ERROR_BUSY, error->message);
>+      g_error_free (error);
>       goto out;
>     }
> 
>+  /* eject already unmounts the file systems on the ejected drive, so we do not
>+   * need any particular handling of the unmount flag here */
>   n = 0;
>   argv[n++] = "eject";
>   argv[n++] = device->priv->device_file;
>@@ -7182,7 +7196,7 @@ device_drive_detach_authorized_cb (Daemon *daemon,
>       goto out;
>     }
> 
>-  if (device_local_is_busy (device, TRUE, &error))
>+  if (device_local_is_busy (device, TRUE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -7402,7 +7416,7 @@ device_partition_delete_authorized_cb (Daemon *daemon,
>   part_number_as_string = NULL;
>   error = NULL;
> 
>-  if (device_local_is_busy (device, FALSE, &error))
>+  if (device_local_is_busy (device, FALSE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -7422,7 +7436,7 @@ device_partition_delete_authorized_cb (Daemon *daemon,
>       goto out;
>     }
> 
>-  if (device_local_is_busy (enclosing_device, FALSE, &error))
>+  if (device_local_is_busy (enclosing_device, FALSE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -7760,7 +7774,7 @@ device_filesystem_create_internal (Device *device,
>   passphrase_stdin = NULL;
>   error = NULL;
> 
>-  if (device_local_is_busy (device, TRUE, &error))
>+  if (device_local_is_busy (device, TRUE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -8274,7 +8288,7 @@ device_partition_create_authorized_cb (Daemon *daemon,
>       goto out;
>     }
> 
>-  if (device_local_is_busy (device, FALSE, &error))
>+  if (device_local_is_busy (device, FALSE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -8507,7 +8521,7 @@ device_partition_modify_authorized_cb (Daemon *daemon,
>       goto out;
>     }
> 
>-  if (device_local_is_busy (enclosing_device, FALSE, &error))
>+  if (device_local_is_busy (enclosing_device, FALSE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -8751,7 +8765,7 @@ device_partition_table_create_authorized_cb (Daemon *daemon,
> 
>   error = NULL;
> 
>-  if (device_local_is_busy (device, TRUE, &error))
>+  if (device_local_is_busy (device, TRUE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -9067,7 +9081,7 @@ device_luks_unlock_internal (Device *device,
> 
>   daemon_local_get_uid (device->priv->daemon, &uid, context);
> 
>-  if (device_local_is_busy (device, FALSE, &error))
>+  if (device_local_is_busy (device, FALSE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -9697,7 +9711,7 @@ device_filesystem_set_label_authorized_cb (Daemon *daemon,
> 
>   if (!fs_details->supports_online_label_rename)
>     {
>-      if (device_local_is_busy (device, FALSE, &error))
>+      if (device_local_is_busy (device, FALSE, TRUE, &error))
>         {
>           dbus_g_method_return_error (context, error);
>           g_error_free (error);
>@@ -9755,7 +9769,7 @@ device_filesystem_set_label (Device *device,
> 
>   if (!fs_details->supports_online_label_rename)
>     {
>-      if (device_local_is_busy (device, FALSE, &error))
>+      if (device_local_is_busy (device, FALSE, TRUE, &error))
>         {
>           dbus_g_method_return_error (context, error);
>           g_error_free (error);
>@@ -10433,7 +10447,7 @@ device_linux_md_add_spare_authorized_cb (Daemon *daemon,
>    * hot adding a new disk if an old one failed
>    */
> 
>-  if (device_local_is_busy (slave, TRUE, &error))
>+  if (device_local_is_busy (slave, TRUE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -10584,7 +10598,7 @@ device_linux_md_expand_authorized_cb (Daemon *daemon,
>           goto out;
>         }
> 
>-      if (device_local_is_busy (slave, TRUE, &error))
>+      if (device_local_is_busy (slave, TRUE, TRUE, &error))
>         {
>           dbus_g_method_return_error (context, error);
>           g_error_free (error);
>@@ -10714,7 +10728,7 @@ linux_md_remove_component_device_changed_cb (Daemon *daemon,
>   if (device == data->slave)
>     {
> 
>-      if (device_local_is_busy (data->slave, FALSE, &error))
>+      if (device_local_is_busy (data->slave, FALSE, TRUE, &error))
>         {
>           dbus_g_method_return_error (data->context, error);
>           g_error_free (error);
>@@ -11134,7 +11148,7 @@ daemon_linux_md_start_authorized_cb (Daemon *daemon,
>             }
>         }
> 
>-      if (device_local_is_busy (slave, FALSE, &error))
>+      if (device_local_is_busy (slave, FALSE, TRUE, &error))
>         {
>           dbus_g_method_return_error (context, error);
>           g_error_free (error);
>@@ -11490,7 +11504,7 @@ daemon_linux_md_create_authorized_cb (Daemon *daemon,
>           goto out;
>         }
> 
>-      if (device_local_is_busy (slave, FALSE, &error))
>+      if (device_local_is_busy (slave, FALSE, TRUE, &error))
>         {
>           dbus_g_method_return_error (context, error);
>           g_error_free (error);
>@@ -13730,7 +13744,7 @@ daemon_linux_lvm2_vg_add_pv_authorized_cb (Daemon *daemon,
>     }
> 
>   error = NULL;
>-  if (device_local_is_busy (pv, TRUE, &error))
>+  if (device_local_is_busy (pv, TRUE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>@@ -13860,7 +13874,7 @@ daemon_linux_lvm2_vg_remove_pv_authorized_cb (Daemon *daemon,
>     }
> 
>   error = NULL;
>-  if (device_local_is_busy (pv, TRUE, &error))
>+  if (device_local_is_busy (pv, TRUE, TRUE, &error))
>     {
>       dbus_g_method_return_error (context, error);
>       g_error_free (error);
>-- 
>1.7.4.1
>

Actions: View | Splinter Review

Attachments on bug 34710: 43791 | 43798 | 43846 | 45212 | 45214 | 45215 | 45216 | 45307 | 45308 | 45319 | 47050 | 47051 | 47052 | 47053 | 47983 | 47984 | 47985 | 48169

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.