Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

daemon/graphdriver/btrfs/btrfs.go args.lim.max_referenced undefined #44698

Closed
s4uliu5 opened this issue Dec 27, 2022 · 7 comments · Fixed by #44707
Closed

daemon/graphdriver/btrfs/btrfs.go args.lim.max_referenced undefined #44698

s4uliu5 opened this issue Dec 27, 2022 · 7 comments · Fixed by #44707
Labels
area/kernel area/storage/btrfs kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.

Comments

@s4uliu5
Copy link

s4uliu5 commented Dec 27, 2022

Description

While building docker-20.10.22 on Funtoo linux with btrfs-progs 6.1, got this error

daemon/graphdriver/btrfs/btrfs.go:437:11: args.lim.max_referenced undefined (type _Ctype_struct_btrfs_qgroup_limit has no field or method max_referenced)

Reproduce

  1. extract source
  2. hack/make.sh dynbinary

Expected behavior

Build not fail.

docker version

Not relevant.

docker info

Not relevant.

Additional Info

Only fails with btrfs-progs 6.1, with previous versions (6.0.x) builds ok.

IMHO the change of btrfs-progs that caused this affect is

kdave/btrfs-progs@0345143

@s4uliu5 s4uliu5 added kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed. status/0-triage labels Dec 27, 2022
@thaJeztah
Copy link
Member

Thanks for reporting! Hm... from that commit (kdave/btrfs-progs@0345143);

ioctl.h is a public header but there are no users of the btrfs_qgroup_limit structure.

Looks like they missed that it was used in this repository (at least; it's possible other runtimes may be using it as well). I see use of it was added for quota support in 401c8d1 (#19651)

That will be a tricky one; I don't think we can easily change the code to match the new version (as we need to build for various distros, most of which will still have the older btrfs-progs).

@kdave @josefbacik any suggestions how to keep it backward compatible?

zhsj added a commit to zhsj/moby that referenced this issue Dec 28, 2022
Closes: moby#44698

Signed-off-by: Shengjing Zhu <zhsj@debian.org>
zhsj added a commit to zhsj/moby that referenced this issue Dec 29, 2022
Closes: moby#44698

Signed-off-by: Shengjing Zhu <zhsj@debian.org>
(cherry picked from commit ffbbe3d)
Signed-off-by: Shengjing Zhu <zhsj@debian.org>
zhsj added a commit to zhsj/moby that referenced this issue Dec 29, 2022
Closes: moby#44698

Signed-off-by: Shengjing Zhu <zhsj@debian.org>
(cherry picked from commit ffbbe3d)
Signed-off-by: Shengjing Zhu <zhsj@debian.org>
@Conan-Kudo
Copy link

@thaJeztah This change will be reverted in the next btrfs-progs bugfix release (v6.1.1): https://lore.kernel.org/linux-btrfs/20230103113941.GN11562@twin.jikos.cz/

@Conan-Kudo
Copy link

FYI, the (correct) longer-term fix is to move to libbtrfsutil instead of using libbtrfs: https://lore.kernel.org/linux-btrfs/20230103120640.GO11562@twin.jikos.cz/

@thaJeztah
Copy link
Member

Thanks for the heads-up @Conan-Kudo - appreciated! ❤️

We had some discussion about what headers would be best to use, and why we were not using the kernel headers (that last part we tracked down to support for RHEL7, which had experimental support for BTRFS, but (I think) not in the kernel headers of the kernel version used - @corhere or @neersighted did some digging into options).

@neersighted
Copy link
Member

neersighted commented Jan 3, 2023

Indeed, the plan is to depend on the kernel uAPI here, which exposes the fields we currently use. That being said, libbtrfsutil is good to keep in mind if we need to depend on functionality not in the uAPI in the future (e.g. userspace code).

@Conan-Kudo
Copy link

using the kernel uAPI directly tends to be problematic since the btrfs-progs keeps a copy of the latest interfaces with backwards compatibility for older kernels, whereas the uAPI headers in the kernel lack this.

@neersighted
Copy link
Member

That's true -- however the only kernel without the fields we currently need is EL 7's 3.10-based kernel. In the case of a EL 7 system, newer kernel headers can be used, or one can simply not compile the Btrfs graphdriver (which should be preferred as all EL 7 vendors shipping a Red Hat-compatible kernel have deprecated support; OL 7 users on the UEK will have the correct uAPI headers available).

If this situation changes and header availability is more of a concern, we'll likely depend on the userspace versions again.

neersighted added a commit to neersighted/moby that referenced this issue Jan 6, 2023
By relying on the kernel UAPI (userspace API), we can drop a dependency
and simplify building Moby, while also ensuring that we are using a
stable/supported source of the C types and defines we need.

btrfs-progs mirrors the kernel headers, but the headers it ships with
are not the canonical source and as [we have seen before][44698], could
be subject to changes.

Depending on the canonical headers from the kernel both is more
idiomatic, and ensures we are protected by the kernel's promise to not
break userspace.

  [44698]: moby#44698

Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
neersighted added a commit to neersighted/moby that referenced this issue Jan 6, 2023
By relying on the kernel UAPI (userspace API), we can drop a dependency
and simplify building Moby, while also ensuring that we are using a
stable/supported source of the C types and defines we need.

btrfs-progs mirrors the kernel headers, but the headers it ships with
are not the canonical source and as [we have seen before][44698], could
be subject to changes.

Depending on the canonical headers from the kernel both is more
idiomatic, and ensures we are protected by the kernel's promise to not
break userspace.

  [44698]: moby#44698

Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
neersighted added a commit to neersighted/moby that referenced this issue Jan 6, 2023
By relying on the kernel UAPI (userspace API), we can drop a dependency
and simplify building Moby, while also ensuring that we are using a
stable/supported source of the C types and defines we need.

btrfs-progs mirrors the kernel headers, but the headers it ships with
are not the canonical source and as [we have seen before][44698], could
be subject to changes.

Depending on the canonical headers from the kernel both is more
idiomatic, and ensures we are protected by the kernel's promise to not
break userspace.

  [44698]: moby#44698

Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
neersighted added a commit to neersighted/moby that referenced this issue Jan 6, 2023
By relying on the kernel UAPI (userspace API), we can drop a dependency
and simplify building Moby, while also ensuring that we are using a
stable/supported source of the C types and defines we need.

btrfs-progs mirrors the kernel headers, but the headers it ships with
are not the canonical source and as [we have seen before][44698], could
be subject to changes.

Depending on the canonical headers from the kernel both is more
idiomatic, and ensures we are protected by the kernel's promise to not
break userspace.

  [44698]: moby#44698

Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
thaJeztah pushed a commit to thaJeztah/docker that referenced this issue Jan 10, 2023
By relying on the kernel UAPI (userspace API), we can drop a dependency
and simplify building Moby, while also ensuring that we are using a
stable/supported source of the C types and defines we need.

btrfs-progs mirrors the kernel headers, but the headers it ships with
are not the canonical source and as [we have seen before][44698], could
be subject to changes.

Depending on the canonical headers from the kernel both is more
idiomatic, and ensures we are protected by the kernel's promise to not
break userspace.

  [44698]: moby#44698

Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
(cherry picked from commit 3208dca)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kernel area/storage/btrfs kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants