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