Dne 01. 02. 22 v 9:40 Martin Wilck napsal(a):
On Mon, 2022-01-31 at 14:33 +0100, Peter Rajnoha wrote:
(just discussed this with Zdenek too)
The patch makes sense to me!
We added all the DM_UDEV_PRIMARY_SOURCE_FLAG and related for exactly
such cases where we need to take the existing values already scanned
in previous event, main use-case being the trigger at boot. We just
didn't cover the 13-dm-disk.rules with the same logic regarding the
suspended state to keep the symlinks - I didn't think it would cause
issues (because, usually, after suspend, we anticipate incoming
resume where the device is scanned again).
But yes, if temporarily losing the symlink causes issues, your patch
solves that (Zdenek will push that upstream).
Thank you very much! It occured to me that if we want to solve my use
case with minimal risk, we could make the the case in which the
symlinks are preserved conditional on ACTION=="add" (i.e. true coldplug
events). Tell me if you'd prefer that, I can re-submit.
The problem is handling of 'suspended' state in udev rules - as I've mentioned
original no userland tool should mostly care about this.
However since there are those things like udev 'trigger' and it would be kind
of ugly if the 'left-over' suspended device would stop whole system operation
it's most likely better ATM to have some kind of support for this.
It's should be noted there is still 'race' risk of system freezing in the case
the suspend happens just after sysfs test and before actual i.e. 'blkid' use.
But let's assume the chance of having this to happen is pretty minimal, and
usually tools doing suspend & resume will work correctly and the 'crash' will
not likely happen in this particular moment - and suspended device just
hanging there already for longer time.
The missed issue to be solve is - that ALL rules have to rely on a single
suspend check - otherwise we risk 'partial' processing which is the last
thing we want to see (aka all or nothing).
Your real problem was not the suspend on its own - which still may happen
anytime (so i.e. just after the test whether device is suspended),
but the bug was related to bad exit path cleaning udev db content in this case
- which is clear bug. Next bug is that other rules have to be consistent and
properly skip its processing if the 1st. rules decides its meant to be skipped.
ATM your patch is already upstream and great thanks for pointing this out.
Regards
Zdenek
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
