After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 676555 - File system checking, repair and resize
File system checking, repair and resize
Status: RESOLVED FIXED
Product: gnome-disk-utility
Classification: Core
Component: Disks UI
3.0.x
Other Linux
: Normal major
: ---
Assigned To: gnome-disk-utility-maint
gnome-disk-utility-maint
: 644380 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-05-22 13:12 UTC by nodiscc
Modified: 2017-09-04 01:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Detect UDisks version 2.7.2 (1.04 KB, patch)
2017-07-18 09:35 UTC, Kai Lüke
committed Details | Review
Move partition size helpers (5.47 KB, patch)
2017-07-18 09:36 UTC, Kai Lüke
committed Details | Review
Resize, Check and Repair Dialogs (70.40 KB, patch)
2017-07-18 09:45 UTC, Kai Lüke
none Details | Review
Resize, Check and Repair Dialogs (72.59 KB, patch)
2017-07-19 15:30 UTC, Kai Lüke
none Details | Review
Resize, Check and Repair Dialogs (70.87 KB, patch)
2017-07-23 22:20 UTC, Kai Lüke
none Details | Review
Resize, Check and Repair Dialogs (71.40 KB, patch)
2017-08-01 20:14 UTC, Kai Lüke
none Details | Review
Resize and repair helper functions (11.25 KB, patch)
2017-08-05 10:31 UTC, Kai Lüke
committed Details | Review
Resize, Check and Repair Dialogs (67.70 KB, patch)
2017-08-05 10:35 UTC, Kai Lüke
none Details | Review
Resize, Check and Repair Dialogs (69.29 KB, patch)
2017-08-07 14:03 UTC, Kai Lüke
none Details | Review
Resize, Check and Repair Dialogs (69.47 KB, patch)
2017-08-07 14:44 UTC, Kai Lüke
committed Details | Review
Correct tooltip texts (2.68 KB, patch)
2017-08-08 12:55 UTC, Kai Lüke
committed Details | Review

Description nodiscc 2012-05-22 13:12:53 UTC
I know this is a limitation of the fsck utility, but could we have a dialog box that allows the user to schedule a check at next boot - like touch /forcefsck does ?

At the moment ther is no way to check / and /home partitions from the GUI, these are rather important partitions don't you think?

Thanks
Comment 1 Mantas Kriaučiūnas 2012-10-02 08:58:13 UTC
It seems Gnome-disk-utility developers removed "check for errors" possibility in versions 3.4-3.6 :(
It's a pity, because lots of people used this and also wish to have a possibility to check mounted partitions, see for example this bugreport:
https://launchpad.net/ubuntu/+source/gnome-disk-utility/+bug/761478
Comment 2 David Zeuthen (not reading bugmail) 2012-11-15 18:04:23 UTC
File-system integrity checking is a very complicated topic and the current state-of-the-art is a kinda mess - for example, the fact that you can't do online fsck is really a mess. See

 https://plus.google.com/110933625728671692704/posts/8VHxeVy6Wzs

for some interesting discussion. I didn't want to deal with this mess in the udisks1 -> udisks2 transition so I simply removed the feature since it didn't add much value; for example, running 'sudo fsck /dev/sda2' is a much better experience, at least that way you get some output!

My gut feeling is that we probably should do *something* but I'm not entirely sure what it should be except for launching a root shell in a terminal. I'm going to repurpose this bug for investigating this. Hope you don't mind.
Comment 3 David Zeuthen (not reading bugmail) 2012-11-15 18:30:16 UTC
*** Bug 644380 has been marked as a duplicate of this bug. ***
Comment 4 Alan 2016-08-13 15:40:58 UTC
This feature would be useful for Tails, as we used to use it in our doumentation, see https://labs.riseup.net/code/issues/8070.
Comment 5 Stuart Axon 2017-01-11 15:13:12 UTC
Even if this only works for removable filesystems to start with, it would be pretty useful (I'd imagine people want to check / fix them way more than their permanently mounted ones).
Comment 6 Michael Catanzaro 2017-01-11 22:45:11 UTC
Hey, unfortunately nobody has been working actively on Disks for several years now, but I'm happy to review patches if anybody wants to try implementing this!
Comment 7 Kai Lüke 2017-07-18 09:35:41 UTC
Created attachment 355807 [details] [review]
Detect UDisks version 2.7.2
Comment 8 Kai Lüke 2017-07-18 09:36:04 UTC
Created attachment 355808 [details] [review]
Move partition size helpers
Comment 9 Kai Lüke 2017-07-18 09:45:42 UTC
Created attachment 355810 [details] [review]
Resize, Check and Repair Dialogs

If supported by UDisks the resize and
repair actions for filesystems (and partitions)
are offered in the volume gear menu. As long
running actions their completion is indicated by
a system notification.

Job progress and canceling is not implemented in UDisks, as well as NTFS support but there is nothing to do from GNOME Disks for that.

Only offline check and repair is supported because online error detection/recovery in a filesystem is something different. But the 'touch /forefsck' idea can be added for the root fs.
Comment 10 Kai Lüke 2017-07-18 16:45:25 UTC
Just in case someone encounters it: the FAT resize support of libparted refuses to work in some cases, maybe libblockdev should switch to calling the fatresize binary ("GNU Parted cannot resize this partition to this size" or the less obvious "Failed to read the filesystem on the device").
Comment 11 Kai Lüke 2017-07-18 20:26:07 UTC
A general problem is that the partition resize followed by the filesystem resize is sometimes to quick and UDisks does not have the filesystem interface ready yet. Maybe some busy waiting with timeout callbacks needed?
Comment 12 Ondrej Holy 2017-07-19 09:36:55 UTC
Review of attachment 355807 [details] [review]:

I don't have experiences with meson, but looks ok. Just the output is a bit confusing since we actually need udisks and udisks is found, but in older version, but the output is:
Native dependency udisks2 found: NO found '2.7.1' but need: '>= 2.7.2'
Dependency udisks2 found: NO

So not sure we can do something with or not...
Comment 13 Ondrej Holy 2017-07-19 09:36:59 UTC
Review of attachment 355808 [details] [review]:

Looks ok, thanks!
Comment 14 Ondrej Holy 2017-07-19 09:37:10 UTC
Review of attachment 355810 [details] [review]:

It looks overall pretty good. I am not just really satisfied with the notifications. I think that the result displayed only thru the notifications is not sufficient (they may be e.g. disabled), maybe we need some in-app progress indicator as Nautilus has... 

It would be again much better to add each feature in a separate patch (ie. resize, check, repair)...

The following warnings needs to be fixed:
[22/32] Compiling C object 'src/disks/gnome-disks@exe/gduresizedialog.c.o'.
../../gnome-disk-utility/src/disks/gduresizedialog.c: In function ‘calc_usage’:
../../gnome-disk-utility/src/disks/gduresizedialog.c:670:31: warning: passing argument 1 of ‘gtk_button_set_label’ from incompatible pointer type [-Wincompatible-pointer-types]
         gtk_button_set_label (data->apply, _("Fit to size"));
                               ^~~~
In file included from /usr/include/gtk-3.0/gtk/gtk.h:54:0,
                 from ../../gnome-disk-utility/src/disks/gduapplication.h:13,
                 from ../../gnome-disk-utility/src/disks/gduresizedialog.c:15:
/usr/include/gtk-3.0/gtk/gtkbutton.h:123:23: note: expected ‘GtkButton * {aka struct _GtkButton *}’ but argument is of type ‘GtkWidget * {aka struct _GtkWidget *}’
 void                  gtk_button_set_label          (GtkButton      *button,
                       ^~~~~~~~~~~~~~~~~~~~

::: src/disks/gduresizedialog.c
@@ +18,3 @@
+#include "gduutils.h"
+
+#ifdef HAVE_UDISKS2_7_2

This should be in makefile instead...

::: src/disks/gduresizedialog.h
@@ +15,3 @@
+G_BEGIN_DECLS
+
+void     gdu_resize_dialog_show (GduWindow    *window,

nitpick: I would remove the multiple spaces before the identificator, because it is probably too few anyway to align with new functions with some longer return type...

::: src/disks/gduwindow.c
@@ +331,3 @@
+static void on_generic_menu_item_check (GtkMenuItem *menu_item,
+                                        gpointer     user_data);
+

The empty lines are redundant here...

@@ +2776,3 @@
+      entry = gdu_utils_can_resize (window->client, type, FALSE);
+      if (entry != NULL && entry->available && !read_only)
+        show_flags->volume_menu |= SHOW_FLAGS_VOLUME_MENU_RESIZE;

Those options should not be shown at all if HAVE_UDISKS2_7_2 is not set...

@@ +3071,3 @@
+    mount_points = udisks_filesystem_get_mount_points (filesystem);
+
+  if (filesystem == NULL || g_strv_length ((gchar **) mount_points) > 0)

Shouldn't this logic be part of the dialog itself?

@@ +3118,3 @@
+        {
+          s = g_strdup_printf (_("Filesystem %s on %s has been repaired"),
+                               name, udisks_object_info_get_name (info));

This is slightly confusing when the filesystem was ok and did not need to be repaired...

Shouldn't be the repair operation offered only if the check failed? But this is not really possible with the current UI...

@@ +3194,3 @@
+                           "Consider backing it up first in order to use forensic recovery tools\n"
+                           "that retrieve lost files.\n"
+                           "Depending on the amount of data this operation takes longer time."));

The text should not be wrapped manually, this doesn't work correctly with translations etc. Max-width-chars should be used instead...

@@ +3234,3 @@
+
+      object = (UDisksObject *) g_dbus_interface_get_object (G_DBUS_INTERFACE (filesystem));
+      block = udisks_object_peek_block (object);

Shouldn't be the returned values checked whether they are != NULL to be bullet proof...? Or is it ok? Applies on other places also...

@@ +3265,3 @@
+                     gpointer          user_data)
+{
+#ifdef HAVE_UDISKS2_7_2

Should be before the function because...

@@ +3295,3 @@
+
+static void
+on_generic_menu_item_check (GtkMenuItem *menu_item,

...all those new functions should be guarded by HAVE_UDISKS2_7_2 because it should never be called without HAVE_UDISKS2_7_2...

@@ +3305,3 @@
+  g_assert (object != NULL);
+
+

Double new line...

@@ +3306,3 @@
+
+
+  dialog = gtk_dialog_new_with_buttons (_("Confirm Check"),

I think that gtk_message_dialog should be used for such tasks, but worth to check with designers... same for repair...

::: src/disks/ui/resize-dialog.ui
@@ +18,3 @@
+    <property name="page_increment">10</property>
+  </object>
+  <object class="GtkDialog" id="resize-dialog">

Missing dialog title...

::: src/libgdu/gduutils.c
@@ +791,3 @@
+gdu_utils_can_resize (UDisksClient *client, const gchar *fstype, gboolean flush)
+{
+  static GHashTable *cache = NULL;

I am not really sure about the need for cache... wouldn't be better to just simply call udisks_manager_call_can_resize for the concrete filesystem?  Is the "flush" parameter a preparation for some future PackageKit integration?

@@ +796,3 @@
+#endif
+
+  if (flush && cache != NULL)

This is a bit in conflict with g_once_init...

@@ +804,3 @@
+  if (g_once_init_enter (&cache))
+    {
+      GHashTable *init_cache = NULL;

\n

@@ +810,3 @@
+      for (gsize i = 0; supported_fs[i] != NULL; i++)
+        {
+          GVariant *out_available;

\n

::: src/libgdu/gduutils.h
@@ +64,3 @@
 
+typedef enum {
+  OFFLINE_SHRINK = 1 << 1,

1 << 0?

@@ +79,3 @@
+const UtilCacheEntry *gdu_utils_can_resize (UDisksClient *client,
+                                            const gchar  *fstype,
+                                            gboolean      flush);

Wouldn't be better to avoid creating new structures and do simply the following?
gboolean gdu_utils_can_resize (UDisksClient *client,
const gchar *fstype,
gboolean flush,
ResizeFlags *mode_out,
gchar **missing_util_out);
Comment 15 Kai Lüke 2017-07-19 10:04:43 UTC
Thanks, yes, the notifications are just a quick solution, the nautilus-like idea is good and I try to propose it as change to the pending mockups.

There is a difference in running a read-only check on a dirty-marked filesystem which reports no problems (it's just marked dirty but not damaged), and running a repair on that same filesystem in order to really clean up the dirty-marker. Otherwise some tools will not want to work on this non-clean filesystem.
Comment 16 Kai Lüke 2017-07-19 15:30:23 UTC
Created attachment 355959 [details] [review]
Resize, Check and Repair Dialogs
Comment 17 Ondrej Holy 2017-07-20 12:20:50 UTC
Review of attachment 355959 [details] [review]:

Thanks for the update :-)

Still not sure what to do with the notifications. I am not against easy solutions for beginning, but I am not really satisfied with the notifications (can be disabled, text can be trimmed, can disappear etc.). Maybe I would rather simply show message dialog with the result for the beginning (similarly as in case of error) instead of the notifications... and I think that resize doesn't need notifications (at least not now), it is basically the same as any other operation currently. So, you should not care about the issue with the new window from notifications for now :-). What do you think?

The callbacks are real hell :-D I am thinking about some universal callback(s) with finite-state machine (not-mandatory just an idea :-). However, you can merge fs_repair_next_fs_resize_cb and fs_repair_next_depends_cb, and also fs_resize_no_repair_next_is_part_cb and part_resize_next_is_fs_resize_no_repair_cb at least...

Also, we should probably prevent resizing of partitions inside LUKS, it seems it is not working correctly...

When I was testing the functionality, the first check call crashed on, not sure why:
GNOME-Disks:ERROR:../../gnome-disk-utility/src/disks/gduwindow.c:3208:fs_check_cb: assertion failed: (object != NULL)

::: src/disks/gduresizedialog.c
@@ +642,3 @@
+
+  if (!data->running)
+    {

You should save the source_id and remove it when destroying instead of this...

@@ +762,3 @@
+  gtk_style_context_add_class (gtk_widget_get_style_context (data->apply), "destructive-action");
+  gtk_widget_grab_default (data->apply);
+

It is common to add some function like setup_dialog for the following assignments and others in order to simplify resize_dialog_show...

@@ +785,3 @@
+
+  if (data->max_size == data->current_size)
+    {

It is confusing that in some cases Difference is shown and in some not... I think we should show it in all cases.

@@ +845,3 @@
+          mount_points = udisks_filesystem_get_mount_points (data->filesystem);
+
+          if (g_strv_length ((gchar **) mount_points) > 0 &&

Can mount_points be empty? Shouldn't offline branch be called also in that case?

@@ +897,3 @@
+  else
+    {
+      object = (UDisksObject *) g_dbus_interface_get_object (G_DBUS_INTERFACE (filesystem));

Since we rely on that object != NULL, we should rather use UDISKS_OBJECT() instead of casting, which will abort processing if it is not... applies on other places also...

::: src/disks/meson.build
@@ +33,3 @@
 
+if config_h.get('HAVE_UDISKS2_7_2')
+  sources += files('gduresizedialog.c')

resource_data += files('ui/resize-dialog.ui')?

@@ +34,3 @@
+if config_h.get('HAVE_UDISKS2_7_2')
+  sources += files('gduresizedialog.c')
+endif

Nice! How does "make dist" works with meson? Will those files be distributed correctly regardless HAVE_UDISKS2_7_2?

::: src/libgdu/gduutils.c
@@ +806,3 @@
+  UtilCacheEntry *result;
+
+  if (flush && cache != NULL)

g_once_init_enter suggests that this function may be called from more threads, but this statement is not guarded and may lead to undefined results... and I think that "flush" is a bit in conflict with g_once definition... G_LOCK?

@@ +843,3 @@
+    {
+      if (mode_out != NULL)
+        *mode_out = result->mode;

Maybe you could do it as following in order to save some lines:
result = g_hash_table_lookup (cache, fstype);
if (mode_out != NULL)
  *mode_out = result ? result->mode : NULL;
...

@@ +846,3 @@
+
+      if (missing_util_out != NULL)
+        *missing_util_out = result->missing_util == NULL ? NULL : g_strdup (result->missing_util);

It is safe to use NULL in g_strdup, so...
*missing_util_out = g_strdup (result->missing_util);

@@ +947,3 @@
+
+          if (udisks_manager_call_can_check_sync (udisks_client_get_manager (client),
+                                                 supported_fs[i], &out_available, NULL, NULL))

Wrongly aligned...

::: src/libgdu/gduutils.h
@@ +65,3 @@
+#ifdef HAVE_UDISKS2_7_2
+
+/* Defined by libblockdev/UDisks */

Ok :-)
Comment 18 Kai Lüke 2017-07-23 22:20:51 UTC
Created attachment 356248 [details] [review]
Resize, Check and Repair Dialogs

As discussed the notifications are gone for now and resizing is limited to cases where Disks can also resize the underlying block.
Excluding the ui resource file has to be done with the xml file and I think it's ok just to ship it. The meson dist command correctly takes all files from the git repository.
There are some race conditions for the case where first the partition is resized and then the filesystem because the reread of the partition table lets the whole partition device node gone for some time and then reappear, so I need to introduce appropriate waiting before returning from UDisks Partition.Resize().
Comment 19 Ondrej Holy 2017-07-26 07:32:17 UTC
Review of attachment 356248 [details] [review]:

The race conditions need to be settled down before pushing, but I think it is more-or-less ready to push otherwise :-) Just the planned redesign should do something with the message dialogs for check/repair results... just an idea, but is it really necessary to use them in the case of success currently?

::: src/disks/gduresizedialog.c
@@ +65,3 @@
+  g_object_unref (data->block);
+  if (data->filesystem != NULL)
+    g_object_unref (data->filesystem);

You can use g_clear_object (&data->filesystem) instead if-statement...

@@ +641,3 @@
+   * the LUKS, LVM partition or the mounted loop device file.
+   */
+  g_assert (data->partition != NULL);

assert?

@@ +826,3 @@
+                                    NULL, /* cancellable */
+                                    (GAsyncReadyCallback) resize_get_usage_mount_cb,
+                                    g_object_ref (window));

The mount operation may take a pretty much time... the dialog should be probably shown anyway, but with spinner...

::: src/libgdu/gduutils.c
@@ +794,3 @@
+}
+
+G_LOCK_DEFINE (can_resize_lock);

\n

@@ +811,3 @@
+    {
+      g_hash_table_destroy (cache);
+      cache = NULL;

Btw. you can use g_clear_pointer in such cases...

@@ +837,3 @@
+    }
+
+  G_UNLOCK (can_resize_lock);

I would add \n after G_UNLOCK and remove \n before it...

@@ +843,3 @@
+
+  if (missing_util_out != NULL)
+        *missing_util_out = result ? g_strdup (result->missing_util) : NULL;

Wrong indentation, same for other functions...
Comment 20 Ondrej Holy 2017-07-26 10:58:20 UTC
(In reply to Ondrej Holy from comment #19)
> Review of attachment 356248 [details] [review] [review]:
> ...
> @@ +641,3 @@
> +   * the LUKS, LVM partition or the mounted loop device file.
> +   */
> +  g_assert (data->partition != NULL);
> 
> assert?

It is ok, it is handled correctly in gduwindow.c... so please ignore this note.
Comment 21 Kai Lüke 2017-08-01 20:14:24 UTC
Created attachment 356753 [details] [review]
Resize, Check and Repair Dialogs

Good, thanks for the style feedback and which functions GLib provides for this.
I also added handling the mount callback while the dialog is opened and it was not easy but I hope the solution is ok now.

For the race condition there is a UDisks PR but it's not ideal due to not changing the library ABI — Bad luck it was released before this issue came up…
Comment 22 Ondrej Holy 2017-08-02 10:32:29 UTC
Review of attachment 356753 [details] [review]:

Thanks for update, it is almost ready, just...

::: src/disks/gduresizedialog.c
@@ +57,3 @@
+};
+
+struct MountErrorData

It should work, but I think that this structure is a bit redundant and it can be handled nicer. You can add reference counter for ResizeDialogData and also it would be nice to use GCancellable for udisks_filesystem_call_mount. Grep GNOME Disks codes for ref_count for inspiration...

@@ +607,3 @@
+      data->running_id = 0;
+
+      if (data->support & ONLINE_SHRINK || data->support & OFFLINE_SHRINK)

Wouldn't be better to move this if-statement in gdu_resize_dialog_show and do not call calc_usage at all in this case?

@@ +746,3 @@
+    {
+      gtk_widget_set_sensitive (data->apply, FALSE);
+      data->min_size = data->current_size; /* do not allow shrinking initially */

I think you need to call resize_dialog_update (data) in order to not allow shrinking initially... but maybe we should make the scales hidden until we know the minimal size, because it is a bit confusing. But this can be probably improved later once we have feedback from designers.

@@ +812,3 @@
+            }
+          else if ((!(data->support & ONLINE_SHRINK) && shrinking) ||
+                   (!(data->support & ONLINE_GROW) && !shrinking))

Btw are we sure that OFFLINE_ is always supported at this point if ONLINE_ is not?

::: src/disks/ui/resize-dialog.ui
@@ +68,3 @@
+        </child>
+        <child>
+          <object class="GtkSpinner" id="spinner">

Maybe a better place for the spinner would be in the headerbar next to the "Confirm" button, I have seen this somewhere. But this can be improved later once we have feedback from designers.
Comment 23 Kai Lüke 2017-08-05 10:31:58 UTC
Created attachment 356993 [details] [review]
Resize and repair helper functions
Comment 24 Kai Lüke 2017-08-05 10:35:26 UTC
Created attachment 356994 [details] [review]
Resize, Check and Repair Dialogs

Ok, some corner cases were fixed and the appearance of the slider reflects the relations between current, minimal and maximal size.
Comment 25 Ondrej Holy 2017-08-07 08:56:18 UTC
Review of attachment 356993 [details] [review]:

Looks ok to me apart from some nitpicking... :-)

Please be more descriptive in a commit message. It should be obvious what the patch does, so that you add some new functions...

nitpick: the size helpers might be in a separate patch, but feel free to ignore it this time...

::: src/libgdu/gduutils.c
@@ +1578,3 @@
+      UDisksPartition *tmp_partition = UDISKS_PARTITION (l->data);
+      guint64 start;
+      guint64 end;

nitpick: guint64 start, end;

@@ +1585,3 @@
+      start = udisks_partition_get_offset (tmp_partition);
+      end = start + udisks_partition_get_size (tmp_partition);
+      if (end > current_end && (end < next_pos))

nitpick: end > current_end && end < next_pos

@@ +1587,3 @@
+      if (end > current_end && (end < next_pos))
+        next_pos = end;
+      if (start >= current_end && (start < next_pos))

dtto
Comment 26 Ondrej Holy 2017-08-07 08:56:25 UTC
Review of attachment 356994 [details] [review]:

I love the improved scale widget! Great work! But have some notes...

Why are some jobs titled as "Uknown" (eg. "Uknown (filesystem-repair)")? Isn't some patch for udisks needed?

Spinner seems to be stopped when waiting for mount, but not sure why...

I would say that the space between the scale and the first spin button should be the same as between the description label and the scale.
Also the scale should have same left and right margins as the description label... but I am not designer.

The dialog spacing isn't correct with non-standard font sizes, but this can be fixed later...

::: src/disks/gduresizedialog.c
@@ +187,3 @@
+  context = gtk_widget_get_style_context (data->size_scale);
+  if (data->css_provider)
+    gtk_style_context_remove_provider (context, data->css_provider);

g_clear_object (&data->css_provider);

@@ +439,3 @@
+  ResizeDialogData *data = user_data;
+
+  if (data->wait_for_filesystem < 20 && udisks_object_peek_filesystem (data->object) == NULL)

Please add some define on the top of file instead of hard-coding the values here...

@@ +443,3 @@
+      data->wait_for_filesystem++;
+      return G_SOURCE_CONTINUE;
+    }

else if (udisks_object_peek_filesystem (data->object) == NULL)
{
gdu_utils_show_error ...?
return G_SOURCE_REMOVE;
}

@@ +471,3 @@
+    {
+      udisks_client_settle (gdu_window_get_client (data->window));
+      if (resize_filesystem_waiter (data) == G_SOURCE_CONTINUE)

Please add comment, why this is needed... ideally with some relevant udisks bug.

@@ +473,3 @@
+      if (resize_filesystem_waiter (data) == G_SOURCE_CONTINUE)
+        {
+          g_timeout_add (500, resize_filesystem_waiter, data);

Please add some define on the top of file instead of hard-coding the values here...

@@ +645,3 @@
+  unused = gdu_utils_get_unused_for_block (data->client, data->block);
+
+  if (unused == -1)

Maybe we want to limit the number of retries same as in resize_filesystem_waiter for sure...

@@ +785,3 @@
+      if (calc_usage (data) == G_SOURCE_CONTINUE)
+        {
+          data->running_id = g_timeout_add (500, calc_usage, data);

dtto

::: src/disks/ui/gdu.css
@@ +32,3 @@
+	padding-left: 0px;
+	border-style: solid;
+	border-color: @warning_color;

I would prefer just gray (or red). The orange from Adwaita is a bit in contrast with the red "Resize" button, but it is just my opinion, I am ok if designers approved this...
Comment 27 Ondrej Holy 2017-08-07 08:58:04 UTC
Review of attachment 356994 [details] [review]:

I've made some tests and I can reproduce some wrong behavior (on my 8GB flash disk) using the following steps:

1) create two partitions with FAT
2) shrink the first one
3) grow the first one to the initial size
4) "Error repairing filesystem after resize
Error repairing filesystem on /dev/sdb1: 
Process reported exit code 1: Seek to 8178892288: Invalid argument
(udisks-error-quark, 0)"

(Consequently, the resize dialog shows wrong partition size 18446744070076...)

Can you please take a look what is wrong?
Comment 28 Ondrej Holy 2017-08-07 09:01:06 UTC
(In reply to Ondrej Holy from comment #27)
> Review of attachment 356994 [details] [review] [review]:
> 
> I've made some tests and I can reproduce some wrong behavior (on my 8GB
> flash disk) using the following steps:
> 
> 1) create two partitions with FAT
> 2) shrink the first one
> 3) grow the first one to the initial size
> 4) "Error repairing filesystem after resize
> Error repairing filesystem on /dev/sdb1: 
> Process reported exit code 1: Seek to 8178892288: Invalid argument
> (udisks-error-quark, 0)"
> 
> (Consequently, the resize dialog shows wrong partition size
> 18446744070076...)

You can also see "Size 1.3 GB — 3.6 GB free (1379779297428.8% full)" then...
Comment 29 Ondrej Holy 2017-08-07 10:59:37 UTC
(In reply to Ondrej Holy from comment #26)
> Review of attachment 356994 [details] [review] [review]:
> 
> I love the improved scale widget! Great work! But have some notes...
> 
> Why are some jobs titled as "Uknown" (eg. "Uknown (filesystem-repair)")?
> Isn't some patch for udisks needed?

s/Uknown/Unknown/
Comment 30 Ondrej Holy 2017-08-07 12:26:41 UTC
(In reply to Ondrej Holy from comment #28)
> (In reply to Ondrej Holy from comment #27)
> > Review of attachment 356994 [details] [review] [review] [review]:
> > 
> > I've made some tests and I can reproduce some wrong behavior (on my 8GB
> > flash disk) using the following steps:
> > 
> > 1) create two partitions with FAT
> > 2) shrink the first one
> > 3) grow the first one to the initial size
> > 4) "Error repairing filesystem after resize
> > Error repairing filesystem on /dev/sdb1: 
> > Process reported exit code 1: Seek to 8178892288: Invalid argument
> > (udisks-error-quark, 0)"
> > 
> > (Consequently, the resize dialog shows wrong partition size
> > 18446744070076...)
> 
> You can also see "Size 1.3 GB — 3.6 GB free (1379779297428.8% full)" then...

I don't see this issue with latest udisks and libblockdev, so probably ok...

(In reply to Ondrej Holy from comment #29)
> (In reply to Ondrej Holy from comment #26)
> > Review of attachment 356994 [details] [review] [review] [review]:
> > 
> > I love the improved scale widget! Great work! But have some notes...
> > 
> > Why are some jobs titled as "Uknown" (eg. "Uknown (filesystem-repair)")?
> > Isn't some patch for udisks needed?
> 
> s/Uknown/Unknown/

This seems to be also fixed with the latest udisks...
Comment 31 Kai Lüke 2017-08-07 14:03:32 UTC
Created attachment 357114 [details] [review]
Resize, Check and Repair Dialogs

Sometimes FAT resize refuses to work:
https://github.com/storaged-project/libblockdev/issues/265

And also the patch depends on this PR (or a similar solution) for dealing with the race condition:
https://github.com/storaged-project/libblockdev/pull/264
But if things are clear in UDisks we can maybe remove the waiting part for filesystems. But that would raise the requirement to an unknown UDisks release in the future.

The occupied part of the resize bar is now black.
Comment 32 Ondrej Holy 2017-08-07 14:22:32 UTC
Review of attachment 357114 [details] [review]:

Thanks for the update, looks ok to me apart from some nitpicking :-)

Please reconsider adding some minimal requirement for libblockdev also...

::: src/disks/gduresizedialog.c
@@ +18,3 @@
+#include "gduutils.h"
+
+# define FILESYSTEM_WAIT_STEP_MS 500

#define

@@ +19,3 @@
+
+# define FILESYSTEM_WAIT_STEP_MS 500
+# define FILESYSTEM_WAIT_SEC 10

dtto

@@ +198,3 @@
+    {
+      gtk_scale_add_mark (GTK_SCALE (data->size_scale), min_size_units, GTK_POS_BOTTOM, _("Minimal Size"));
+      css = g_strdup_printf (".partition-scale contents {\n  border-left-width: %dpx;\n}\n",

nitpick: forgot to say last time, but the following would be probably nicer:
ss = g_strdup_printf (".partition-scale contents {\n"
                      "  border-left-width: %dpx;\n"
                      "}\n",

@@ +501,3 @@
+       * of the partition interface to be the same and then wait for the filesystem
+       * interface to show up (only if it was there before). This depends on inclusion
+       * of PR 264 for libblockdev.

nitpick: It is better to add direct link.
Comment 33 Kai Lüke 2017-08-07 14:44:48 UTC
Created attachment 357121 [details] [review]
Resize, Check and Repair Dialogs

Added warning about possible data loss.
Comment 34 Ondrej Holy 2017-08-07 14:50:22 UTC
Review of attachment 357121 [details] [review]:

Thanks! Looks ok...
Comment 35 Kai Lüke 2017-08-07 21:47:23 UTC
Maybe it's good to leave this open until it's known if there is something to do after the next release of any of the dependencies.
Comment 36 Kai Lüke 2017-08-08 12:55:24 UTC
Created attachment 357189 [details] [review]
Correct tooltip texts

Some fix for more appropriate tooltip contents.
Comment 37 Ondrej Holy 2017-08-08 13:38:32 UTC
Review of attachment 357189 [details] [review]:

Looks ok, thanks. Please fix that also for new-disk-image-dialog.ui, create-partition-dialog.ui...
Comment 38 Kai Lüke 2017-08-08 23:06:28 UTC
Comment on attachment 357189 [details] [review]
Correct tooltip texts

Yes, the other dialogs would involve removing some of the unnecessary xml elements at the same time, so maybe this should be done for the whole files?
Comment 39 Ondrej Holy 2017-08-09 12:26:45 UTC
Not sure what you mean with removing unnecessary xml elements, but it would be good to just fix the tooltips first before string freeze and let other changes for the next devel cycle for sure...
Comment 40 Kai Lüke 2017-09-04 01:20:11 UTC
UDisks 2.7.3 got the fix to wait for the partition to reappear, so closing this now.
Implementing the progress information on the job should be tracked here: https://github.com/storaged-project/udisks/issues/402

New filesystem support requests need to go here: https://github.com/storaged-project/libblockdev