zpool: add option to mount zpool not at / #211
Conversation
| @@ -55,7 +66,7 @@ | |||
| default = _: '' | |||
| readarray -t zfs_devices < <(cat "$disko_devices_dir"/zfs_${config.name}) | |||
| zpool create ${config.name} \ | |||
| ${config.mode} \ | |||
| -R ${config.mountRoot} ${config.mode} \ | |||
There was a problem hiding this comment.
man zpool-create suggests that implicitly sets cachefile=none and altroot properties on the pool, even if no alt-root is used. Do we want this or should we use lib.optionalString here?
There was a problem hiding this comment.
I just set / to keep backwards compatibility but it doesn't make much sense as it will most likely overwrite the system that the command is run from.
We don't need to worry about altroot and since at this stage the system is not fully configured yet, the missing cachefile should not make a big difference.
There was a problem hiding this comment.
Ah yes, did a bit more docs reading, cachefile=none seems harmless.
I'd still prefer to only set -R if actually used, but don't care too much :)
| @@ -70,7 +81,8 @@ | |||
| in | |||
| { | |||
| dev = '' | |||
| zpool list '${config.name}' >/dev/null 2>/dev/null || zpool import '${config.name}' | |||
| zpool list '${config.name}' >/dev/null 2>/dev/null || \ | |||
| zpool import -R ${config.mountRoot} '${config.name}' | |||
There was a problem hiding this comment.
Given that you set the altroot here, during import, is it even necessary to do so above, during creation, as well? The remark about cachefile=none still applies here.
There was a problem hiding this comment.
Yes, because zpool create mounts the pool already.
There was a problem hiding this comment.
Maybe I am missing something, but why do we need to set altroot during creation? Shouldn't the import suffice to mount it in a live system? I'll try to do a practice test tomorrow
There was a problem hiding this comment.
It's important to have -R on create too, otherwise any datasets that have option.mountpoint set will be mounted on top of the live system when they are created. Not setting -R shadows existing mountpoints like /, which would be fatal.
|
LGTM. I'm already relying on this fork to make disko work from a live environment. This is a very important change to support the non-legacy mountpoints. |
|
Shouldn't this use disko.rootMountPoint? #238 |
|
disko.rootMountPoint is the location where disko mounts the whole device tree with the mountScript (usually /mnt), this option tells where to mount the zpool to, which can be any folder, this gets appended to the rootMountPoint only when running the mountScript |
|
This setting is basically duplicated with disko.rootMountPoint because I didn't know about it when I implemented this. |
Otherwise creating and mounting the zpool in a live image removes the root disk of the live image.
Tested on physical hardware.