Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upPodman failed to destroy BTRFS snapshot on container delete #3963
Comments
|
Sounds like c/storage cleanup - @nalind Agree? |
|
Actually, wait - how are we using the btrfs driver with rootless? @giuseppe PTAL |
|
It's a bit confusing at the moment. For historical reasons, However, recently, rmdir(2) will delete a btrfs subvolume, without privilege escalation required, so long as the user otherwise has privilege for the subvolume and contents. I haven't benchmarked a fully populated set of subvolumes to compare |
|
Ok, I don't truly understand what is going on here. But are we doing something wrong in the storage driver that we can fix to make rootless podman with btrfs work? Or should we simply block rootless podman with BTRFS because it will never work correctly and force users to use fuse-overlay? |
|
If kernel 4.18+ use Optimization 1: If root, use
For kernels 4.17 and older, then you could check for |
|
We've never actually tested the btrfs driver on rootless before, and it was never written with rootless support in mind. However, it seems like everything except cleanup is already working. Given that we seem to have a way forward there (at least on newer kernels), this sounds like a reasonable fix. |
|
FWIW, I'm not sure what use case would exist where a user owns the subvolume but doesn't own the contents, but... |
|
I've never tried the btrfs driver and I am surprised it works with rootless. @cmurf would it be enough if we attempt to Would you like to work on a patch for that? |
|
@giuseppe actually it's a good idea to try the optimized case first, since it's way faster, and then use |
|
For users experiencing this issue, what storage driver should those running Podman rootless use before BTRFS is supported? |
|
We'd recommend fuse-overlayfs |
|
I'm good with closing this is as "unsupported" if you'd like to track this as a pending feature elsewhere, but I'll leave that up to the dev team. Thanks for your quick responses. |
|
Apologies if I'm stating the obvious, but as Podman is creating the BTRFS subvolume it could easily tag the subvolume as USER_SUBVOL_RM_ALLOWED on creation, right? I'm not a Go programmer but I an read it, and it appears that support for this option could be added consistently with the other BTRFS options used on subvolume creation in func parseoptions in c/storage/drivers/btrfs/btrfs.go. This would allow the non-root user to delete BTRFS subbvolumes on supported kernels. This should cover most cases, and a verbose error message may cover the edge cases with a simple addition to code. |
|
I think that's definitely possible. The biggest issue right now is probably finding someone to work on it - most people are on fuse-overlayfs, so fixing the btrfs driver is lower priority on the storage side. |
|
With this mount option set, the user must have privilege for just the subvolume they want to delete, contents aren't checked for privileges. That lack of checking content ownership is why |
|
@cmurf Yep, makes total sense now... I just found all this out for troubleshooting. I can confirm Podman works as expect with BTRFS as a storage driver when running in rootless mode. For troubleshooters, add the The @mheon I was wrong here and had a different configuration issue. |
|
I think the second part needs an issue of its own. I know btrfs and overlayfs can co-exist in production environments with tens of thousands of snapshots. I'm not sure what the nature of this lack of support could be about. There are nuances that can be workload specific where one works better than the other, and even where overlayfs copy-up operation can be made more efficient using cloning (Btrfs has had reflinks since forever, and XFS enables them in the most recent xfsprogs at mkfs time). |
|
Hmm, on Fedora 31, by default it appears podman is using fuse-overlayfs on Btrfs. |
|
@SwitchedToGitlab We would welcome a PR on container/storage to better support BTRFS for rootless containers. |
|
Or at least add some of this information on how to setup btrfs for use in rootless containers in some of the troublsheooting rootless.md files on github. |
|
Hello. I noticed this issue on btrfs ML and if nobody is working on, I'm willing to help (though I'm not familiar with podman and may take some time). |
|
That would be great, Changes might be required in github.com/containers/storage though since most of the BTRFS code is in there. |
|
Thanks, let me try. |
|
Honestly using overlay is greatly preferred. Not sure if this is something that really needs a fix in code. It works great with BTRFS once you enable That said, @t-msn I think we run into trouble at or about line 308 in containers/storage. The elegant solution IMHO would be to trap for errors on this |
Right. The needed fix is when that mount option is not set, the subvolume remove fails with an error, in which case the fallback should be to 'rm -rf' the subvolume. It's slower than subvolume delete, but at least it won't fail, unless the user doesn't actually own what they're deleting. |
|
Are either of you guys at All systems go this weekend? |
well, I think the fix is trivial and won't hurt anyone (please see below).
We cannot call IOC_SNAP_DESTROY ioctl if it contains other subolumes. This is the reason subovlDelte() performs path walk to remove subvolume bottom-up, but it is not necessary for "rm -r" I check the code and notice that system.EnsureRemoveAll() is called after subvolDelete() and it uses os.RemoveAll() to ensure the target path is removed. Therefore the simplest solution would be just ignoring the error of subvolDelete() and fallback to system.EnsureRemoveAll(): t-msn/storage@41c2a90 I followed the tutorial (https://github.com/containers/libpod/blob/master/docs/tutorials/rootless_tutorial.md and https://github.com/containers/libpod/blob/master/docs/tutorials/podman_tutorial.md) and with above fix I can do "podman rm". (BTW, subvol quota operation needs privilege.) |
|
This issue had no activity for 30 days. In the absence of activity or the "do-not-close" label, the issue will be automatically closed within 7 days. |
|
@t-msn - Would you be willing to submit your change (t-msn/storage@41c2a90) as a Pull Request? |
Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)
bug
Description
ROOTLESS Podman fails to delete BTRFS subvolumes when building an image or deleting a container. This causes a cascade of errors, such as container name re-use errors, as podman believes the container was removed when using
podman ps -ahowever when attempting to re-run thepodman runcommand the user will receive an name re-use error message.Using SUDO this works just fine.
I would very much assume that this is a configuration issue on my part somewhere, as without privilege elevation using sudo I cannot delete the specified BTRFS by hand using
btrfs su delete <path to subvolume>either.Thanks in advance for your help.
Steps to reproduce the issue:
Build a rootless image using the BTRFS driver.
Get error message listed below.
Describe the results you received:
Describe the results you expected:
BTRFS subvolumes to be deleted on container deletion.
Additional information you deem important (e.g. issue happens only occasionally):
Consistent regardless of the image.
Output of
podman version:Output of
podman info --debug:Package info (e.g. output of
rpm -q podmanorapt list podman):Additional environment details (AWS, VirtualBox, physical, etc.):
Bare metal install on Intel i7 and spinning rust HDD.
Pastes of Storage.conf and libpod.conf