-
Notifications
You must be signed in to change notification settings - Fork 382
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.
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
OCI layout extensions #2633
base: main
Are you sure you want to change the base?
OCI layout extensions #2633
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
//go:build cgo | ||
|
||
package reflink | ||
|
||
/* | ||
#include <linux/fs.h> | ||
|
||
#ifndef FICLONE | ||
#define FICLONE _IOW(0x94, 9, int) | ||
#endif | ||
*/ | ||
import "C" | ||
import ( | ||
"io" | ||
"os" | ||
|
||
"golang.org/x/sys/unix" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please run |
||
) | ||
|
||
// Copy attempts to reflink the source to the destination fd. | ||
// If reflinking fails or is unsupported, it falls back to io.Copy(). | ||
func Copy(src, dst *os.File) error { | ||
_, _, errno := unix.Syscall(unix.SYS_IOCTL, dst.Fd(), C.FICLONE, src.Fd()) | ||
if errno == 0 { | ||
return nil | ||
} | ||
|
||
_, err := io.Copy(dst, src) | ||
return err | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
//go:build !linux || !cgo | ||
|
||
package reflink | ||
|
||
import ( | ||
"io" | ||
"os" | ||
) | ||
|
||
// Copy attempts to reflink the source to the destination fd. | ||
// If reflinking fails or is unsupported, it falls back to io.Copy(). | ||
func Copy(src, dst *os.File) error { | ||
_, err := io.Copy(dst, src) | ||
return err | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
bbbbbbbbbbbbbbbbbbbbbbbb |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -17,6 +17,7 @@ import ( | |||||
"github.com/containers/image/v5/internal/manifest" | ||||||
"github.com/containers/image/v5/internal/private" | ||||||
"github.com/containers/image/v5/internal/putblobdigest" | ||||||
"github.com/containers/image/v5/internal/reflink" | ||||||
"github.com/containers/image/v5/types" | ||||||
"github.com/containers/storage/pkg/fileutils" | ||||||
digest "github.com/opencontainers/go-digest" | ||||||
|
@@ -38,6 +39,9 @@ type ociImageDestination struct { | |||||
|
||||||
// newImageDestination returns an ImageDestination for writing to an existing directory. | ||||||
func newImageDestination(sys *types.SystemContext, ref ociReference) (private.ImageDestination, error) { | ||||||
if ref.sourceIndex != -1 { | ||||||
return nil, fmt.Errorf("Destination reference must not contain a manifest index @%d", ref.sourceIndex) | ||||||
} | ||||||
var index *imgspecv1.Index | ||||||
if indexExists(ref) { | ||||||
var err error | ||||||
|
@@ -138,36 +142,46 @@ func (d *ociImageDestination) PutBlobWithOptions(ctx context.Context, stream io. | |||||
if inputInfo.Size != -1 && size != inputInfo.Size { | ||||||
return private.UploadedBlob{}, fmt.Errorf("Size mismatch when copying %s, expected %d, got %d", blobDigest, inputInfo.Size, size) | ||||||
} | ||||||
if err := blobFile.Sync(); err != nil { | ||||||
|
||||||
if err := d.blobFileSyncAndRename(blobFile, blobDigest, &explicitClosed); err != nil { | ||||||
return private.UploadedBlob{}, err | ||||||
} | ||||||
succeeded = true | ||||||
return private.UploadedBlob{Digest: blobDigest, Size: size}, nil | ||||||
} | ||||||
|
||||||
func (d *ociImageDestination) blobFileSyncAndRename(blobFile *os.File, blobDigest digest.Digest, closed *bool) error { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documenting the interaction of |
||||||
if err := blobFile.Sync(); err != nil { | ||||||
return err | ||||||
} | ||||||
|
||||||
// On POSIX systems, blobFile was created with mode 0600, so we need to make it readable. | ||||||
// On Windows, the “permissions of newly created files” argument to syscall.Open is | ||||||
// ignored and the file is already readable; besides, blobFile.Chmod, i.e. syscall.Fchmod, | ||||||
// always fails on Windows. | ||||||
if runtime.GOOS != "windows" { | ||||||
if err := blobFile.Chmod(0644); err != nil { | ||||||
return private.UploadedBlob{}, err | ||||||
return err | ||||||
} | ||||||
} | ||||||
|
||||||
blobPath, err := d.ref.blobPath(blobDigest, d.sharedBlobDir) | ||||||
if err != nil { | ||||||
return private.UploadedBlob{}, err | ||||||
return err | ||||||
} | ||||||
if err := ensureParentDirectoryExists(blobPath); err != nil { | ||||||
return private.UploadedBlob{}, err | ||||||
return err | ||||||
} | ||||||
|
||||||
// need to explicitly close the file, since a rename won't otherwise not work on Windows | ||||||
// need to explicitly close the file, since a rename won't otherwise work on Windows | ||||||
blobFile.Close() | ||||||
explicitClosed = true | ||||||
|
||||||
*closed = true | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Nit: Move this one line above, please, to keep it immediately next to the actual |
||||||
if err := os.Rename(blobFile.Name(), blobPath); err != nil { | ||||||
return private.UploadedBlob{}, err | ||||||
return err | ||||||
} | ||||||
succeeded = true | ||||||
return private.UploadedBlob{Digest: blobDigest, Size: size}, nil | ||||||
|
||||||
return nil | ||||||
} | ||||||
|
||||||
// TryReusingBlobWithOptions checks whether the transport already contains, or can efficiently reuse, a blob, and if so, applies it to the current destination | ||||||
|
@@ -300,6 +314,63 @@ func (d *ociImageDestination) CommitWithOptions(ctx context.Context, options pri | |||||
return os.WriteFile(d.ref.indexPath(), indexJSON, 0644) | ||||||
} | ||||||
|
||||||
// PutBlobFromLocalFileOptions is unused but may receive functionality in the future. | ||||||
type PutBlobFromLocalFileOptions struct{} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
please, multiple options can be independent items at the call site. |
||||||
|
||||||
// PutBlobFromLocalFile arranges the data from path to be used as blob with digest. | ||||||
// It computes, and returns, the digest and size of the used file. | ||||||
// | ||||||
// This function can be used instead of dest.PutBlob() where the ImageDestination requires PutBlob() to be called. | ||||||
func PutBlobFromLocalFile(ctx context.Context, dest types.ImageDestination, file string, options ...PutBlobFromLocalFileOptions) (digest.Digest, int64, error) { | ||||||
d, ok := dest.(*ociImageDestination) | ||||||
if !ok { | ||||||
return "", -1, errors.New("internal error: PutBlobFromLocalFile called with a non-oci: destination") | ||||||
} | ||||||
|
||||||
blobFileClosed := false | ||||||
blobFile, err := os.CreateTemp(d.ref.dir, "oci-put-blob") | ||||||
if err != nil { | ||||||
return "", -1, err | ||||||
} | ||||||
defer func() { | ||||||
if !blobFileClosed { | ||||||
blobFile.Close() | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
}() | ||||||
|
||||||
srcFile, err := os.Open(file) | ||||||
if err != nil { | ||||||
return "", -1, err | ||||||
} | ||||||
defer srcFile.Close() | ||||||
|
||||||
// reflink.Copy will io.Copy() in case reflinking fails | ||||||
err = reflink.Copy(srcFile, blobFile) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Absolutely non-blocking, this is internal-only anyway, and it works just fine as is: Maybe the function could be named explicitly enough to make the comment unnecessary, something like |
||||||
if err != nil { | ||||||
return "", -1, err | ||||||
} | ||||||
|
||||||
_, err = blobFile.Seek(0, 0) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
please |
||||||
if err != nil { | ||||||
return "", -1, err | ||||||
} | ||||||
blobDigest, err := digest.FromReader(blobFile) | ||||||
if err != nil { | ||||||
return "", -1, err | ||||||
} | ||||||
|
||||||
fileInfo, err := blobFile.Stat() | ||||||
if err != nil { | ||||||
return "", -1, err | ||||||
} | ||||||
|
||||||
if err := d.blobFileSyncAndRename(blobFile, blobDigest, &blobFileClosed); err != nil { | ||||||
return "", -1, err | ||||||
} | ||||||
|
||||||
return blobDigest, fileInfo.Size(), nil | ||||||
} | ||||||
|
||||||
func ensureDirectoryExists(path string) error { | ||||||
if err := fileutils.Exists(path); err != nil && errors.Is(err, fs.ErrNotExist) { | ||||||
if err := os.MkdirAll(path, 0755); err != nil { | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x/sys/unix.FICLONE
exists; if that works, we can avoid the CGo dependency.