Support sparsefiles Copy in blockfile snapshotter by fidencio · Pull Request #12066 · containerd/containerd

fidencio · 2025-07-07T15:19:31Z

Add the ability to copy sparse files, allowing it to be set in the configuration file.

This is needed as the golang implementatio of io.Copy is equivalent to cp --sparse=nevar ..., meaning that even if the initial scratch file is a sparse file, it's cop will still be a non-sparse one, leading to each snapshot block file occupying the full blockfile set by the user.

k8s-ci-robot · 2025-07-07T15:19:41Z

Hi @fidencio. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

fidencio · 2025-07-07T15:20:56Z

I'm revisiting #10197 on this PR, and addressing the comments made by @dmcgowan.

@AkihiroSuda, you already approved the previous one, would you mind to also taking a look at this one, considering the original contributor has been busy and didn't have time to address @dmcgowan's comments?

fidencio · 2025-07-08T12:27:51Z

I've taken the freedom here to do some changes that I'm not sure if they'll be acceptable by the maintainers (@dmcgowan, I'm looking at you).

The implementation provided by the first author of this patch was already linux specific, as it'd bring in a dependency on the fallocate binary. I dropped such dependency and implemented the sparse copy in a different way.

This new way is not windows friendly, unfortunately, and while I could try harder to get it to work on Windows, I'm not sure whether this is worth it, as AFAICT there's no VMM based runtime for Windows, leading to no-one using it on Windows.

Is this assumption correct, @dmcgowan?

@hsiangkao suggested to use copy_file_range to simplify the implementation, and know that I realised that the code is already non-windows specific, maybe that's a better approach.

I'll wait a little bit till I get some feedback and revisit this one during this evening.

thanks,

fidencio · 2025-07-08T14:45:57Z

I've split the files in a way that we only keep the copySparse for Linux, and warn the users that it has no effect in other OSes.

One question is about the default "block size" to copy, which I'm using 2MB here, but maybe it's an overkill and we should go for something bigger (16MB?).

In any case, feedback is more than welcome.

AkihiroSuda · 2025-07-20T05:18:01Z

Can we have unit tests

We can, but I'd really prefer doing it in a seperate PR, @AkihiroSuda.

The reason for that is this PR just being a revival of #10197, which I was not the author.

If you don't mind, let's get this one merged and I will open a PR adding unit tests to it.

Sounds fair?

mythi

minor typo nits and a couple of questions

mythi · 2025-07-21T06:10:56Z

+
+	// CopySparse forces the creation of a sparse file when copying
+	// the 'ScratchFile'
+	CopySparse bool `toml:"copy_sparse"`


I wonder if the config option is still needed/justified? The original implementation had it but was relying on an external tool to sparsify the target layer and needed user opt-in to do it. This PR is mainly about an optimized copy for Linux which skips holes if the admin had already opted-in to create a sparse ScratchFile.

I think the config option is still justified, as copy_sparse = true leads to oversubscription on disk ... and I can see usecases where the admins are not okay with that.

If that is the case, then the provided scratch file should not be sparse. My understanding is the code is copy sparse only preserves the sparse nature, it does not make a non-sparse file sparse.

Additionally, this is not the right driver to use for that case, we have better options now. I think we should just enable this by default on ext4 and don't need to a config. Is there another filesystem in use where this would make sense?

mythi · 2025-09-19T13:45:25Z

@dmcgowan is this still targeted for 2.2?

I was testing this w/ Kata and rebased to 2.2-beta.0 on Linux XFS today and things worked OK. I could run more tests if needed. Only one observation related to initial scratch creation from scratch_file was that the initial copying had some errors when copying over two different mount points:

error="unable to load CRI image service plugin dependency: failed to generate scratch file: failed to copy scratch: failed to copy file to target /srv2/containerd/io.containerd.snapshotter.v1.blockfile/scratch: invalid cross-device link"

(my scratch_file was on ext4 rootfs as /opt/containerd/blockfile and root_path is on an XFS fs).

ChengyuZhu6 · 2025-09-19T15:02:22Z

@dmcgowan is this still targeted for 2.2?

I was testing this w/ Kata and rebased to 2.2-beta.0 on Linux XFS today and things worked OK. I could run more tests if needed. Only one observation related to initial scratch creation from scratch_file was that the initial copying had some errors when copying over two different mount points:
error="unable to load CRI image service plugin dependency: failed to generate scratch file: failed to copy scratch: failed to copy file to target /srv2/containerd/io.containerd.snapshotter.v1.blockfile/scratch: invalid cross-device link"
(my scratch_file was on ext4 rootfs as /opt/containerd/blockfile and root_path is on an XFS fs).

I think we could implement a check during blockfile snapshotter's initialization to detect the cross-filesystem case. If detected, it would log a warning and fallback to using io.Copy.

mythi · 2025-09-25T07:46:49Z

I think we could implement a check during blockfile snapshotter's initialization to detect the cross-filesystem case. If detected, it would log a warning and fallback to using io.Copy.

Yes that would work but I think documentation would also have to be improved/changed so that users can prepare for that fallback and do the initial configuration properly.

dmcgowan · 2025-10-15T23:33:41Z

@dmcgowan is this still targeted for 2.2?

We can get this in

This is needed as the golang implementatio of io.Copy is equivalent to cp --sparse=nevar ..., meaning that even if the initial scratch file is a sparse file, it's cop will still be a non-sparse one, leading to each snapshot block file occupying the full blockfile set by the user.

My understanding is the golang io.Copy is also equivalent to cp --reflink=auto, in which case a sparse file will remain sparse if the underlying FS support CoW. I think that should be pointed out in the docs that this option should not be used with underlying FS that already support shared extents (btrfs, xfs, apfs). Basing this off https://cs.opensource.google/go/go/+/master:src/os/zero_copy_linux.go

dmcgowan

I think this would be much easier to take in without the confusing configuration option. It has no use on some filesystem and on others I don't think there is a good reason not to use it. Additionally some of the comments are misleadering or wrong. Could we initially just enable by default for ext4, this driver was initially not intended to be used on ext4 but this change could make it usable.

dmcgowan · 2025-10-16T20:23:35Z

+
+	// CopySparse forces the creation of a sparse file when copying
+	// the 'ScratchFile'
+	CopySparse bool `toml:"copy_sparse"`


If that is the case, then the provided scratch file should not be sparse. My understanding is the code is copy sparse only preserves the sparse nature, it does not make a non-sparse file sparse.

Additionally, this is not the right driver to use for that case, we have better options now. I think we should just enable this by default on ext4 and don't need to a config. Is there another filesystem in use where this would make sense?

mythi · 2025-10-31T14:07:27Z

Could we initially just enable by default for ext4, this driver was initially not intended to be used on ext4 but this change could make it usable.

I got the config entry dropped and hope to get it pushed through @fabiano back to this PR to get it in 2.2. I did some basic testing on ext4 and xfs using Kata containers.

$ sudo du -h /opt/containerd/blockfile
33M	/opt/containerd/blockfile
$ sudo ls -lh /opt/containerd/blockfile
-rw-r--r-- 1 root root 1.0G Oct 31 06:34 /opt/containerd/blockfile
$ sudo du -h /var/lib/containerd/io.containerd.snapshotter.v1.blockfile/scratch
33M	/var/lib/containerd/io.containerd.snapshotter.v1.blockfile/scratch
$ sudo ls -lh /var/lib/containerd/io.containerd.snapshotter.v1.blockfile/scratch
-rw-r--r-- 1 root root 1.0G Oct 31 06:40 /var/lib/containerd/io.containerd.snapshotter.v1.blockfile/scratch
$ sudo du -h /var/lib/containerd/io.containerd.snapshotter.v1.blockfile/snapshots/230
274M	/var/lib/containerd/io.containerd.snapshotter.v1.blockfile/snapshots/230
$ sudo ls -lh /var/lib/containerd/io.containerd.snapshotter.v1.blockfile/snapshots/230
-rw-r--r-- 1 root root 1.0G Oct 31 06:41 /var/lib/containerd/io.containerd.snapshotter.v1.blockfile/snapshots/230
...

