From 50b7a2125f77f23901eff12c76efb80881236513 Mon Sep 17 00:00:00 2001 From: Marius Vollmer 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 --- 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 no-block is %TRUE. + If the option no-discard is set to + %TRUE then Udisks tells the formatting utility not to issue + BLKDISCARD ioctls. + If the option config-items 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