On 05/01/2020 19:54, Steve Langasek wrote:
On Sun, Jan 05, 2020 at 09:16:34AM +0000, Roger Leigh wrote:sbuild::chroot::ptr chroot_zfs_snapshot::clone_source () const { ptr clone(new chroot_zfs_snapshot(*this)); ... should be chroot_zfs_snapshot::clone_source () const { ptr clone(new chroot_directory(*this)); chroot_facet_source_clonable::const_ptr psrc (get_facet<chroot_facet_source_clonable>()); assert(psrc); psrc->clone_source_setup(*this, clone); return clone; }I did look into this as an option, but got tangled up because the source of a zfs clone is not a directory, it's a dataset - which means it is not an absolute filesystem path and is not stat()able. Rather than hacking the directory implementation to relax these constraints and make it compatible with zfs, I decided to keep the zfs implementation self-contained.
Sure, that's completely understandable, but you could set the path to be the mountpoint of the dataset and have it work correctly this way.
The problem that I see with the current approach is that the source chroot session is using a cloned copy of the dataset, just like a regular session, and when you end that session, you'll lose the changes. They won't be preserved in the original dataset. I might be misreading things, but that's what it looks like would happen. That looks fundamentally incompatible with how source chroot sessions are supposed to work. It must work on the source dataset or else it's not going to change it as intended.
If the 05zfs script special-cased source chroots, then you could keep the existing approach on the C++ side, but skip the snapshot/clone in the script and work directly with the original dataset. But I don't currently see any logic to handle this; it's snapshotting and cloning unconditionally.
I see two possible approaches:1) Keep the current code, but unset the clone/snapshot settings and use 05zfs to special-case the snapshot/clone creation and deletion for chroot sessions only, and skip entirely for source sessions. You could do this if the source dataset and clone dataset names are the same. Set the variables so that it will use the mountpoint for the source dataset.
2) Use a directory chroot. Obtain the mountpoint path of the source dataset from zfs so it can be stat()ed
(1) looks simplest to do; it's just a single conditional in 05zfs to skip the snapshot/clone stuff.
Regards, Roger