Skip to content

Add /etc/xattr.conf in the sandbox to workaround cross-FS copy issue #3754

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add /etc/xattr.conf in the sandbox to workaround cross-FS copy issue #3754

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6189dfa
Add /etc/xattr.conf in the sandbox to workaround cross-FS copy issue
gdonval May 30, 2025
a206b3b
Reduce padding to pass linting
gdonval May 30, 2025
4fbeea1
Linting
gdonval May 30, 2025
5166894
Linting
gdonval May 30, 2025
30a481d
Add full URLs to bug reports
gdonval Jun 2, 2025
a1f5d08
Move local option components inline
gdonval Jun 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion mkosi/tree.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,34 @@ def copy() -> None:
if src.is_dir():
cmdline += ["--no-target-directory"]

run(cmdline, sandbox=sandbox(options=options))
# A valid /etc/xattr.conf is necessary to work around cross-filesystem copy failures:
# https://github.com/systemd/mkosi/issues/3732.
# A bug is opened with coreutils:
# https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78623
# When the bug closes, this can collapse to the following with appropriate adjustments:
#
# run(cmdline, sandbox=sandbox(options=options))
#
with tempfile.TemporaryDirectory() as tmp:
import textwrap

p = Path(tmp) / "xattr.conf"
p.write_text(
textwrap.dedent(
"""
* skip # Default: ignore all xattr but
system.* permissions # allow well-known 'system'
trusted.* permissions # allow well-known 'trusted'
user.* permissions # allow well-known 'user'
security.* permissions # allow well-known 'security'
# but
trusted.SGI_* skip # skip XFS-specific namespace (not portable)
user.Beagle.* skip # skip Beagle index data (not portable)
security.evm skip # skip evm (may only be written by kernel)
"""
)
)
Comment on lines +163 to +177
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this a bit smarter and also copy the btrfs attrs if we detect the destination filesystem is also btrfs?

Copy link
Author

@gdonval gdonval Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only btrfs xattr I know about is btrfs.compression, which is definitely something I think should be set by systemd-repart (Compression=...), not copied over from whatever host configuration is.

Normal file attributes (chattr) are not affected by this barred one: btrfs that calculates the c-flag on the fly depending on btrfs.compression xattr, which we've established is not something we should be copying over anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we established that? I don't remember agreeing to that. If a file is compressed on one btrfs filesystem, why shouldn't it be compressed after copying?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a file is compressed on one btrfs filesystem, why shouldn't it be compressed after copying?

Because the one who "chose" to compress it was myself from 7 years ago, not the package manager.

On my system, I, myself, chose to btrfs set property set / compression "zstd", 7 years ago when I installed my local system. And now, this is inherited by mkosi's workspace folder (as xattr btrfs.compression = zstd).

Do we want to persist local arbitrary choices or do we want that choice to be explicit made in repart.d files?

If you want btrfs.compression to be copied, you need to make sure you start with a clean slate (i.e. clean file system) so that you can ensure this was an explicit packaging choice.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make it smarter, but if we do, we might as well make a few checks on a single canary file before copying the tree and use cp -a, which will opportunistically keep as much metadata as possible.

Copy link
Author

@gdonval gdonval Jun 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the problem... systemd-repart does not set btrfs.compression. Compression=zstd seems to actually do nothing with btrfs: it's supposed to use a mkfs.btrfs flag to force compression whilst copying files but compsize reports no compression was ever attempted. That definitely is suboptimal. Proof:

image

Debug shows the same:

image

root-x86-64 is as generated by systemd-repart with Compression=zstd whereas new-btrfs-root was generated with sudo mkfs.btrfs --compress=zstd -L "new-btrfs-root" -r ".../root-x86-64/" btrfs.img.

Not a bug in mkosi, just a constraint added to a very hairy problem...

run(cmdline, sandbox=sandbox(options=options + ["--ro-bind", p, "/etc/xattr.conf"]))

# Subvolumes always have inode 256 so we can use that to check if a directory is a subvolume.
if (
Expand Down
Loading