Fix loop device setup

[tbzatek]

This will trigger extra uevent after loop device setup to ensure all objects are updated.

Resolves #800
Resolves #817
Resolves #580
Resolves #828

[vojtechtrefny]

Looks good to me.

[tbzatek]

Changed wording of an error message...

Anyway, I'm still thinking about reworking uevent trigger a little bit... Historically we've been getting sysfs path from a GUdevDevice instance, either by holding a reference within UDisksLinuxBlockObject or by querying actual udev db. Only to get a sysfs path for the device, the actual trigger was always done by writing to that sysfs file.

What we can do is to bypass udev and construct the sysfs path manually, avoiding potential race from udev holding either old device path or none (yet) at all. In most of the cases the path construction would be pretty straightforward, however we might need to resolve device symlinks in some cases. In particular the lvm2 module will hit the issue (see my last commit in #814).

[vpodzime]

Hate to see things like this being added to UDisks, but it's the only realistic way of fixing things. ACK.

[tbzatek]

Hate to see things like this being added to UDisks, but it's the only realistic way of fixing things. ACK.

Yeah, we tend to generate too many uevents, basically every intrusive operation is followed by explicit uevent trigger. That's in place already over majority of the damon methods. It's the most reliable way however to ensure everything is up-to-date.

[gramozeka]

I put together the suggested fix, but the bug persists.
maybe i'm doing something wrong or missing some dependencies, but it doesn't work

udisksctl loop-setup -r -f image.iso Error setting up loop device for image.iso: GDBus.Error:org.freedesktop.UDisks2.Error.Failed: Error waiting for loop object after creating '/dev/loop2': Timed out waiting for object
I attach the conclusion strace maybe this will help in finding a solution.
test.txt

PS// update. I experimented a little - the problem turned out to be in the 5.8.12 kernel, having rolled back to the stable 5.4.80 kernel everything worked fine.

[tbzatek]

@gramozeka, have you built and ran the daemon with the fix? strace-ing udisksctl is pointless as it's just a D-Bus client.

[gramozeka]

have you built and ran the daemon with the fix?... is pointless as it's just a D-Bus client.

of course, I understand how it works (I hope the insanity did not finish me), udisks does not work in dolphin (kf5)&nautilus(gnome-3.38) aka "connect disk image" with new kernels (I did not even touch the 5.9.XX branch), I used this to get debug. And yes, I used a branch udisks-8d5c90a1f4bbe548a1ae361f8d69970669d6e72e, the problem is not in the code udisks, the problem is new kernel breaks. With the kernel 5.4.XX everything works fine, both the stable version udisks-2.9.1 and that version with fixes.
I don't know what the kernel's writers have done, obviously crooked hands are a steady trend of our time)(perhaps this is a diversion? windiws10 is always everywhere-into the firebox linux)))

here is the output strace with a working kernel(in atached)
test.txt

[vojtechtrefny]

Still looks good to me. Hope it actually fixes the issues :-)

Could we add some test case for this? (Something more reliable than the "randomly broken" test_loop_ro from #800?)

[tbzatek]

TODO: https://github.com/storaged-project/udisks/blob/master/src/udisksstate.c#L631

[tbzatek]

Could we add some test case for this? (Something more reliable than the "randomly broken" test_loop_ro from #800?)

Hmm, do you have any suggestion? I can't think of any test that would actually raise the error to test the negative case. This commit fixes the org.freedesktop.UDisks2.Manager.LoopSetup() method call that is tested via several test_loop.UdisksLoopDeviceTest tests.

[tbzatek]

Actually found another tiny bug in my code, hard to hit in real use case scenarios though...