containerd/btrfs/v2 (CGO-less implementation from dennwc/btrfs)#34
containerd/btrfs/v2 (CGO-less implementation from dennwc/btrfs)#34AkihiroSuda wants to merge 38 commits intocontainerd:mainfrom
Conversation
AkihiroSuda
force-pushed
the
import-dennwc-btrfs
branch
4 times, most recently
from
2a8b3cf to
89855b5
Compare
| - name: Project checks | ||
| uses: containerd/project-checks@v1 | ||
| # Disabled during repo migration | ||
| if: false |
There was a problem hiding this comment.
This TODO can be worked out after merging this PR
|
|
||
| To apply the Apache License to your work, attach the following | ||
| boilerplate notice, with the fields enclosed by brackets "{}" | ||
| boilerplate notice, with the fields enclosed by brackets "[]" |
In the next batch of commits, the content of the repo will be replaced with the content of github.com/dennwc/btrfs Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda
force-pushed
the
import-dennwc-btrfs
branch
2 times, most recently
from
1376856 to
c0def28
Compare
(cherry picked from commit dennwc/btrfs@2d0abe4) Signed-off-by: Akihiro Suda <[email protected]>
…receive. * better code generator * regenerate btrfs_tree.h constants * implement subvolume create, delete and snapshot create commands * exec-based snapshot send and receive (cherry picked from commit dennwc/btrfs@b300237) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@f03fa74) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@6a4b966) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@4363265) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@41809ca) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@7eae92f) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@8f48d3e) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@14de038) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@b56d643) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@3343324) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@6f59d60) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@94acb6e) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@4ac498b) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@08f5a6b) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@54573d1) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@3b69894) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@51976ca) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@00defaf) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@e24c76d) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@a46fd03) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@d917b30) Signed-off-by: Akihiro Suda <[email protected]>
Fix containerd#1 Signed-off-by: Akihiro Suda <[email protected]> (cherry picked from commit dennwc/btrfs@ee67271) Signed-off-by: Akihiro Suda <[email protected]>
(cherry picked from commit dennwc/btrfs@90a0320) Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda
force-pushed
the
import-dennwc-btrfs
branch
2 times, most recently
from
3a0330c to
031f9b7
Compare
Temporarily delete cmd/gbtrfs/go.mod (added back in a separate commit) Signed-off-by: Akihiro Suda <[email protected]>
Reran `go generate` with btrfs_tree.h from kernel 5.13 https://github.com/torvalds/linux/blob/v5.13/include/uapi/linux/btrfs_tree.h btrfs_tree.h is licensed under the terms of "GPL-2.0 WITH Linux-syscall-note": https://github.com/torvalds/linux/blob/v5.13/LICENSES/exceptions/Linux-syscall-note containerd/btrfs (dennwc/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. Signed-off-by: Akihiro Suda <[email protected]>
Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda
force-pushed
the
import-dennwc-btrfs
branch
2 times, most recently
from
92aed35 to
e9c9e41
Compare
We would like to invite dennwc as a maintainer of this repo, so the status of the repo is now changed from core to non-core. Signed-off-by: Akihiro Suda <[email protected]>
Import dennwc/ioctl@0178042 Signed-off-by: Akihiro Suda <[email protected]>
Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda
force-pushed
the
import-dennwc-btrfs
branch
from
e9c9e41 to
412d0f4
Compare
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
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.xbranch (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 onv1.0.0)
|
|
||
| type blockGroup uint64 | ||
|
|
||
| // SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note (see headers.go) |
There was a problem hiding this comment.
Would love to have someone check/verify if this is sufficient (see my other comment)
There was a problem hiding this comment.
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.
| Copyright The containerd Authors. | ||
|
|
There was a problem hiding this comment.
Should the headers in this project be updated to preserve the old copyright? Or do we consider "The containerd authors" to cover this?
| Copyright The containerd Authors. | |
| Copyright The containerd Authors. | |
| Copyright 2016-2021 Denys Smirnov. |
There was a problem hiding this comment.
That will not pass the CI 😅
There was a problem hiding this comment.
Oh 🤦 we use a single set of templates for all project 😞
Same may apply to v1, as v1 imports consts from GPL2 headers as well kdave/btrfs-progs#380 |
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. |
|
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.) 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. |
|
What are the API changes package users need to do to use v2? We should probably document them. |
|
Current status: waiting for cncf/foundation#174 to be approved ( |
|
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. |
7 participants
🟡 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
v1branch.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.gois machine-generated frombtrfs_tree.hwhich is licensed under the terms ofGPL-2.0 WITH Linux-syscall-note.containerd/btrfsshall be considered asuser programs that use kernel services by normal system callsmentioned in the note above, anddoes *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