storage: add splitfdstream for efficient layer transfer with reflink support by giuseppe · Pull Request #651 · podman-container-tools/container-libs

giuseppe · 2026-02-10T12:32:04Z

Add a new splitfdstream package that enables efficient container layer transfer between storage instances by separating tar metadata from file content. File content is passed as file descriptors.

mtrmac

6% of the code base for an extremely niche use case :/

I’m afraid I can’t spend time on this now; and anyway some other work on pkg/archive needs to happen first, and expect the rebase to be somewhat non-trivial.

giuseppe · 2026-02-10T15:55:34Z

6% of the code base for an extremely niche use case :/

I’m afraid I can’t spend time on this now; and anyway some other work on pkg/archive needs to happen first, and expect the rebase to be somewhat non-trivial.

what other work would block this feature?

I don't love either the amount of extra code, but it is almost all in new APIs/packages that are experimental and not leaking too much to the rest of the library.

We went around this problem for a long time, and in the end this seems like an ~ok solution to not expose too much of the containers storage internals augmenting the tar format that is already plugged everywhere.

giuseppe · 2026-02-10T17:43:19Z

to add more context: the goal of this feature is to be able to copy a layer, similarly to what we would do with Diff, but having the possibility to deduplicate files using reflinks. We need two separate copies because OS files can't be kept on the containers storage for different reasons, a podman system reset -f should not break the system, and OS files need to be relabeled.

There are other interesting use cases enabled with a RPC like facilitate usage from different languages (Rust in our case), inject into a build container to grab R/O access to layers/images, but this is out of scope now.

I realize complexity is not trivial just to get access to the raw files, but what alternatives do we have?

From the store API PoV we could just extend it to return a map $LAYER_FILE -> $PATH_TO_THE_FILE, but how would we use it from Rust?

Any suggestions?

mtrmac · 2026-02-10T18:24:55Z

(Just to avoid a misunderstanding, I didn’t really read the PR. I have, at this point, ~no opinion on RPC vs. Go API.)

cgwalters · 2026-02-10T19:18:18Z

+	return &tarReaderIterator{
+		tr:     tr,
+		trBuf:  trBuf,
+		buffer: make([]byte, 1<<20),


Seems like 1<<20 should be in a const somewhere with a rationale?

cgwalters · 2026-02-10T19:19:02Z

+
+// splitFDStreamSocketDescriptor is the fd for the Unix socket used to
+// receive file descriptors via SCM_RIGHTS in the re-exec child.
+const splitFDStreamSocketDescriptor = 5


Hardcoding this seems a bit fragile, can't we pass it as an env var or a CLI arg?

cgwalters · 2026-02-10T19:20:11Z

+// FDs are streamed to the child process one-at-a-time over a Unix socket
+// using SCM_RIGHTS, avoiding EMFILE from inheriting too many FDs at exec.


It doesn't have to be truly one at a time, see also https://github.com/cgwalters/jsonrpc-fdpass?tab=readme-ov-file#41-fd-batching

In my initial PoC work here I handled this by just sending the splitfdstream inline data itself as a fd (could be O_TMPFILE or memfd or pipe), and then sending all the associated fds over jsonrpc-fdpass which automatically handles buffering.

Ensure we processing just 1 recvmsg() at a time (only ~200 fds on Linux) seems really reasonable and unlikely to get anywhere close to modern fd limits.

So if we do choose jsonrpc-fdpass, I think we also need to standardize this "splitfdstream serialization".

cgwalters · 2026-02-10T19:30:29Z

+		return err
+	}
+
+	// Try copy_file_range - kernel-level copy, more efficient than userspace


Surprising we weren't doing this before.

cgwalters · 2026-02-10T19:33:57Z

+
+// ReadLine reads bytes until a newline is encountered, using recvmsg.
+// Any FDs received are closed since line-reading doesn't expect them.
+func (p *FDPasser) ReadLine() ([]byte, error) {


Why would one want this?

cgwalters · 2026-02-10T19:35:02Z

+
+// JSON-RPC 2.0 protocol constants
+const (
+	JSONRPCVersion = "2.0"


BTW see also https://github.com/cgwalters/jsonrpc-fdpass-go

giuseppe · 2026-02-11T16:13:53Z

still PoC, so you can avoid a review as it is not yet ready but we can use to analyze the complexity.

Moved to use https://github.com/cgwalters/jsonrpc-fdpass-go

Now the code size is almost 50% than the last revision

packit-as-a-service · 2026-02-11T16:46:06Z

Packit jobs failed. @containers/packit-build please check.

kolyshkin · 2026-05-04T22:50:02Z

I think these two commits can deserve own PR and merged separately:

storage, overlay: use openat2 instead of using procfs
composefs: do not leak open file

giuseppe · 2026-05-05T07:51:08Z

I think these two commits can deserve own PR and merged separately:

storage, overlay: use openat2 instead of using procfs

composefs: do not leak open file

I've created two new PRs with these patches:

I've left "storage, overlay: use openat2 instead of using procfs" as part of this PR as well to avoid rebase conflicts, I will drop it once #820 is merged

kolyshkin · 2026-05-08T16:40:57Z

+		s.TrackConnection()
+		go s.HandleConnection(unixConn)


nit: since storage/go.mod has go 1.25, you can use WaitGroup.Go instead of Add/Done. IOW replace this with

s.connections.Go(func() { s.HandleConnection(unixConn) })

and remove TrackConnection and the call to s.connections.Done().

Unless, of course, you need TrackConnection separately (I suspect I am missing something here since you made it public).

Right, you're using it later.

What if we make this internal, and make HandleConnection call connections.Go(func() { handleConnection(conn) })? I mean, do not expose the refcount details to the API

kolyshkin · 2026-05-08T18:04:31Z

 	roLayerStoresUseGetters []roLayerStore // Almost all users should use the provided accessors instead of accessing this field directly.

-	// FIXME: The following fields need locking, and don’t have it.
+	// FIXME: The following fields need locking, and don't have it.


Just checked there are another 62 instances of this symbol (it is https://en.wikipedia.org/wiki/Right_single_quotation_mark)

508 instances in the whole container-libs

kolyshkin · 2026-05-08T18:10:52Z

Sign in to view

+			}
+			driver, ok := s.graphDriver.(splitfdstream.SplitFDStreamDriver)
+			if !ok {
+				s.stopUsingGraphDriver()


my bad, I missed the startUsingGraphDriver.

kolyshkin · 2026-05-08T18:15:14Z

+			if !ok {
+				s.stopUsingGraphDriver()
+				return nil, nil, fmt.Errorf("driver does not support splitfdstream")
+			}


You already did this check above, no need to check it twice.

kolyshkin · 2026-05-08T18:25:42Z

+	if options == nil {
+		return nil, nil, fmt.Errorf("options cannot be nil")
+	}


not important, but you can do

if options == nil { options = &splitfdstream.GetSplitFDStreamOpts{} }

instead to slightly simplify usage by allowing nil for use "defaults" (if such usage makes sense).

kolyshkin · 2026-05-08T19:28:43Z

+		}
+
+		flatPath := string(buf[:n])
+		if strings.Contains(flatPath, "..") || filepath.IsAbs(flatPath) {


I think having .. in the file name is OK here (having .. as an element in path is not). The best way to check is probably something like

if filepath.Clean("/"+flatPath) != "/"+flatPath {

OR, if redirect attribute never contains ".." then the existing check is fine.

kolyshkin · 2026-05-08T19:32:08Z

+	var fd int
+	var openErr error
+
+	if composefsMountFd >= 0 {


Maybe factor out reading the attribute into a separate function, so that the code below can be unified?

kolyshkin · 2026-05-08T19:36:48Z

+	// Clean the file name to prevent path traversal
+	cleanName := filepath.Clean(header.Name)
+	if strings.Contains(cleanName, "..") {
+		return false, fmt.Errorf("invalid file path: %s", header.Name)
+	}


Perhaps something like this: https://github.com/opencontainers/runc/blob/main/internal/pathrs/path.go#L40

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Extend the store with splitfdstream capabilities exposed via a UNIX socket for JSON-RPC communication. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Implement the SplitFDStreamDriver interface for the overlay driver, enabling efficient layer operations with reflink support. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Add a new json-proxy method that returns a Unix socket file descriptor to the client. The client then speaks the jsonrpc-fdpass protocol directly over that socket for splitfdstream operations, bypassing the json-proxy for bulk data transfer. Bump protocolVersion to "0.2.9". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

cgwalters · 2026-06-08T19:45:33Z

Hi, I wanted to raise here that after some more investigation, I am proposing a change in direction. Basically, I found that varlink has "de facto" decent support for fd passing (originating from systemd). I think it makes sense to use that as a future replacement for the current skopeo proxy protocol, and to not use jsonrpc-fdpass.

See composefs/composefs-rs#309 for some PoC work.

mtrmac · 2026-06-08T20:19:18Z

I don’t know the details but ISTR Podman has felt burned by varlink in the past. @mheon ?

cgwalters · 2026-06-08T21:16:58Z

Right, obviously there's a huge history here in trying to use varlink as the RPC mechanism instead of the Docker HTTP API. No one is proposing having varlink at a scope as large as that, this would really just be a subset of image APIs that need fd passing and can't really be covered by the HTTP APIs.

github-actions Bot added the storage Related to "storage" package label Feb 10, 2026

This was referenced Feb 10, 2026

[WIP] composefs-splitfdstream: new crate composefs/composefs-rs#228

Draft

reflink by default (if possible) when copying between containers-storage: instances #144

Open

giuseppe force-pushed the splitfdstream branch 2 times, most recently from 4cca2a3 to 3b6f591 Compare February 10, 2026 14:59

mtrmac reviewed Feb 10, 2026

View reviewed changes

giuseppe force-pushed the splitfdstream branch from 3b6f591 to 26cd4b6 Compare February 10, 2026 16:01

cgwalters reviewed Feb 10, 2026

View reviewed changes

giuseppe force-pushed the splitfdstream branch 2 times, most recently from 23ed544 to 8d74f84 Compare February 11, 2026 16:07

giuseppe force-pushed the splitfdstream branch from 8d74f84 to 51a6c01 Compare February 11, 2026 16:25

giuseppe force-pushed the splitfdstream branch 2 times, most recently from 1b087bb to bc6c5ff Compare February 12, 2026 09:15

giuseppe mentioned this pull request Feb 12, 2026

Add an undocumented general "run with RPC service" podman-container-tools/buildah#6675

Merged

giuseppe force-pushed the splitfdstream branch 7 times, most recently from a0876b6 to d77fbc4 Compare February 17, 2026 12:44

giuseppe force-pushed the splitfdstream branch from d77fbc4 to 6d605ac Compare February 19, 2026 10:12

giuseppe changed the title ~~[WIP] storage: add splitfdstream for efficient layer transfer with reflink support~~ storage: add splitfdstream for efficient layer transfer with reflink support Feb 19, 2026

giuseppe force-pushed the splitfdstream branch from 6d605ac to 0ebb56c Compare February 19, 2026 14:49

giuseppe marked this pull request as ready for review February 19, 2026 14:58

giuseppe force-pushed the splitfdstream branch from b5d4056 to 45309a2 Compare May 5, 2026 07:49

giuseppe force-pushed the splitfdstream branch from 45309a2 to 7043b87 Compare May 5, 2026 16:49

kolyshkin mentioned this pull request May 5, 2026

storage, overlay: use openat2 instead of using procfs #820

Merged

kolyshkin reviewed May 5, 2026

View reviewed changes

Comment thread storage/pkg/archive/archive.go Outdated

giuseppe force-pushed the splitfdstream branch from 7043b87 to c302541 Compare May 6, 2026 07:24

kolyshkin reviewed May 8, 2026

View reviewed changes

giuseppe force-pushed the splitfdstream branch from c302541 to 9a07f72 Compare May 11, 2026 15:46

giuseppe and others added 8 commits May 12, 2026 13:01

storage, chunked: new function GenerateDumpFromTarHeaders

d6aa4dd

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

storage/pkg/splitfdstream: new package

1a751bc

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

storage/store: add new APIs to resolve image ID

3a86008

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

storage: add SplitFDStreamStore interface

f71950b

Extend the store with splitfdstream capabilities exposed via a UNIX socket for JSON-RPC communication. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

storage/archive,chrootarchive: add support for splitfdstream

a6f6ad7

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

storage/overlay: implement SplitFDStreamDriver

dabe421

Implement the SplitFDStreamDriver interface for the overlay driver, enabling efficient layer operations with reflink support. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

image, storage: expose the underlying SplitFDStreamStore

670d76d

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the splitfdstream branch from 9a07f72 to 7be3fef Compare May 12, 2026 11:01

cgwalters mentioned this pull request May 29, 2026

Unified storage bootc-dev/bootc#20

Open

giuseppe mentioned this pull request Jun 5, 2026

oci: Add varlink APIs using "splitdirfdstream" composefs/composefs-rs#309

Open

		// FDs are streamed to the child process one-at-a-time over a Unix socket
		// using SCM_RIGHTS, avoiding EMFILE from inheriting too many FDs at exec.

Uh oh!

Conversation

giuseppe commented Feb 10, 2026

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

giuseppe commented Feb 10, 2026

Uh oh!

giuseppe commented Feb 10, 2026

Uh oh!

mtrmac commented Feb 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

giuseppe commented Feb 11, 2026

Uh oh!

packit-as-a-service Bot commented Feb 11, 2026

Uh oh!

kolyshkin commented May 4, 2026

Uh oh!

giuseppe commented May 5, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kolyshkin May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Jun 8, 2026

Uh oh!

mtrmac commented Jun 8, 2026

Uh oh!

cgwalters commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kolyshkin May 8, 2026 •

edited

Loading