Update preadv2/pwritev2 flag handling in vfs2.

We do not support RWF_SYNC/RWF_DSYNC and probably shouldn't silently accept
them, since the user may incorrectly believe that we are synchronizing I/O.
Remove the pwritev2 test verifying that we support these flags.

gvisor.dev/issue/2601 is the tracking bug for deciding which RWF_.* flags
we need and supporting them.

Updates #2923, #2601.

PiperOrigin-RevId: 319351286
This commit is contained in:
Dean Deng
2020-07-01 22:04:42 -07:00
committed by gVisor bot
parent 7263fe5046
commit a17b052d37
7 changed files with 39 additions and 60 deletions
+3 -2
View File
@@ -191,8 +191,9 @@ var DirentType = abi.ValueSet{
// Values for preadv2/pwritev2.
const (
// Note: gVisor does not implement the RWF_HIPRI feature, but the flag is
// accepted as a valid flag argument for preadv2/pwritev2.
// NOTE(b/120162627): gVisor does not implement the RWF_HIPRI feature, but
// the flag is accepted as a valid flag argument for preadv2/pwritev2 and
// silently ignored.
RWF_HIPRI = 0x00000001
RWF_DSYNC = 0x00000002
RWF_SYNC = 0x00000004
+6 -2
View File
@@ -102,7 +102,9 @@ func (fd *regularFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs
return 0, syserror.EINVAL
}
// Check that flags are supported. Silently ignore RWF_HIPRI.
// Check that flags are supported.
//
// TODO(gvisor.dev/issue/2601): Support select preadv2 flags.
if opts.Flags&^linux.RWF_HIPRI != 0 {
return 0, syserror.EOPNOTSUPP
}
@@ -155,7 +157,9 @@ func (fd *regularFileFD) PWrite(ctx context.Context, src usermem.IOSequence, off
return 0, syserror.EINVAL
}
// Check that flags are supported. Silently ignore RWF_HIPRI.
// Check that flags are supported.
//
// TODO(gvisor.dev/issue/2601): Support select pwritev2 flags.
if opts.Flags&^linux.RWF_HIPRI != 0 {
return 0, syserror.EOPNOTSUPP
}
+6 -2
View File
@@ -130,7 +130,9 @@ func (fd *specialFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs
return 0, syserror.EINVAL
}
// Check that flags are supported. Silently ignore RWF_HIPRI.
// Check that flags are supported.
//
// TODO(gvisor.dev/issue/2601): Support select preadv2 flags.
if opts.Flags&^linux.RWF_HIPRI != 0 {
return 0, syserror.EOPNOTSUPP
}
@@ -176,7 +178,9 @@ func (fd *specialFileFD) PWrite(ctx context.Context, src usermem.IOSequence, off
return 0, syserror.EINVAL
}
// Check that flags are supported. Silently ignore RWF_HIPRI.
// Check that flags are supported.
//
// TODO(gvisor.dev/issue/2601): Support select pwritev2 flags.
if opts.Flags&^linux.RWF_HIPRI != 0 {
return 0, syserror.EOPNOTSUPP
}
+3 -1
View File
@@ -588,8 +588,10 @@ func (f *fileDescription) Read(ctx context.Context, dst usermem.IOSequence, opts
}
func readFromHostFD(ctx context.Context, hostFD int, dst usermem.IOSequence, offset int64, flags uint32) (int64, error) {
// Check that flags are supported.
//
// TODO(gvisor.dev/issue/2601): Support select preadv2 flags.
if flags != 0 {
if flags&^linux.RWF_HIPRI != 0 {
return 0, syserror.EOPNOTSUPP
}
reader := hostfd.GetReadWriterAt(int32(hostFD), offset, flags)
+10 -4
View File
@@ -295,8 +295,11 @@ func (fd *regularFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs
return 0, syserror.EINVAL
}
// Check that flags are supported. Silently ignore RWF_HIPRI.
if opts.Flags&^linux.RWF_HIPRI != 0 {
// Check that flags are supported. RWF_DSYNC/RWF_SYNC can be ignored since
// all state is in-memory.
//
// TODO(gvisor.dev/issue/2601): Support select preadv2 flags.
if opts.Flags&^(linux.RWF_HIPRI|linux.RWF_DSYNC|linux.RWF_SYNC) != 0 {
return 0, syserror.EOPNOTSUPP
}
@@ -326,8 +329,11 @@ func (fd *regularFileFD) PWrite(ctx context.Context, src usermem.IOSequence, off
return 0, syserror.EINVAL
}
// Check that flags are supported. Silently ignore RWF_HIPRI.
if opts.Flags&^linux.RWF_HIPRI != 0 {
// Check that flags are supported. RWF_DSYNC/RWF_SYNC can be ignored since
// all state is in-memory.
//
// TODO(gvisor.dev/issue/2601): Support select preadv2 flags.
if opts.Flags&^(linux.RWF_HIPRI|linux.RWF_DSYNC|linux.RWF_SYNC) != 0 {
return 0, syserror.EOPNOTSUPP
}
+1
View File
@@ -531,6 +531,7 @@ syscall_test(
syscall_test(
add_overlay = True,
test = "//test/syscalls/linux:pwritev2_test",
vfs2 = "True",
)
syscall_test(
+10 -49
View File
@@ -69,7 +69,7 @@ ssize_t pwritev2(unsigned long fd, const struct iovec* iov,
}
// This test is the base case where we call pwritev (no offset, no flags).
TEST(Writev2Test, TestBaseCall) {
TEST(Writev2Test, BaseCall) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
@@ -97,7 +97,7 @@ TEST(Writev2Test, TestBaseCall) {
}
// This test is where we call pwritev2 with a positive offset and no flags.
TEST(Pwritev2Test, TestValidPositiveOffset) {
TEST(Pwritev2Test, ValidPositiveOffset) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
std::string prefix(kBufSize, '0');
@@ -129,7 +129,7 @@ TEST(Pwritev2Test, TestValidPositiveOffset) {
// This test is the base case where we call writev by using -1 as the offset.
// The write should use the file offset, so the test increments the file offset
// prior to call pwritev2.
TEST(Pwritev2Test, TestNegativeOneOffset) {
TEST(Pwritev2Test, NegativeOneOffset) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
const std::string prefix = "00";
@@ -164,7 +164,7 @@ TEST(Pwritev2Test, TestNegativeOneOffset) {
// pwritev2 requires if the RWF_HIPRI flag is passed, the fd must be opened with
// O_DIRECT. This test implements a correct call with the RWF_HIPRI flag.
TEST(Pwritev2Test, TestCallWithRWF_HIPRI) {
TEST(Pwritev2Test, CallWithRWF_HIPRI) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
@@ -189,47 +189,8 @@ TEST(Pwritev2Test, TestCallWithRWF_HIPRI) {
EXPECT_EQ(buf, content);
}
// This test checks that pwritev2 can be called with valid flags
TEST(Pwritev2Test, TestCallWithValidFlags) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
GetAbsoluteTestTmpdir(), "", TempPath::kDefaultFileMode));
const FileDescriptor fd =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR));
std::vector<char> content(kBufSize, '0');
struct iovec iov;
iov.iov_base = content.data();
iov.iov_len = content.size();
EXPECT_THAT(pwritev2(fd.get(), &iov, /*iovcnt=*/1,
/*offset=*/0, /*flags=*/RWF_DSYNC),
SyscallSucceedsWithValue(kBufSize));
std::vector<char> buf(content.size());
EXPECT_THAT(read(fd.get(), buf.data(), buf.size()),
SyscallSucceedsWithValue(buf.size()));
EXPECT_EQ(buf, content);
SetContent(content);
EXPECT_THAT(pwritev2(fd.get(), &iov, /*iovcnt=*/1,
/*offset=*/0, /*flags=*/0x4),
SyscallSucceedsWithValue(kBufSize));
ASSERT_THAT(lseek(fd.get(), 0, SEEK_CUR),
SyscallSucceedsWithValue(content.size()));
EXPECT_THAT(pread(fd.get(), buf.data(), buf.size(), /*offset=*/0),
SyscallSucceedsWithValue(buf.size()));
EXPECT_EQ(buf, content);
}
// This test calls pwritev2 with a bad file descriptor.
TEST(Writev2Test, TestBadFile) {
TEST(Writev2Test, BadFile) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
ASSERT_THAT(pwritev2(/*fd=*/-1, /*iov=*/nullptr, /*iovcnt=*/0,
/*offset=*/0, /*flags=*/0),
@@ -237,7 +198,7 @@ TEST(Writev2Test, TestBadFile) {
}
// This test calls pwrite2 with an invalid offset.
TEST(Pwritev2Test, TestInvalidOffset) {
TEST(Pwritev2Test, InvalidOffset) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
@@ -255,7 +216,7 @@ TEST(Pwritev2Test, TestInvalidOffset) {
SyscallFailsWithErrno(EINVAL));
}
TEST(Pwritev2Test, TestUnseekableFileValid) {
TEST(Pwritev2Test, UnseekableFileValid) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
int pipe_fds[2];
@@ -285,7 +246,7 @@ TEST(Pwritev2Test, TestUnseekableFileValid) {
// Calling pwritev2 with a non-negative offset calls pwritev. Calling pwritev
// with an unseekable file is not allowed. A pipe is used for an unseekable
// file.
TEST(Pwritev2Test, TestUnseekableFileInValid) {
TEST(Pwritev2Test, UnseekableFileInvalid) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
int pipe_fds[2];
@@ -304,7 +265,7 @@ TEST(Pwritev2Test, TestUnseekableFileInValid) {
EXPECT_THAT(close(pipe_fds[1]), SyscallSucceeds());
}
TEST(Pwritev2Test, TestReadOnlyFile) {
TEST(Pwritev2Test, ReadOnlyFile) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
@@ -323,7 +284,7 @@ TEST(Pwritev2Test, TestReadOnlyFile) {
}
// This test calls pwritev2 with an invalid flag.
TEST(Pwritev2Test, TestInvalidFlag) {
TEST(Pwritev2Test, InvalidFlag) {
SKIP_IF(pwritev2(-1, nullptr, 0, 0, 0) < 0 && errno == ENOSYS);
const TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(