[buildd-tools-devel] Filesystem union v3
Roger Leigh
rleigh at codelibre.net
Tue Jun 30 23:15:56 UTC 2009
On Tue, Jun 30, 2009 at 08:05:38PM +0200, Jan-Marek Glogowski wrote:
>
> This version incorporates most changes from
>
> http://git.debian.org/?p=users/rleigh/schroot.git;a=shortlog;h=refs/heads/fs-union
>
> It's based on my v2 patch-set. The patches from 02 to 08 just got a little
> cleanup based on the comments from Roger Leigh at the beginning of this month.
Thanks for the new patches, but I'm afraid I just finished my work of
merging most of the changes in your v2 patch set with my changes in the
fs-union branch. As a result, I'm afraid they can't be applied as-is,
since the bulk of the changes are now present on the master branch. I
literally *just* finished three days of merging work when your patches
arrived!
> > - fixed-length buffers
> This is minimized to one occation now to read the pipe to get the filename.
Is this the pipe usage in sbuild-session?
What is the pipe used for in sbuild-session; it's not clear to me what
the purpose is.
> > - use of dynamic cast in sbuild-session for chroot_plain. String
> > comparisons are slower than dynamic cast and are *not* type-safe.
> As chroot_directory is based on chroot_plain, dynamic cast won't work, as we
> only want to skip the plain but not the directoty chroot. I'm now using
> get_run_setup_scripts() instead of the string comparison.
Could you check the current master and see if this is still an issue,
especially WRT the comments below:
One problem I spotted with the code is that it breaks the chroot
inheritance hierarchy, I think due to a misunderstanding of how
source chroots and union chroots fit in to the overall structure.
I fixed this up on what is now on the master branch, but I hope
this may clear things up:
* All chroot types inherit from sbuild::chroot. Chroot types are
plain, directory, loopback, file, block_device and lvm_snapshot.
* chroot_mountable, chroot_source and chroot_union are *not*
chroot types, they are *interface classes* that give the above
chroot types particular extra features. They do inherit from
sbuild::chroot, but this is only so that they can call some
methods (I'll be removing this ability soon by passing in a
chroot reference to their methods). This is the reason why
all sbuild::chroot inheritance is "virtual public".
* In your patches, chroot_union was *replacing* sbuild::chroot,
inheritance. If you look at my fixed version, you'll see that
- the classes inherit from sbuild::chroot (or other classs in
the chroot type hierarchy) and then use multiple inheritance
to additionally inherit any of the extra interfaces they
support
- in class methods, we "chain up" to the chroot type methods
and then any additional interface methods as appropriate.
* I've fixed up the session flag and environment usage.
* In order to support unions on block-devices, this necessitated
splitting block-device into a base class from which both
block-device and lvm-snapshot could inherit, so lvm-snapshot
doesn't get affected at all by union stuff.
* Union filesystem support is configurable with SBUILD_FEATURE_UNION.
So, what needs doing now:
* I missed out the chroot_loopback testcase you added, so this
still needs adding.
* I missed out UUID support and the setup script changes for that.
I'm not sure what the purpose of this is, and why we need it.
* I also missed the run_parts changes; these can also be added.
* The patches you just posted very likely contain bits I didn't
merge due to either just missing them by accident or
deliberately choosing not to merge them (such as the UUID
stuff). It would be great if you could go through and see
if we are missing anything that still needs adding.
* Testing. Lots and lots of testing. I've done all the merging
work, but not yet seen what's broken. We need to go through
and make sure all the different chroot types still function
correctly and fix and add testcases for all the breakage.
Previously, I was seeing breakage with --begin-session where
--run-session would break due to some of the session data
being missing; I'll need to check that again.
* Documentation. I think that sbuild.conf.5.in is ok, but
sbuild-setup.5.in needs the UNION_* options documenting.
Many thanks for all your work so far; it looks like it's now
coming together nicely. Once the final bits are tidied up it
should be great.
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/buildd-tools-devel/attachments/20090701/dab97476/attachment.pgp>
More information about the Buildd-tools-devel
mailing list