Skip to content

Fix quota option handling#729

Merged
giuseppe merged 6 commits intocontainers:mainfrom
mtrmac:quota-options
Apr 23, 2026
Merged

Fix quota option handling#729
giuseppe merged 6 commits intocontainers:mainfrom
mtrmac:quota-options

Conversation

@mtrmac
Copy link
Copy Markdown
Contributor

@mtrmac mtrmac commented Mar 30, 2026

Primarily fix overlay so that it doesn’t modify callers’ StorageOpt.

Also report errors earlier, remove duplication, and other minor cleanups. See individual commit messages for details.

Motivated by #719; Cc: @akhilsaivenkata .

@github-actions github-actions Bot added the storage Related to "storage" package label Mar 30, 2026
@packit-as-a-service
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@giuseppe
Copy link
Copy Markdown
Member

@mtrmac anything more you want to do on this PR or can we merge it?

mtrmac added 6 commits April 23, 2026 13:22
This is more consistent with the other graph drivers.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This avoids the awkward creation of a Driver object just to pass that
around.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Don't overwrite the caller's StorageOpt map; instead, handle that
at parse time.

To implement defaults in parseStorageOpt, we need to call it
even if opts is nil or StorageOpt is nil.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Don't have it in two places.

So that we fail without affecting the file system, move the parseStorageOpt
call earlier.

Unknown options when creating read-only layers now cause
failures (but this is all hypothetical anyway, because c/storage does not
allow external callers to pass StorageOpt when creating read-only layers.)

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... so that we don't change the filesystem on error.

Also move the "opts == nil" handling, to be consistent
with overlay.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac
Copy link
Copy Markdown
Contributor Author

mtrmac commented Apr 23, 2026

@giuseppe I think this is ready for merging. I have only rebased it now, to make sure it didn’t break in the meantime.

Copy link
Copy Markdown
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@giuseppe giuseppe merged commit 5e61f82 into containers:main Apr 23, 2026
36 of 37 checks passed
@mtrmac mtrmac deleted the quota-options branch April 23, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

storage Related to "storage" package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants