-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Support size option when creating tmpfs volumes #20451
Conversation
345d123
to
df5edfa
Compare
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.
Test suggestion, and, very confusing wording.
@@ -431,11 +431,14 @@ EOF | |||
|
|||
@test "podman volume type=tmpfs" { | |||
myvolume=myvol$(random_string) | |||
run_podman volume create -o type=tmpfs -o device=tmpfs $myvolume | |||
run_podman volume create -o type=tmpfs -o o=size=2M -o device=tmpfs $myvolume |
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.
Suggestion: nuke lines 439-441, and replace this block here with:
run_podman run --rm -v $myvolume:/vol $IMAGE stat -f -c "%T %b %s" /vol
read fstype fsblocks fsblocksize <<<"$output"
assert "$fstype" = "tmpfs" "stat %T - filesystem type"
assert "$(($fsblocks * $fsblocksize))" == "$((2048 * 1024))" "filesystem size (product of nblocks * blocksize)"
@@ -55,7 +55,9 @@ The `o` option sets options for the mount, and is equivalent to the filesystem | |||
options (also `-o`) passed to **mount(8)** with the following exceptions: | |||
|
|||
- The `o` option supports `uid` and `gid` options to set the UID and GID of the created volume that are not normally supported by **mount(8)**. | |||
- The `o` option supports the `size` option to set the maximum size of the created volume, the `inodes` option to set the maximum number of inodes for the volume and `noquota` to completely disable quota support even for tracking of disk usage. Currently these flags are only supported on "xfs" file system mounted with the `prjquota` flag described in the **xfs_quota(8)** man page. | |||
- The `o` option supports the `size` option to set the maximum size of the created volume, the `inodes` option to set the maximum number of inodes for the volume and `noquota` to completely disable quota support even for tracking of disk usage. |
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.
Sometimes I can forgive a missing Oxford comma, but absolutely not here. Please add one here:
...for the volume, and `noquota`
^---- between "volume" and "and"
The `size` and `inodes` options are supported on "xfs" file system mounted with the `prjquota` flag described in the **xfs_quota(8)** man page. | ||
The `size` option is supported on "tmpfs" file systems. |
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.
I'm having a hard time with this. I find this more understandable, but am not sure if it's correct:
The `size` option is supported only on `tmpfs` and `xfs[note]` filesystems.
The `inodes` option is supported only on `xfs[note]`. Any other use will throw an error, maybe, I dunno?
Note: xfs filesystems must be mounted with the `prjquota` flag as described in **xfs_quota(8)**. Podman will throw an error if they're not.
Code LGTM |
Fixes: containers#20449 Signed-off-by: Daniel J Walsh <[email protected]>
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.
I let @edsantiago give his final blessing
- The `o` option supports the `size` option to set the maximum size of the created volume, the `inodes` option to set the maximum number of inodes for the volume, and `noquota` to completely disable quota support even for tracking of disk usage. | ||
The `size` option is supported on the "tmpfs" and "xfs[note]" file systems. | ||
The `inodes` option is supported on the "xfs[note]" file systems. | ||
Note: xfs filesystems must be mounted with the `prjquota` flag described in the **xfs_quota(8)** man page. Podman will throw an error if they're not. |
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.
LGTM, but can someone confirm if this is true? I can't find a way to get a system with xfs.
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.
@umohnani8 is working with this now.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes: #20449
Does this PR introduce a user-facing change?