LISAFS client file read must return io.EOF when appropriate.

This is consistent with POSIX semantics and with 9P client file.
Some VFS callers like splice really depend on such a non-nil
error being returned from FD.Read(). Otherwise, sendfile will
hang forever, spaming FD.Read() for more data.

Added a regression test. This test reproduces the issue when:
- LISAFS is enabled.
- sendfile(2) called with input file for which we don't have
  cached mapping (so will need to read from gofer). Can be
  simulated with shared gofer mode.
- Host FD is not available for input file to perform read. Tested
  internally.
Hence this test will be more meaningful when lisafs is enabled
by default in all tests.

PiperOrigin-RevId: 458340830
This commit is contained in:
Ayush Ranjan
2022-06-30 16:44:27 -07:00
committed by gVisor bot
parent 1eb2c3659d
commit dba08ba26f
2 changed files with 41 additions and 1 deletions
+14 -1
View File
@@ -16,6 +16,7 @@ package lisafs
import (
"fmt"
"io"
"golang.org/x/sys/unix"
"gvisor.dev/gvisor/pkg/abi/linux"
@@ -174,7 +175,19 @@ func (f *ClientFD) Read(ctx context.Context, dst []byte, offset uint64) (uint64,
ctx.UninterruptibleSleepStart(false)
err := f.client.SndRcvMessage(PRead, uint32(req.SizeBytes()), req.MarshalUnsafe, resp.CheckedUnmarshal, nil, req.String, resp.String)
ctx.UninterruptibleSleepFinish(false)
return uint64(resp.NumBytes), err
if err != nil {
return 0, err
}
// io.EOF is not an error that a lisafs server can return. Use POSIX
// semantics to return io.EOF manually: zero bytes were returned and a
// non-zero buffer was used.
// NOTE(b/237442794): Some callers like splice really depend on a non-nil
// error being returned in such a case. This is consistent with P9.
if resp.NumBytes == 0 && len(buf) > 0 {
return 0, io.EOF
}
return uint64(resp.NumBytes), nil
})
}
+27
View File
@@ -18,6 +18,8 @@
#include <sys/sendfile.h>
#include <unistd.h>
#include <string_view>
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "absl/strings/string_view.h"
@@ -627,6 +629,31 @@ TEST(SendFileTest, SendFileToSelf) {
SyscallSucceedsWithValue(kSendfileSize));
}
// NOTE(b/237442794): Regression test. Make sure sendfile works with a count
// larger than input file size.
TEST(SendFileTest, LargeCount) {
// Create input file with some wisdom. It is imperative to use a
// Shakespearean quote, consistent with the rest of this file.
constexpr std::string_view kData =
"We know what we are, but know not what we may be.";
const TempPath in_file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(
GetAbsoluteTestTmpdir(), kData, TempPath::kDefaultFileMode));
const TempPath out_file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
// Open the input file as read only.
const FileDescriptor inf =
ASSERT_NO_ERRNO_AND_VALUE(Open(in_file.path(), O_RDONLY));
// Open the output file as write only.
const FileDescriptor outf =
ASSERT_NO_ERRNO_AND_VALUE(Open(out_file.path(), O_WRONLY));
// Set a count larger than kDataSize.
EXPECT_THAT(sendfile(outf.get(), inf.get(), nullptr, 2 * kData.size()),
SyscallSucceedsWithValue(kData.size()));
}
} // namespace
} // namespace testing