containerd/btrfs/v2 (CGO-less implementation from dennwc/btrfs)

[AkihiroSuda]

🟡 waiting for cncf/foundation#174 to be approved (License exception for GPL-2.0 WITH Linux-syscall-note file in containerd/btrfs)

---

Migrate dennwc/btrfs@90a0320a to this repo as containerd/btrfs/v2.
The existing implementation can be still maintained in v1 branch.

Thanks to @dennwc for donating the code (containerd/containerd#5658 (comment))

License

The repo is licensed under the terms of Apache License 2.0.

It should be noted that btrfs_tree_hc.go is machine-generated from btrfs_tree.h which is licensed under the terms of GPL-2.0 WITH Linux-syscall-note.

   NOTE! This copyright does *not* cover user programs that use kernel
 services by normal system calls - this is merely considered normal use
 of the kernel, and does *not* fall under the heading of "derived work".
 Also note that the GPL below is copyrighted by the Free Software
 Foundation, but the instance of code that it refers to (the Linux
 kernel) is copyrighted by me and others who actually wrote it.

 Also note that the only valid version of the GPL as far as the kernel
 is concerned is _this_ particular version of the license (ie v2, not
 v2.2 or v3.x or whatever), unless explicitly otherwise stated.

			Linus Torvalds

containerd/btrfs shall be considered as user programs that use kernel services by normal system calls mentioned in the note above, and does *not* fall under the heading of "derived work" of the GPL-2.0 code. (IANAL)

Change on project status

The status of this repo is going to be changed from "core" to "non-core", to invite @dennwc as a non-core committer.

---

Close #7

[AkihiroSuda]

This TODO can be worked out after merging this PR

[AkihiroSuda]

lol

[dmcgowan]

LGTM

[thaJeztah]

Don't have enough knowledge about the BTRFs things, so did not review the implementation.

containerd/btrfs shall be considered as user programs that use kernel services by normal system calls mentioned in the note above, and does not fall under the heading of "derived work" of the GPL-2.0 code. (IANAL)

Does someone know if we can get assistance / review on this from the CNCF? e.g. should this repository have a NOTICE file, which also mentions this?

(Thinking out loud); to deal with go modules;

• looks like no relevant changes were merged after v1.0.0 (v1.0.0...main)
• for CI, we probably should create that release/1.x branch (from the v1.0.0 tag or from "main"), and update GitHub actions to also run on PRs and merged on that branch
• after merging this PR, perhaps tag main as v2.0.0-alpha.0 (e.g.) so that go modules will generate sensible pseudo versions (otherwise it will create pseudo-versions based on v1.0.0)

[thaJeztah]

Would love to have someone check/verify if this is sufficient (see my other comment)

[estesp]

Yes, this is the correct designation, and along the lines with what @AkihiroSuda said, this is just more explicit now that the "header" is in our codebase. We were always linking against GPLv2 code (libbtrfs and the requisite headers) and were already depending on this exception, which is stated as:

This exception is used together with one of the above SPDX-Licenses
  to mark user space API (uapi) header files so they can be included
  into non GPL compliant user space application code.

[thaJeztah]

Should the headers in this project be updated to preserve the old copyright? Or do we consider "The containerd authors" to cover this?

Suggested change

	Copyright The containerd Authors.
	Copyright The containerd Authors.
	Copyright 2016-2021 Denys Smirnov.

[AkihiroSuda]

That will not pass the CI 😅

[thaJeztah]

Oh 🤦 we use a single set of templates for all project 😞

[AkihiroSuda]

Does someone know if we can get assistance / review on this from the CNCF? e.g. should this repository have a NOTICE file, which also mentions this?

Same may apply to v1, as v1 imports consts from GPL2 headers as well kdave/btrfs-progs#380

[estesp]

LGTM

[Conan-Kudo]

The status of this repo is going to be changed from "core" to "non-core", to invite @dennwc as a non-core committer.

This is a weird thing to do. My reading of switching to "non-core" also implies that containerd would no longer gate or officially support this, and that containerd tarballs would no longer contain this code. That would be very problematic to me.

To me, this sounds like the governance is not flexible enough. It doesn't make sense to transition this to "non-core" status if there's at least one "core" member involved in here.

[AkihiroSuda]

The btrfs plugin for containerd is maintained in the core repo, and will remain in the core repo, and will remain as a built-in official core plugin for the foreseeable future (until containerd v2.0 at least, and containerd v2.x will probably keep btrfs built-in.)

https://github.com/containerd/containerd/blob/main/snapshots/btrfs/btrfs.go

The underlying low-level btrfs library in this repo is going to be a non-core, but we core committers will continue to maintain this repo too.

[stevvooe]

What are the API changes package users need to do to use v2? We should probably document them.

[AkihiroSuda]

Current status: waiting for cncf/foundation#174 to be approved (License exception for GPL-2.0 WITH Linux-syscall-note file in containerd/btrfs)

[stevvooe]

Not LGTM.

We had a conversation on this matter in slack and I thought we were in agreement that we wouldn't break the interface. All that needs to be done to remove cgo is replace the struct unpacking in https://github.com/containerd/btrfs/pull/34/files#diff-37cee27ecb051878390f4745b46e50e69389589a620a6714441f7d14dd058179L24.

While I appreciate the contributions from @dennwc, I think we can do a more careful merge to get the same result without breaking the interface. We can bring in a lot of the definitions from https://github.com/dennwc/btrfs/blob/90a0320aae50c617db7a9c9d90072c37d1a81907/ioctl_h.go#L131. For root_item, where I was struggling with layout the most, the implementation is in https://github.com/dennwc/btrfs/blob/90a0320aae50c617db7a9c9d90072c37d1a81907/btrfs_tree.go#L241.