Thanks for the report,
as there are user jobs attached to UDisks but running in Disks I don't know at the moment whether all this can be handled in UDisks but probably there is a way.

Adding it per call for each operation in Disks sounds like a mess and it's maybe more feasible to only consider long-running stuff.
However I don't know yet how the session presents the warning if suspend is invoked manually - if Disks has more than one inhibit call at the same time, does it look strange? Maybe a singleton with atomic state changes is needed then.
But on the other hand the jobs are queried anyway for the GUI updates, so a generic way could also be found.

Any opinions?

I don't know!

systemd inhibits:
https://www.freedesktop.org/wiki/Software/systemd/inhibit/

If the session is triggering the suspend, then it makes more sense to prevent this from GNOME Disks and not UDisks, I think.
But from reading the docs I do not understand which one is appropriate. Blocking IDLE is described for automatic suspending but blocks the sceen lock as well.
Blocking SLEEP/SUSPEND works as well for automatic suspending, right?

What will happen as soon as it gets unblocked? There may be a race between the suspend and the next blocking if blocking is done per job (e.g. a full whipe and a format operation after each other). Or are there some seconds before the suspend would start?

I think I'm in favor of the noisy solution and adding the calls everywhere manually. If the call is missing somewhere, the bug is then more obvious. Still there must be a single global state because a finishing job on one drive should not allow suspend if one more is still running.

(In reply to Kai Lüke from comment #4)
> If the session is triggering the suspend, then it makes more sense to
> prevent this from GNOME Disks and not UDisks, I think.
> But from reading the docs I do not understand which one is appropriate.
> Blocking IDLE is described for automatic suspending but blocks the sceen
> lock as well.

No, blocking IDLE doesn't block the screen lock. It blocks the screen lock coming on as a result of IDLE.

> Blocking SLEEP/SUSPEND works as well for automatic suspending, right?

It will block automatic suspending, but will also block requests by the user to suspend.

> What will happen as soon as it gets unblocked? There may be a race between
> the suspend and the next blocking if blocking is done per job (e.g. a full
> whipe and a format operation after each other). Or are there some seconds
> before the suspend would start?

Idle is the only one affected here, the others blocked action not being the result of a timeout. There's a fix for that at:
https://gitlab.gnome.org/GNOME/mutter/merge_requests/54

I don't think there's any problems in doubling up the inhibit calls. I'm guessing udisks doesn't give up formatting drives if the front-end quits, right, and as it runs as a system service, best would be to inhibit at the system level, using systemd. This inhibition will be mirrored by gnome-session.

Thanks for the hints and background info!
In fact UDisks is already having some inhibit calls (only erase and format) and they are just missing for the other cases.
Except for disk cloning which is a job running in GNOME Disks that probably needs to be handled in Disks unless UDisks takes care for user jobs as well (I have to check if that would even be possible).

https://github.com/storaged-project/udisks/issues/516

UDisks jobs are covered in UDisks with this patch:
https://github.com/storaged-project/udisks/pull/524

For the local jobs in GNOME Disks I will also try to post a fix soon…
Instead of covering benchmark, clone, restore manually, I intend to add it to the local job creation and destruction code.

Created attachment 370979 [details] [review]
Register inhibitor for benchmark thread

Disk clone and restore already had their inhibitors, so I don't understand why the QA reported that it did not work for clone - maybe it was something else?

Therefore I would just register an inhibitor for the benchmark thread, which is the only missing part.

(Sidenote: The benchmarking code does not use local jobs and needs to keep the dialog open, a GUI redesign could change that in order to allow concurrent benchmarking if anybody cares about that feature.)

Note:

(In reply to Kai Lüke from comment #9)
> Disk clone and restore already had their inhibitors, so I don't understand
> why the QA reported that it did not work for clone - maybe it was something
> else?

There are more operations than just a clone&restore, like a performance test, or a SMART check. We can check clone again.

Yes, just testing for disk image cloneing/restoring is enough, thanks.
Since it uses the same code as the Epiphany download task to inhibit the session, I think either both or none will work in your test system.

For the performance benchmarking I attached a patch, so this will be fixed.
All others excep clone/restore will be covered in UDisks with a pending patch (only format/erase were covered up to now).

The patch adds a new string, but this string is not displayed anywhere to my knowledge, because gnome-session-binary just exposes a general "user session inhibited" as a summary for all its own inhibitors to systemd-inhibit.
Does this justify inclusion in the 3.28.2 release despite of the string freeze?

Ah, did not see it before, but GNOME Shell's shutdown dialog shows at least one of the current inhibitor reasons, thus it really adds a new translatable string.

Review of attachment 370979 [details] [review]:

Thanks, looks ok!

::: src/disks/gdubenchmarkdialog.c
@@ +1178,3 @@
+                                            GTK_APPLICATION_INHIBIT_LOGOUT,
+                                            /* Translators: Reason why suspend/logout is being inhibited */
+                                            C_("create-inhibit-message", "Benchmarking device"));

For stable branches, I would rather use already translated "Benchmark" instead of freeze break...

Ah, thanks, I did not think about making separate patches, that's really better.