Review: Approve
I think this is fine and probably helps. I think it's a bit overbroad probably
but maybe that's not a real problem considering how much overbroad stuff there
is in curtin currently...
Diff comments:
> diff --git a/curtin/block/zfs.py b/curtin/block/zfs.py
> index ccdddbb..81ba1e1 100644
> --- a/curtin/block/zfs.py
> +++ b/curtin/block/zfs.py
> @@ -89,6 +90,7 @@ class ZPoolEncryption:
> zfs_create(
> self.poolname, "keystore", {"encryption": "off"}, keystore_size,
> )
> + udevadm_settle()
I don't know for sure (obviously with racy things like this) but I suspect this
might actually be the real fix here: afaict the
/dev/zvol/{self.poolname}/keystore node is created by udev. Would it be cleaner
if zfs_create called udevadm_settle(exists=$path) before returning? (It might
also be nice if it returned the path too?)
>
> with ExitStack() as es:
> for vdev in self.vdevs:
> @@ -97,13 +99,19 @@ class ZPoolEncryption:
> # cryptsetup format and open this keystore
> keystore_volume = f"/dev/zvol/{self.poolname}/keystore"
> cmd = ["cryptsetup", "luksFormat", keystore_volume, self.keyfile]
> - util.subp(cmd)
> +
> + # strace has shown that udevd does indeed probe these keystores
FWIW the issue we see with https://systemd.io/BLOCK_DEVICE_LOCKING/ type stuff
AIUI is that udev accessing the _parent_ block device causes the _partition_ or
child nodes to disappear briefly. I don't think that's what's happening here?
If it is you would need to be locking the parent block device but I don't think
there is one for zfs?
That said I doubt any of this hurts!
> + with util.FlockEx(keystore_volume):
> + util.subp(cmd, capture=True)
> +
> + udevadm_settle()
> +
> dm_name = f"keystore-{self.poolname}"
> cmd = [
> "cryptsetup", "open", "--type", "luks", keystore_volume,
> dm_name, "--key-file", self.keyfile,
> ]
> - util.subp(cmd)
> + util.subp(cmd, capture=True)
Should this udevadm_settle(exists="f"/dev/mapper/{dm_name}") too? Although
device mapper stuff integrates somewhat with udev so maybe not...
>
> with ExitStack() as es:
> # format as ext4, mount it, move the previously-generated
> systemkey
--
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/464246
Your team curtin developers is subscribed to branch curtin:master.
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
