On 05/01/2020 02:23, Steve Langasek wrote:
Hi Roger,
On Sat, Jan 04, 2020 at 12:54:56PM +0000, Roger Leigh wrote:
When you clone a source snapshot, we want to make that snapshot the
default
state for the original dataset when you end the session. The "file" chroot
type is the most comparable here; the LVM and Btrfs snapshots operate
directly on the original copy. Right now the ZFS source chroot clone looks
as ephemeral as a regular cloned session, so I'm not sure the current
behaviour is corect. We want the saving of the updated source chroot to be
done such that it appears atomic for any concurrent usage to clone the old
state or the new state, and never an intermediate state as the original is
updated. It's this part that I wasn't sure about. I don't see an
equivalent to "zfs promote" which would allow copying back a clone (or
snapshot of a clone) to the source dataset, and I don't see any
source-chroot-specific logic in 05zfs which would preserve any source chroot
changes.
Ok, you're right that I haven't done anything wrt 'zfs promote'. Prior to
switching to zfs, all of my schroot work used the lvm-snapshot type, which
has the same limitation you describe here: session clones taken while the
source chroot is in the process of being updated end up cloning some
intermediate state. I have never had a problem with this limitation in
practice, since my usage of schroot is entirely human-driven, so it's very
easy for me to avoid launching new schroots while in the middle of a
"maintenance window".
Being able to update the source atomically certainly seems like a nice
enhancement, but given that the implementation is currently at parity with
the lvm-snapshot type, I am not likely to invest effort in this myself at
this time.
In this case, I think you should copy the btrfs-snapshot approach and
make the source chroot a "directory" chroot type. Then it will operate
directly on the source dataset. That is to say:
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;
}
This isn't quite as elegant as doing an atomic update, but if it's
primarily under the user's control rather than being scripted and done
automatically then it should be good enough, and it matches the
constraints the other chroot snapshot types have operated under.
I would certainly be happy to see a more sophisticated approach in the
future which provides some form of atomic update and guarantees cloning
new sessions will always use a known good state, but this will clearly
need more work to do properly.
Kind regards,
Roger