This is needed as the golang implementation of `io.Copy` is equivalent to `cp --sparse=never ...`, meaning that even if the initial scratch file is a sparse file, it's copy will still be a non-sparse one, leading to each snapshot block file occupying the full blockfile set by the user. Signed-off-by: Fabiano Fidêncio <ffidencio@nvidia.com> Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>

fidencio · 2026-02-22T21:16:59Z

I'm closing this one as I have no intent to work on this.

github-project-automation Bot added this to Pull Request Review Jul 7, 2025

github-project-automation Bot moved this to Needs Triage in Pull Request Review Jul 7, 2025

k8s-ci-robot added size/M needs-ok-to-test labels Jul 7, 2025

hsiangkao reviewed Jul 8, 2025

View reviewed changes

Comment thread plugins/snapshots/blockfile/blockfile.go Outdated

jwnx reviewed Jul 8, 2025

View reviewed changes

Comment thread plugins/snapshots/blockfile/blockfile.go Outdated

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch from 5d5db28 to bf60f0b Compare July 8, 2025 09:52

k8s-ci-robot added size/L and removed size/M labels Jul 8, 2025

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch from bf60f0b to 8ce1824 Compare July 8, 2025 09:55

hsiangkao reviewed Jul 8, 2025

View reviewed changes

Comment thread plugins/snapshots/blockfile/blockfile.go Outdated

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch 8 times, most recently from b547de5 to 2cd2a04 Compare July 8, 2025 12:21

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch 2 times, most recently from ca7eddd to e47709b Compare July 8, 2025 14:27

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch 5 times, most recently from 5a84603 to d385f0a Compare July 9, 2025 17:28

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch from b25e0cd to 5a939b3 Compare July 18, 2025 23:28

dmcgowan added this to the 2.2 milestone Jul 19, 2025

dmcgowan moved this from Needs Triage to Needs Reviewers in Pull Request Review Jul 19, 2025

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch 2 times, most recently from 2ce808f to 84aeabc Compare July 19, 2025 09:24

AkihiroSuda reviewed Jul 20, 2025

View reviewed changes

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch 2 times, most recently from 666bd3d to 48b9ba4 Compare July 20, 2025 11:13

mythi reviewed Jul 21, 2025

View reviewed changes

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch 2 times, most recently from 4fcec3b to 53b11c3 Compare July 24, 2025 04:18

dmcgowan self-assigned this Oct 16, 2025

dmcgowan requested changes Oct 16, 2025

View reviewed changes

github-project-automation Bot moved this from Needs Reviewers to Needs Update in Pull Request Review Oct 16, 2025

dmcgowan modified the milestones: 2.2, 2.3 Oct 23, 2025

fidencio force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch 2 times, most recently from fdf2557 to 1652928 Compare October 31, 2025 14:16

mythi force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch from 1652928 to 5b11937 Compare November 4, 2025 07:37

mythi force-pushed the topic/support-sparsefiles-copy-in-blockfile-snapshotter-revisited branch from 5b11937 to 6cf6eb3 Compare November 4, 2025 07:45

fidencio closed this Feb 22, 2026

github-project-automation Bot moved this from Needs Update to Done in Pull Request Review Feb 22, 2026

dcantah mentioned this pull request Mar 2, 2026

Blockfile snapshotter: Preserve sparseness on linux #12956

Draft

Conversation

fidencio commented Jul 7, 2025

Uh oh!

k8s-ci-robot commented Jul 7, 2025

Uh oh!

fidencio commented Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fidencio commented Jul 8, 2025

Uh oh!

fidencio commented Jul 8, 2025

Uh oh!

AkihiroSuda Jul 20, 2025

Choose a reason for hiding this comment

Uh oh!

fidencio Jul 20, 2025

Choose a reason for hiding this comment

Uh oh!

fidencio Jul 20, 2025

Choose a reason for hiding this comment

Uh oh!

mythi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mythi Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

fidencio Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

dmcgowan Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mythi commented Sep 19, 2025

Uh oh!

ChengyuZhu6 commented Sep 19, 2025

Uh oh!

mythi commented Sep 25, 2025

Uh oh!

dmcgowan commented Oct 15, 2025

Uh oh!

dmcgowan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmcgowan Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

mythi commented Oct 31, 2025

Uh oh!

fidencio commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants