erofs-differ: support EROFS native image layers #11733
Conversation
|
@hsiangkao: GitHub didn't allow me to request PR reviews from the following users: rchincha. Note that only containerd members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this: 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. |
|
Note that it still needs some decision. Therefore, I don't touch |
|
@hsiangkao can you pls rebase the PR against |
|
I think we can skip the configuration for this as using a new media type would already be optional and previously unhandled. Whatever ever format we support here we would need to support "forever", but I think that is reasonable here |
|
@rchincha this aligns with what is proposed there. Although I would also agree that OCI should not define a new media type here since there is nothing OCI specific about the format. With the tar layers, there are entries with special meaning that warranted a more specific media type. We need to update |
This would be ideal for future extensibility. You would still need the unpacker for a given/configured media-type. opencontainers/image-spec#1191 (comment) Can adjust stacker to produce layers with the updated media-type (@hsiangkao had a comment about it as well) |
@dmcgowan I wonder if and adjust Does that seem more reasonable? Cc @rchincha |
|
if we check if |
ok, rebased. |
Yeah, I'm fine with that and dropped. |
I think its best to avoid touching this file. It is easy to backport support for media types but it is hard to deprecate them once they are in. I would avoid listing out the whole media types and maybe stick with just checking for We will need to continue to address new media types, I don't want that discussion to hold this up so we can continue experimenting. |
|
Also opened #11744 to unblock experimentation for new functionality |
hsiangkao
force-pushed
the
erofs-layers
branch
2 times, most recently
from
3eab2ca to
1eef0f8
Compare
plugins/diff/erofs/differ_linux.go
Outdated
| @@ -245,6 +268,20 @@ func (s erofsDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts [] | |||
| } | |||
| defer ra.Close() | |||
|
|
|||
| layerBlobPath := path.Join(layer, "layer.erofs") | |||
| if native { | |||
| // TODO: prefer to use hardlink as an optimization here | |||
There was a problem hiding this comment.
io.Copy in this case will usually do a copy_file_range which is better in this when reflinking is supported. We can also optimize this for the mac case to clone. I think we can remove this comment
There was a problem hiding this comment.
yes, but it's only applied to fses which supports reflink... ext4 is a primary fs which is hard to support reflink anyway.
There was a problem hiding this comment.
The comment is removed.
| if err != nil { | ||
| return emptyDesc, err | ||
| } | ||
| _, err = io.Copy(f, content.NewReader(ra)) |
There was a problem hiding this comment.
f should be closed unconditionally after this call
Naming quagmire ... has to be situated under some vendor tree, but which one! Would prioritize getting containerd to launch containers with these images/layers first. |
If the layer media type is expected as an EROFS native layer (ending with `.erofs`), copy the content as the layer blob. Signed-off-by: Gao Xiang <[email protected]>
github-project-automation
bot
moved this from Needs Update
to Review In Progress
in Pull Request Review
github-project-automation
bot
moved this from Review In Progress
to Done
in Pull Request Review
If the layer media type is expected as an EROFS native layer, copy the content as the layer blob.