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