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
[testing] use smaller filesize on 4KB pagesize systems #2132
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2132 +/- ##
=======================================
Coverage 45.53% 45.53%
=======================================
Files 80 80
Lines 8726 8726
=======================================
Hits 3973 3973
Misses 4107 4107
Partials 646 646
Continue to review full report at Codecov.
|
snapshots/btrfs/btrfs_test.go
Outdated
deviceName, cleanupDevice, err := testutil.NewLoopback(650 << 20) // 650 MB | ||
loopbackSize := int64(100 << 20) // 100 MB | ||
// mkfs.btrfs needs > 650MB space on ppc64le because of its larger default pagesize (64KB vs 4KB) | ||
if os.Getenv("GOARCH") == "ppc64le" { |
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.
runtime.GOARCH
? (How is this env var set?)
snapshots/btrfs/btrfs_test.go
Outdated
@@ -29,7 +29,13 @@ func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshots.Snap | |||
|
|||
return func(ctx context.Context, root string) (snapshots.Snapshotter, func() error, error) { | |||
|
|||
deviceName, cleanupDevice, err := testutil.NewLoopback(650 << 20) // 650 MB | |||
loopbackSize := int64(100 << 20) // 100 MB | |||
// mkfs.btrfs needs > 650MB space on ppc64le because of its larger default pagesize (64KB vs 4KB) |
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.
sysconf(_SC_PAGESIZE)
?
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.
@AkihiroSuda I'm not sure what you are asking. That is one of the ways to get the value, yes.
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 meant we can check pagesize rather than GOARCH.
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.
ahh yeah that would be probably be better
84a7891
to
2462ce8
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.
LGTM
minor nit: given it's not implicitly clear why the linkage between page size and temp filesystem mount, a comment might be useful for someone else coming across this code |
Use a smaller filesize on systems that have a default 4KB pagesize. This is because mkfs.btrfs uses the default system pagesize as blocksize when creating a device, so if the pagesize is larger, then the file needs to be larger as well. This larger filesize is needed specifically for systems where 64KB is default, such as ppc64le. Signed-off-by: Christopher Jones <tophj@linux.vnet.ibm.com>
2462ce8
to
6e37011
Compare
LGTM |
Uses a much smaller value when creating this file to be used
as a loopback device on non-ppc64le platforms.
This file has to be so large on ppc64le, because mkfs.btrfs
uses the default architecture pagesize as blocksize, which is 64KB
on ppc64le, but only 4KB on amd64 and s390x.
Signed-off-by: Christopher Jones tophj@linux.vnet.ibm.com