neil / rpms / udisks2

Forked from rpms/udisks2 a year ago
Clone
Blob Blame History Raw
From 50b7a2125f77f23901eff12c76efb80881236513 Mon Sep 17 00:00:00 2001
From: Marius Vollmer <mvollmer@redhat.com>
Date: Thu, 23 Nov 2017 11:40:50 +0200
Subject: [PATCH] Add 'no-discard' option to formatting methods

It unfortunately seems to be necessary to use this option in certain
situations.  See https://bugzilla.redhat.com/show_bug.cgi?id=1516041
for example.

(cherry picked from commit 26ac7429b5ef9b5024e54e6eca25b685c0666eb5)

Conflicts:
    src/udiskslinuxblock.c

Signed-off-by: Vratislav Podzimek <vpodzime@redhat.com>
---
 data/org.freedesktop.UDisks2.xml |  4 ++++
 src/tests/integration-test       | 14 +++++++-----
 src/udiskslinuxblock.c           | 48 ++++++++++++++++++++++++++++------------
 src/udiskslinuxfsinfo.c          | 31 ++++++++++++++++++--------
 src/udiskslinuxfsinfo.h          |  1 +
 5 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/data/org.freedesktop.UDisks2.xml b/data/org.freedesktop.UDisks2.xml
index fa53a72..1f71ca1 100644
--- a/data/org.freedesktop.UDisks2.xml
+++ b/data/org.freedesktop.UDisks2.xml
@@ -1404,6 +1404,10 @@
         this allows a deeper check of the parameters even when
         <parameter>no-block</parameter> is %TRUE.
 
+        If the option <parameter>no-discard</parameter> is set to
+        %TRUE then Udisks tells the formatting utility not to issue
+        BLKDISCARD ioctls.
+
         If the option <parameter>config-items</parameter> is set, it
         should be an array of configuration items suitable for
         org.freedesktop.UDisks2.Block.AddConfigurationItem.  They will
diff --git a/src/tests/integration-test b/src/tests/integration-test
index f863578..0edbb6a 100755
--- a/src/tests/integration-test
+++ b/src/tests/integration-test
@@ -825,6 +825,8 @@ class FS(UDisksTestCase):
             self._do_udisks_check(fs_type, 'test%stst' % fs_type)
             # also test fs_create with an empty label
             self._do_udisks_check(fs_type, '')
+            # also test fs_create with the no-discard option
+            self._do_udisks_check(fs_type, '', True)
 
     def _do_cli_check(self, fs_type, label=None):
         """udisks correctly picks up file system changes from command line tools"""
@@ -890,15 +892,17 @@ class FS(UDisksTestCase):
         subprocess.call(['umount', mount_a])
         self.assertProperty(fs, 'mount-points', [])
 
-    def _do_udisks_check(self, fs_type, label=None):
+    def _do_udisks_check(self, fs_type, label=None, no_discard=None):
         """udisks API correctly changes file system"""
 
+
         # create fs
+        options = { }
         if label is not None:
-            options = GLib.Variant('a{sv}', {'label': GLib.Variant('s', label)})
-        else:
-            options = no_options
-        self.fs_create(None, fs_type, options)
+            options['label'] = GLib.Variant('s', label);
+        if no_discard is not None:
+            options['no-discard'] = GLib.Variant('b', no_discard);
+        self.fs_create(None, fs_type, GLib.Variant('a{sv}', options))
 
         # properties
         b_id = self.blkid()
diff --git a/src/udiskslinuxblock.c b/src/udiskslinuxblock.c
index 3d56577..0de5422 100644
--- a/src/udiskslinuxblock.c
+++ b/src/udiskslinuxblock.c
@@ -2656,6 +2656,27 @@ add_blocksize (gchar        **command,
   return TRUE;
 }
 
+static gchar *
+build_command (const gchar *template,
+               const gchar *device,
+               const gchar *label,
+               const gchar *options,
+               GError     **error)
+{
+  gchar *tmp, *tmp2, *command;
+  tmp = udisks_daemon_util_subst_str_and_escape (template, "$DEVICE", device);
+  tmp2 = udisks_daemon_util_subst_str_and_escape (tmp, "$LABEL", label != NULL ? label : "");
+  command = udisks_daemon_util_subst_str (tmp2, "$OPTIONS", options != NULL ? options : "");
+  g_free (tmp);
+  g_free (tmp2);
+  if (strstr (command, "$BLOCKSIZE") && ! add_blocksize (&command, device, error))
+    {
+      g_free (command);
+      return NULL;
+    }
+
+  return command;
+}
 
 void
 udisks_linux_block_handle_format (UDisksBlock             *block,
@@ -2679,8 +2700,8 @@ udisks_linux_block_handle_format (UDisksBlock             *block,
   const gchar *action_id;
   const gchar *message;
   const FSInfo *fs_info;
+  const gchar *command_options = NULL;
   gchar *command = NULL;
-  gchar *tmp;
   gchar *error_message;
   GError *error;
   int status;
@@ -2700,6 +2721,7 @@ udisks_linux_block_handle_format (UDisksBlock             *block,
   const gchar *partition_type = NULL;
   GVariant *config_items = NULL;
   gboolean teardown_flag = FALSE;
+  gboolean no_discard_flag = FALSE;
   BDPartTableType part_table_type = BD_PART_TABLE_UNDEF;
 
   error = NULL;
@@ -2723,6 +2745,7 @@ udisks_linux_block_handle_format (UDisksBlock             *block,
   g_variant_lookup (options, "dry-run-first", "b", &dry_run_first);
   g_variant_lookup (options, "config-items", "@a(sa{sv})", &config_items);
   g_variant_lookup (options, "tear-down", "b", &teardown_flag);
+  g_variant_lookup (options, "no-discard", "b", &no_discard_flag);
 
   partition = udisks_object_get_partition (object);
   if (partition != NULL)
@@ -2886,21 +2909,20 @@ udisks_linux_block_handle_format (UDisksBlock             *block,
       goto out;
     }
 
+  if (no_discard_flag && fs_info->option_no_discard)
+    command_options = fs_info->option_no_discard;
+
   /* If requested, check whether the ultimate filesystem creation
      will succeed before actually getting to work.
   */
   if (dry_run_first && fs_info->command_validate_create_fs)
     {
       const gchar *device = udisks_block_get_device (block);
-      tmp = udisks_daemon_util_subst_str_and_escape (fs_info->command_validate_create_fs, "$DEVICE",
-                                                     device);
-      command = udisks_daemon_util_subst_str_and_escape (tmp, "$LABEL", label != NULL ? label : "");
-      g_free (tmp);
-      if (strstr (command, "$BLOCKSIZE") && ! add_blocksize (&command, device, &error))
-        {
-          handle_format_failure (invocation, error);
-          goto out;
-        }
+      command = build_command (fs_info->command_validate_create_fs, device, label, command_options, &error);
+      if (command == NULL) {
+            handle_format_failure (invocation, error);
+            goto out;
+      }
 
       if (!udisks_daemon_launch_spawned_job_sync (daemon,
                                                     object,
@@ -3081,10 +3103,8 @@ udisks_linux_block_handle_format (UDisksBlock             *block,
       {
         /* Build and run mkfs shell command */
         const gchar *device = udisks_block_get_device (block_to_mkfs);
-        tmp = udisks_daemon_util_subst_str_and_escape (fs_info->command_create_fs, "$DEVICE", device);
-        command = udisks_daemon_util_subst_str_and_escape (tmp, "$LABEL", label != NULL ? label : "");
-        g_free (tmp);
-        if (strstr (command, "$BLOCKSIZE") && ! add_blocksize (&command, device, &error))
+        command = build_command (fs_info->command_create_fs, device, label, command_options, &error);
+        if (command == NULL)
           {
             handle_format_failure (invocation, error);
             goto out;
diff --git a/src/udiskslinuxfsinfo.c b/src/udiskslinuxfsinfo.c
index b7834c0..1556af3 100644
--- a/src/udiskslinuxfsinfo.c
+++ b/src/udiskslinuxfsinfo.c
@@ -70,8 +70,9 @@ const FSInfo _fs_info[] =
       NULL,
       TRUE,  /* supports_online_label_rename */
       TRUE,  /* supports_owners */
-      "mkfs.ext2 -F -L $LABEL $DEVICE",
-      "mkfs.ext2 -n -F -L $LABEL $DEVICE",
+      "mkfs.ext2 -F -L $LABEL $OPTIONS $DEVICE",
+      "mkfs.ext2 -n -F -L $LABEL $OPTIONS $DEVICE",
+      "-E nodiscard", /* option_no_discard */
     },
     {
       FS_EXT3,
@@ -79,8 +80,9 @@ const FSInfo _fs_info[] =
       NULL,
       TRUE,  /* supports_online_label_rename */
       TRUE,  /* supports_owners */
-      "mkfs.ext3 -F -L $LABEL $DEVICE",
-      "mkfs.ext3 -n -F -L $LABEL $DEVICE",
+      "mkfs.ext3 -F -L $LABEL $OPTIONS $DEVICE",
+      "mkfs.ext3 -n -F -L $LABEL $OPTIONS $DEVICE",
+      "-E nodiscard", /* option_no_discard */
     },
     {
       FS_EXT4,
@@ -88,8 +90,9 @@ const FSInfo _fs_info[] =
       NULL,
       TRUE,  /* supports_online_label_rename */
       TRUE,  /* supports_owners */
-      "mkfs.ext4 -F -L $LABEL $DEVICE",
-      "mkfs.ext4 -n -F -L $LABEL $DEVICE",
+      "mkfs.ext4 -F -L $LABEL $OPTIONS $DEVICE",
+      "mkfs.ext4 -n -F -L $LABEL $OPTIONS $DEVICE",
+      "-E nodiscard", /* option_no_discard */
     },
     {
       FS_VFAT,
@@ -99,6 +102,7 @@ const FSInfo _fs_info[] =
       FALSE, /* supports_owners */
       "mkfs.vfat -I -n $LABEL $DEVICE",
       NULL,
+      NULL, /* option_no_discard */
     },
     {
       FS_NTFS,
@@ -108,6 +112,7 @@ const FSInfo _fs_info[] =
       FALSE, /* supports_owners */
       "mkntfs -f -F -L $LABEL $DEVICE",
       "mkntfs -n -f -F -L $LABEL $DEVICE",
+      NULL, /* option_no_discard */
     },
     {
       FS_EXFAT,
@@ -117,6 +122,7 @@ const FSInfo _fs_info[] =
       FALSE, /* supports_owners */
       "mkexfatfs -n $LABEL $DEVICE",
       NULL,
+      NULL, /* option_no_discard */
     },
     {
       FS_XFS,
@@ -124,8 +130,9 @@ const FSInfo _fs_info[] =
       "xfs_admin -L -- $DEVICE",
       FALSE, /* supports_online_label_rename */
       TRUE,  /* supports_owners */
-      "mkfs.xfs -f -L $LABEL $DEVICE",
-      "mkfs.xfs -N -f -L $LABEL $DEVICE"
+      "mkfs.xfs -f -L $LABEL $OPTIONS $DEVICE",
+      "mkfs.xfs -N -f -L $LABEL $OPTIONS $DEVICE",
+      "-K" /* option_no_discard */
     },
     {
       FS_REISERFS,
@@ -135,6 +142,7 @@ const FSInfo _fs_info[] =
       TRUE,  /* supports_owners */
       "mkfs.reiserfs -q -l $LABEL $DEVICE",
       NULL,
+      NULL, /* option_no_discard */
     },
     {
       FS_NILFS2,
@@ -144,6 +152,7 @@ const FSInfo _fs_info[] =
       TRUE,  /* supports_owners */
       "mkfs.nilfs2 -L $LABEL $DEVICE",
       NULL,
+      NULL, /* option_no_discard */
     },
     {
       FS_BTRFS,
@@ -151,8 +160,9 @@ const FSInfo _fs_info[] =
       NULL,
       FALSE, /* supports_online_label_rename */
       TRUE,  /* supports_owners */
-      "mkfs.btrfs -L $LABEL $DEVICE",
+      "mkfs.btrfs -L $LABEL $OPTIONS $DEVICE",
       NULL,
+      "-K", /* option_no_discard */
     },
     {
       FS_MINIX,
@@ -162,6 +172,7 @@ const FSInfo _fs_info[] =
       FALSE, /* supports_owners */
       "mkfs.minix $DEVICE",
       NULL,
+      NULL,
     },
     {
       FS_UDF,
@@ -171,6 +182,7 @@ const FSInfo _fs_info[] =
       TRUE,  /* supports_owners */
       "mkudffs --utf8 --media-type=hd --udfrev=0x201 --blocksize=$BLOCKSIZE --vid $LABEL --lvid $LABEL $DEVICE",
       NULL,
+      NULL,
     },
     {
       FS_F2FS,
@@ -180,6 +192,7 @@ const FSInfo _fs_info[] =
       TRUE,  /* supports_owners */
       "mkfs.f2fs -l $LABEL $DEVICE",
       NULL,
+      NULL,
     },
     /* swap space */
     {
diff --git a/src/udiskslinuxfsinfo.h b/src/udiskslinuxfsinfo.h
index 2daec10..6a7a541 100644
--- a/src/udiskslinuxfsinfo.h
+++ b/src/udiskslinuxfsinfo.h
@@ -35,6 +35,7 @@ typedef struct
   gboolean     supports_owners;
   const gchar *command_create_fs;  /* should have $DEVICE and $LABEL */
   const gchar *command_validate_create_fs;  /* should have $DEVICE and $LABEL */
+  const gchar *option_no_discard;
 } FSInfo;
 
 const FSInfo  *get_fs_info (const gchar *fstype);
-- 
2.9.5