Skip to content

fs: add TestCopyReflinkWithXFS#128

Merged
estesp merged 1 commit intocontainerd:masterfrom
AkihiroSuda:test-reflink
Oct 3, 2018
Merged

fs: add TestCopyReflinkWithXFS#128
estesp merged 1 commit intocontainerd:masterfrom
AkihiroSuda:test-reflink

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

This PR makes sure CopyFile() supports reflink on XFS.

This PR also covers #127

Comment thread fs/copy_linux_test.go Outdated
if err != nil {
t.Fatal(err)
}
if _, err := io.CopyN(a, rand.Reader, aSize); err != nil {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't use crypto rand for this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What should I do instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

math/rand should work just fine here and be much faster

Comment thread testutil/loopback_linux.go Outdated
@AkihiroSuda
Copy link
Copy Markdown
Member Author

updated

Comment thread fs/copy_linux_test.go
@@ -0,0 +1,81 @@
// +build linux
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The file header checking was added, that needs to be added here to pass CI

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Oct 2, 2018

LGTM after header added to make CI hapy

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
@AkihiroSuda
Copy link
Copy Markdown
Member Author

updated

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit be9bd76 into containerd:master Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants