Code review comment for ~raharper/curtin:fix/lvm-over-raid

Revision history for this message
Ryan Harper (raharper) wrote :

> is there a reason not to have lvm_scan do the udevadm_settle ?
> it seems like all callers are probably interested in having that happen.
>
> maybe we even have race conditions elsewhere as I don't see anywhere else
> that does use settle after calling this.

In blockmeta, creating a volgroup does not create any device nodes, but creating the
logical volumes will. We don't currently do a devsync() (which does a settle using
the --exit-if-exists=/path/to/device/expected to have a fast exit path if the
device node has been created) at the end of lvm logical volume creation, but
the subsequent call of the user of the lvm device uses get_path_to_storage() which
will do a devsync().

I don't think we've any race conditions due calls to scan in block-meta, the devsync
ensures we don't.

And generally adding the settle to the scan can't use the more specific --exist-if-exists
parameter since in clear-holders, we don't know what to expect.

So I generally think that leaving the need for settle up to the caller outside of the
scan itself.

> can we make this '--activate=ay' instead of 2 parms?
> Its much more clear that way. I thought you'd typod'
> your commit message with a rogue 'ay' in it.

Yes

> comments needed in dirty-disk yaml for shutting down lvm/raid

Yes, will add comments.

« Back to merge proposal