This enables deferred deletion of physical files outside of global locks, improving performance and reducing lock contention.
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
The `SupportsShifting` method signature has been updated
to include `uidmap` and `gidmap` parameters.
Previously, a driver could only declare general support
for shifting.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace the in-memory buffer with a O_TMPFILE file. This reduces the
memory requirements for a partial pull since the tar-split data can be
written to disk.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This form is available since Go 1.22 (see
https://tip.golang.org/ref/spec#For_range) and will probably be seen
more and more in the new code.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
when an imagestore is used, a R/O layer must be written to the
layers.json under the imagestore, not graphroot.
The lock on the imagestore is already taken through the
multipleLockFile{} mechanism in place.
Closes: https://github.com/containers/storage/issues/2257
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
In go one should never modify a slice while also iterating over it at
the same time. This causes weird side effects as the underlying array
elements are shifted around without the range loop index knowing.
So if you delete a element the loop will then actually skip the next one
and theoretically access out of bounds on the last element which does
not panic but rather return the default zero type, nil here which then
causes the panic on layer.Flags == nil.
Here is a simple example to show the behavior:
func main() {
slice := []int{1, 2, 3, 4, 5, 6, 7, 8, 9}
for _, num := range slice {
if num == 5 {
slice = slices.DeleteFunc(slice, func(n int) bool {
return n == 5
})
}
fmt.Println(num)
}
}
The loop will not print 6, but then as last number it prints 0 (the
default zero type for an int).
Fixes#2184
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Removes the duplicate copy*Map function using the general function newMapFrom.
Reduces the allocation of empty maps using the copyMapPrefferingNil function.
This change may affect the behavior so that instead of an empty allocated map, a nil will be returned.
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
Removes duplicate copy*Slice functions using a generic copy function
or replaces them with the slices.Clone function.
Also simplifies the stringSliceWithoutValue function.
These changes should not change the behavior.
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
The current value obtained by summing the sizes of regular file contents
does not match the size of the uncompressed layer tarball.
We don't have a convenient source to compute the correct size
for estargz without pulling the full layer and defeating the point;
so we must allow for the size being unknown.
For recent zstd:chunked images, we have the full tar-split,
so we can compute the correct size; that will happen in
the following commits.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
because it does not return nil when the slice length is 0.
This behavior caused the slices.Clone function to allocate
a unnecessary amount of memory when the slice length is 0,
and the c/common tests failed.
Signed-off-by: Jan Rodák <hony.com@seznam.cz>
the global singleton was never updated, causing the cache to be always
recreated for each layer.
It is not possible to keep the layersCache mutex for the entire load()
since it calls into some store APIs causing a deadlock since
findDigestInternal() is already called while some store locks are
held.
Another benefit is that now only one goroutine can run load()
preventing multiple calls to load() to happen in parallel doing the
same work.
Closes: https://github.com/containers/storage/issues/2023
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Use the "slices", "maps" standard library packages, or other
readily-available features.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Conservatively use Index* + Delete to delete the
first element where it's not obvious that the code would really
want to delete all instances.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
if the compressed digest was validated, as it happens when
'pull_options = {convert_images = "true"}' is set, then store it as
well so that reusing the blob by its compressed digest works.
Previously, when an image converted to zstd:chunked was pulled a
second time, it would not be recognized by its compressed digest,
resulting in the need to re-pull the image again.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
SetBigData itself calls saveFor; so doing that before raises
fewer questions about stale data / stepping over each other.
The change in timing is externally-observable, but should hopefully
not matter much in practice, because this code is typically called
from layerStore.create as a part of an atomic create+populate operation
proptected by incompleteFlag.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Add a race-condition-free alternative to using CreateLayer and
ApplyDiffFromStagingDirectory, ensuring the store is locked for the
entire duration while the layer is being created and populated.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
enforce that the stagingDirectory must have the same value as the
diffOutput.Target variable. It allows to simplify the internal API.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This allows us to correctly set (CompresedDigest, CompressedSize)
when copying data from another layer; in that case we don't have the
compressed data, so computing the size from compressedCounter
sets an incorrect value.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
introduce the TOCDigest field for a layer. TOCDigest is designed to
store the digest of the Table of Contents (TOC) of the blob.
It is useful when the UncompressedDigest cannot be validated during a
partial image pull, but the TOC itself is validated.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
AFAICS this call is intended to "remap" the parent layer's contents to the
desired IDMappings; but when there is no parent layer, there is
nothing to remap.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
change the file format to store the tar-split as part of the
zstd:chunked image. This will allow clients to rebuild the entire
tarball without having to download it fully.
also store the uncompressed digest for the tarball, so that it can be
stored into the storage database.
Needs: https://github.com/containers/image/pull/1976
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Handle old-fashioned ID mappings when looking at layers. Nowadays,
we'll use an idmapped mount if we can, but we shouldn't blow up if we
had to chown a layer because we couldn't use an idmapped mount.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
tarLogger calls the provided callback in a separate
goroutine, and that can happen after tarLogger.Write
returns; tarLogger.Close is requried to ensure the callbacks
have all been correctly called, and the created uidLog and gidLog
values can be consumed.
So, move most of the IO pipeline that is formed around the
layer stream into a nested function that terminates earlier, notably
so that the "defer idLogger.Close()" is called at the appropriate time.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
We will want to move the next part of the code
into a closure; move variables that will be
accessed outside of that section.
Should not change behavior.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
AFAICS that can't fail with current pgzip; and
pgzip.NewWriter also calls NewWriteLevel, but it just
swallows the error.
Any failure would therefore be very unexpected;
report it instead of suppressing it.0
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
gofumpt is a superset of gofmt, enabling some more code formatting
rules.
This commit is brought to you by
gofumpt -w .
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The lockfile's write record is now updated prior to the actual write
operation. This ensures that, in the event of an unexpected
termination, other processes are correctly notified of an initiated
write operation.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
The documentation says
> The new Buffer takes ownership of buf, and the
> caller should not use buf after this call.
so use the more directly applicable, and simpler, bytes.Reader
instead, to avoid this potentially risky use.
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Implement ListLayers() for the aufs, btrfs, and devicemapper drivers,
along with a unit test for them.
Stop filtering out directories with names that aren't 64-hex chars in
vfs and overlay ListLayers() implementations, which is more a convention
than a hard rule.
Have layerStore.Wipe() try to remove remaining listed layers after it
removes the layers that the layerStore knew of.
Close() a dangling ReadCloser in NaiveCreateFromTemplate.
Switch from using plain defer to using t.Cleanup() to handle deleting
layers that tests create, have the addManyLayers() test function do so
as well.
Remove vfs.CopyDir, which near as I can tell isn't referenced anywhere.
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Jan Rodák