On Sun, Jan 05, 2020 at 08:23:33PM +0000, Roger Leigh wrote: > 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. And the user would have to declare both the mountpoint and the dataset name in the schroot.conf? That would be bad UX. Or else the schroot C++ code would have to call into zfs client code to retrieve the mountpoint of the dataset, just to meet the constraints of the chroot type. And there's actually no reason that the dataset needs to HAVE a defined mountpoint. With the current implementation, you'd set mountpoint=legacy on the dataset, and it is mounted by schroot only when needed. > 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. Sorry, that's not the case. Are you looking at the first version of my patch, or the second? The first version was buggy in precisely this way. The second version works correctly, mounting the source dataset, not a clone. > If the 05zfs script special-cased source chroots, It does in the sense that when called for a source chroot, the zfs-snapshot code does not set up the clone/snapshot variables, making 05zfs a no-op. -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developer https://www.debian.org/ slangasek@ubuntu.com vorlon@debian.org
Attachment:
signature.asc
Description: PGP signature