Skip to content

Commit

Permalink
fusebox: implement Read2 for the server
Browse files Browse the repository at this point in the history 
Prior to this commit, every Read call created a new FileStreamReader (at
a possibly non-zero offset). After this commit, an FSR from previous
Read calls may be re-used for the next Read call if it's already at the
correct offset.

The FUSE read API permits random access but, still, in the common case,
reads happen with sequential access. The FSR class is abstract and not
all implementations support seeking. If seeking is not supported then
re-using the FSR can, in that common case, avoid quadratic I/O
performance (see https://wiki.c2.com/?ShlemielThePainter).

Read2 / Close2 (and, later, Write2) also differ from Read / Close /
Write in that, at the protocol level, they take a numeric handle instead
of a string file name (or file_system_url). Just like how multiple
numerical-identifier file descriptors (with independent read/write
offsets) can refer to the same underlying file, multiple
numerical-identifier handles (and their linked FileStreamReader offsets)
can refer to the same virtual file (named by its FileSystemURL).

Passing a stable numeric handle (instead of an unstable file 'name')
also moves us closer to more POSIX-like behavior in the case where we're
reading from a (virtual) file concurrently with its parent directory
being re-named. It's not entirely fixed, e.g. if reading is random
access not sequential access, but it's closer than it used to be.

This is one half of a multi-repo change. The other half is at
https://crrev.com/c/4023308

BUG=b:255520194
TEST=manual

Change-Id: I8177b4873d1df3ab96e6d655da1a959872a502af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4022851
Reviewed-by: Ben Reich <benreich@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1073681}
  • Loading branch information
Nigel Tao authored and Chromium LUCI CQ committed Nov 19, 2022
1 parent 318ebe0 commit 3da7911
Show file tree
Hide file tree
Showing 5 changed files with 517 additions and 0 deletions.
116 changes: 116 additions & 0 deletions chrome/browser/ash/dbus/fusebox_service_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
// This file provides the "D-Bus protocol logic" half of the FuseBox server,
// coupled with the "business logic" half in fusebox_server.cc.

// The "fusebox_staging" concept is described in
// chrome/browser/ash/fusebox/fusebox_staging.proto
namespace fusebox_staging {
const char kRead2Method[] = "Read2";
}

namespace ash {

namespace {
Expand All @@ -39,6 +45,20 @@ void ReplyToClose(dbus::MethodCall* method_call,
std::move(sender).Run(std::move(response));
}

void ReplyToClose2(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender,
const fusebox_staging::Close2ResponseProto& response_proto) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

std::unique_ptr<dbus::Response> response =
dbus::Response::FromMethodCall(method_call);
dbus::MessageWriter writer(response.get());

writer.AppendProtoAsArrayOfBytes(response_proto);

std::move(sender).Run(std::move(response));
}

void ReplyToMkDir(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender,
const fusebox_staging::MkDirResponseProto& response_proto) {
Expand Down Expand Up @@ -69,6 +89,20 @@ void ReplyToOpen(dbus::MethodCall* method_call,
std::move(sender).Run(std::move(response));
}

void ReplyToOpen2(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender,
const fusebox_staging::Open2ResponseProto& response_proto) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

std::unique_ptr<dbus::Response> response =
dbus::Response::FromMethodCall(method_call);
dbus::MessageWriter writer(response.get());

writer.AppendProtoAsArrayOfBytes(response_proto);

std::move(sender).Run(std::move(response));
}

void ReplyToRead(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender,
int32_t posix_error_code,
Expand All @@ -86,6 +120,20 @@ void ReplyToRead(dbus::MethodCall* method_call,
std::move(sender).Run(std::move(response));
}

void ReplyToRead2(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender,
const fusebox_staging::Read2ResponseProto& response_proto) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

std::unique_ptr<dbus::Response> response =
dbus::Response::FromMethodCall(method_call);
dbus::MessageWriter writer(response.get());

writer.AppendProtoAsArrayOfBytes(response_proto);

std::move(sender).Run(std::move(response));
}

void ReplyToReadDir2(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender,
const fusebox::ReadDir2ResponseProto& response_proto) {
Expand Down Expand Up @@ -167,6 +215,11 @@ void FuseBoxServiceProvider::Start(scoped_refptr<dbus::ExportedObject> object) {
base::BindRepeating(&FuseBoxServiceProvider::Close,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&OnExportedCallback));
object->ExportMethod(fusebox::kFuseBoxServiceInterface,
fusebox::kClose2Method,
base::BindRepeating(&FuseBoxServiceProvider::Close2,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&OnExportedCallback));
object->ExportMethod(fusebox::kFuseBoxServiceInterface, fusebox::kMkDirMethod,
base::BindRepeating(&FuseBoxServiceProvider::MkDir,
weak_ptr_factory_.GetWeakPtr()),
Expand All @@ -175,10 +228,19 @@ void FuseBoxServiceProvider::Start(scoped_refptr<dbus::ExportedObject> object) {
base::BindRepeating(&FuseBoxServiceProvider::Open,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&OnExportedCallback));
object->ExportMethod(fusebox::kFuseBoxServiceInterface, fusebox::kOpen2Method,
base::BindRepeating(&FuseBoxServiceProvider::Open2,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&OnExportedCallback));
object->ExportMethod(fusebox::kFuseBoxServiceInterface, fusebox::kReadMethod,
base::BindRepeating(&FuseBoxServiceProvider::Read,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&OnExportedCallback));
object->ExportMethod(fusebox::kFuseBoxServiceInterface,
fusebox_staging::kRead2Method,
base::BindRepeating(&FuseBoxServiceProvider::Read2,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&OnExportedCallback));
object->ExportMethod(fusebox::kFuseBoxServiceInterface,
fusebox::kReadDir2Method,
base::BindRepeating(&FuseBoxServiceProvider::ReadDir2,
Expand Down Expand Up @@ -239,6 +301,24 @@ void FuseBoxServiceProvider::Close(
base::BindOnce(&ReplyToClose, method_call, std::move(sender)));
}

void FuseBoxServiceProvider::Close2(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

dbus::MessageReader reader(method_call);
fusebox_staging::Close2RequestProto request_proto;
if (!reader.PopArrayOfBytesAsProto(&request_proto)) {
fusebox_staging::Close2ResponseProto response_proto;
response_proto.set_posix_error_code(EINVAL);
ReplyToClose2(method_call, std::move(sender), response_proto);
return;
}

server_.Close2(request_proto, base::BindOnce(&ReplyToClose2, method_call,
std::move(sender)));
}

void FuseBoxServiceProvider::MkDir(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender) {
Expand Down Expand Up @@ -272,6 +352,24 @@ void FuseBoxServiceProvider::Open(dbus::MethodCall* method_call,
base::BindOnce(&ReplyToOpen, method_call, std::move(sender)));
}

void FuseBoxServiceProvider::Open2(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

dbus::MessageReader reader(method_call);
fusebox_staging::Open2RequestProto request_proto;
if (!reader.PopArrayOfBytesAsProto(&request_proto)) {
fusebox_staging::Open2ResponseProto response_proto;
response_proto.set_posix_error_code(EINVAL);
ReplyToOpen2(method_call, std::move(sender), response_proto);
return;
}

server_.Open2(request_proto,
base::BindOnce(&ReplyToOpen2, method_call, std::move(sender)));
}

void FuseBoxServiceProvider::Read(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
Expand All @@ -290,6 +388,24 @@ void FuseBoxServiceProvider::Read(dbus::MethodCall* method_call,
base::BindOnce(&ReplyToRead, method_call, std::move(sender)));
}

void FuseBoxServiceProvider::Read2(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

dbus::MessageReader reader(method_call);
fusebox_staging::Read2RequestProto request_proto;
if (!reader.PopArrayOfBytesAsProto(&request_proto)) {
fusebox_staging::Read2ResponseProto response_proto;
response_proto.set_posix_error_code(EINVAL);
ReplyToRead2(method_call, std::move(sender), response_proto);
return;
}

server_.Read2(request_proto,
base::BindOnce(&ReplyToRead2, method_call, std::move(sender)));
}

void FuseBoxServiceProvider::ReadDir2(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender) {
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/ash/dbus/fusebox_service_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,18 @@ class FuseBoxServiceProvider : public CrosDBusService::ServiceProviderInterface,
// corresponds to the standard stat function described by "man 2 stat".
void Close(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender);
void Close2(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender);
void MkDir(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender);
void Open(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender);
void Open2(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender);
void Read(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender);
void Read2(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender);
void ReadDir2(dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender sender);
void RmDir(dbus::MethodCall* method_call,
Expand Down

0 comments on commit 3da7911

Please sign in to comment.