Skip to content
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

[WasmFS] Allow move to return specific error codes #17785

Merged
merged 3 commits into from
Sep 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions src/library_wasmfs_opfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,16 @@ mergeInto(LibraryManager.library, {

_wasmfs_opfs_move__deps: ['$wasmfsOPFSFileHandles',
'$wasmfsOPFSDirectoryHandles'],
_wasmfs_opfs_move: async function(ctx, fileID, newDirID, namePtr) {
_wasmfs_opfs_move: async function(ctx, fileID, newDirID, namePtr, errPtr) {
let name = UTF8ToString(namePtr);
let fileHandle = wasmfsOPFSFileHandles.get(fileID);
let newDirHandle = wasmfsOPFSDirectoryHandles.get(newDirID);
// TODO: error handling
await fileHandle.move(newDirHandle, name);
try {
await fileHandle.move(newDirHandle, name);
} catch {
let err = -{{{ cDefine('EIO') }}};
{{{ makeSetValue('errPtr', 0, 'err', 'i32') }}};
}
_emscripten_proxy_finish(ctx);
},

Expand Down Expand Up @@ -354,9 +358,13 @@ mergeInto(LibraryManager.library, {
_wasmfs_opfs_get_size_access__deps: ['$wasmfsOPFSAccessHandles'],
_wasmfs_opfs_get_size_access: async function(ctx, accessID, sizePtr) {
let accessHandle = wasmfsOPFSAccessHandles.get(accessID);
// TODO: Error handling
let size = await accessHandle.getSize();
{{{ makeSetValue('sizePtr', 0, 'size', 'i32') }}};
let size;
try {
size = await accessHandle.getSize();
} catch {
size = -{{{ cDefine('EIO') }}};
}
{{{ makeSetValue('sizePtr', 0, 'size', 'i64') }}};
_emscripten_proxy_finish(ctx);
},

Expand All @@ -369,14 +377,21 @@ mergeInto(LibraryManager.library, {
_wasmfs_opfs_get_size_file__deps: ['$wasmfsOPFSFileHandles'],
_wasmfs_opfs_get_size_file: async function(ctx, fileID, sizePtr) {
let fileHandle = wasmfsOPFSFileHandles.get(fileID);
// TODO: Error handling
let size = (await fileHandle.getFile()).size;
{{{ makeSetValue('sizePtr', 0, 'size', 'i32') }}};
let size;
try {
size = (await fileHandle.getFile()).size;
} catch {
size = -{{{ cDefine('EIO') }}};
}
{{{ makeSetValue('sizePtr', 0, 'size', 'i64') }}};
_emscripten_proxy_finish(ctx);
},

_wasmfs_opfs_set_size_access__deps: ['$wasmfsOPFSAccessHandles'],
_wasmfs_opfs_set_size_access: async function(ctx, accessID, size, errPtr) {
_wasmfs_opfs_set_size_access: async function(ctx, accessID,
{{{ defineI64Param('size') }}},
errPtr) {
{{{ receiveI64ParamAsDouble('size') }}};
let accessHandle = wasmfsOPFSAccessHandles.get(accessID);
try {
await accessHandle.truncate(size);
Expand All @@ -388,7 +403,10 @@ mergeInto(LibraryManager.library, {
},

_wasmfs_opfs_set_size_file__deps: ['$wasmfsOPFSFileHandles'],
_wasmfs_opfs_set_size_file: async function(ctx, fileID, size, errPtr) {
_wasmfs_opfs_set_size_file: async function(ctx, fileID,
{{{ defineI64Param('size') }}},
errPtr) {
{{{ receiveI64ParamAsDouble('size') }}};
let fileHandle = wasmfsOPFSFileHandles.get(fileID);
try {
let writable = await fileHandle.createWritable({keepExistingData: true});
Expand Down
6 changes: 3 additions & 3 deletions system/lib/wasmfs/backends/memory_backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ std::vector<Directory::Entry> MemoryDirectory::getEntries() {
return result;
}

bool MemoryDirectory::insertMove(const std::string& name,
std::shared_ptr<File> file) {
int MemoryDirectory::insertMove(const std::string& name,
std::shared_ptr<File> file) {
auto& oldEntries =
std::static_pointer_cast<MemoryDirectory>(file->locked().getParent())
->entries;
Expand All @@ -76,7 +76,7 @@ bool MemoryDirectory::insertMove(const std::string& name,
}
removeChild(name);
insertChild(name, file);
return true;
return 0;
}

std::string MemoryDirectory::getName(std::shared_ptr<File> file) {
Expand Down
9 changes: 4 additions & 5 deletions system/lib/wasmfs/backends/node_backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class NodeFile : public DataFile {
: DataFile(mode, backend), state(path) {}

private:
size_t getSize() override {
off_t getSize() override {
// TODO: This should really be using a 64-bit file size type.
uint32_t size;
if (state.isOpen()) {
Expand All @@ -151,10 +151,10 @@ class NodeFile : public DataFile {
return 0;
}
}
return size_t(size);
return off_t(size);
}

int setSize(size_t size) override {
int setSize(off_t size) override {
WASMFS_UNREACHABLE("TODO: implement NodeFile::setSize");
}

Expand Down Expand Up @@ -256,8 +256,7 @@ class NodeDirectory : public Directory {
abort();
}

bool insertMove(const std::string& name,
std::shared_ptr<File> file) override {
int insertMove(const std::string& name, std::shared_ptr<File> file) override {
// TODO
abort();
}
Expand Down
34 changes: 17 additions & 17 deletions system/lib/wasmfs/backends/opfs_backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ void _wasmfs_opfs_insert_directory(em_proxying_ctx* ctx,
void _wasmfs_opfs_move(em_proxying_ctx* ctx,
int file_id,
int new_dir_id,
const char* name);
const char* name,
int* err);

void _wasmfs_opfs_remove_child(em_proxying_ctx* ctx,
int dir_id,
Expand Down Expand Up @@ -88,23 +89,22 @@ int _wasmfs_opfs_write_access(int access_id,
// Get the size via an AccessHandle.
void _wasmfs_opfs_get_size_access(em_proxying_ctx* ctx,
int access_id,
uint32_t* size);
off_t* size);

// TODO: return 64-byte off_t.
uint32_t _wasmfs_opfs_get_size_blob(int blob_id);

// Get the size of a file handle via a File Blob.
void _wasmfs_opfs_get_size_file(em_proxying_ctx* ctx,
int file_id,
uint32_t* size);
void _wasmfs_opfs_get_size_file(em_proxying_ctx* ctx, int file_id, off_t* size);

void _wasmfs_opfs_set_size_access(em_proxying_ctx* ctx,
int access_id,
uint32_t size,
off_t size,
int* err);

void _wasmfs_opfs_set_size_file(em_proxying_ctx* ctx,
int file_id,
uint32_t size,
off_t size,
int* err);

void _wasmfs_opfs_flush_access(em_proxying_ctx* ctx, int access_id, int* err);
Expand Down Expand Up @@ -222,9 +222,8 @@ class OPFSFile : public DataFile {
}

private:
size_t getSize() override {
// TODO: 64-bit sizes.
uint32_t size;
off_t getSize() override {
off_t size;
switch (state.getKind()) {
case OpenState::None:
proxy([&](auto ctx) {
Expand All @@ -242,10 +241,10 @@ class OPFSFile : public DataFile {
default:
WASMFS_UNREACHABLE("Unexpected open state");
}
return size_t(size);
return size;
}

int setSize(size_t size) override {
int setSize(off_t size) override {
int err = 0;
switch (state.getKind()) {
case OpenState::Access:
Expand All @@ -259,6 +258,8 @@ class OPFSFile : public DataFile {
// become invalidated and refreshing it while ensuring other in-flight
// operations on the same file do not observe the invalidated blob would
// be extremely complicated.
// TODO: Can we assume there are no other in-flight operations on this
// file and do something better here?
return -EIO;
case OpenState::None: {
proxy([&](auto ctx) {
Expand Down Expand Up @@ -401,14 +402,13 @@ class OPFSDirectory : public Directory {
return nullptr;
}

bool insertMove(const std::string& name,
std::shared_ptr<File> file) override {
int insertMove(const std::string& name, std::shared_ptr<File> file) override {
auto old_file = std::static_pointer_cast<OPFSFile>(file);
int err = 0;
proxy([&](auto ctx) {
_wasmfs_opfs_move(ctx.ctx, old_file->fileID, dirID, name.c_str());
_wasmfs_opfs_move(ctx.ctx, old_file->fileID, dirID, name.c_str(), &err);
});
// TODO: Handle errors.
return true;
return err;
}

bool removeChild(const std::string& name) override {
Expand Down
6 changes: 3 additions & 3 deletions system/lib/wasmfs/backends/proxied_file_backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ class ProxiedFile : public DataFile {

// Querying the size of the Proxied File returns the size of the underlying
// file given by the proxying mechanism.
size_t getSize() override {
size_t result;
off_t getSize() override {
off_t result;
proxy([&]() { result = baseFile->locked().getSize(); });
return result;
}

int setSize(size_t size) override {
int setSize(off_t size) override {
WASMFS_UNREACHABLE("TODO: ProxiedFS setSize");
}

Expand Down
12 changes: 6 additions & 6 deletions system/lib/wasmfs/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,11 @@ Directory::Handle::insertSymlink(const std::string& name,
// TODO: consider moving this to be `Backend::move` to avoid asymmetry between
// the source and destination directories and/or taking `Directory::Handle`
// arguments to prove that the directories have already been locked.
bool Directory::Handle::insertMove(const std::string& name,
std::shared_ptr<File> file) {
int Directory::Handle::insertMove(const std::string& name,
std::shared_ptr<File> file) {
// Cannot insert into an unlinked directory.
if (!getParent()) {
return false;
return -EPERM;
}

// Look up the file in its old parent's cache.
Expand All @@ -161,8 +161,8 @@ bool Directory::Handle::insertMove(const std::string& name,
// involving the backend.

// Attempt the move.
if (!getDir()->insertMove(name, file)) {
return false;
if (auto err = getDir()->insertMove(name, file)) {
return err;
}

if (oldIt != oldCache.end()) {
Expand All @@ -189,7 +189,7 @@ bool Directory::Handle::insertMove(const std::string& name,
oldParent->locked().setMTime(now);
setMTime(now);

return true;
return 0;
}

bool Directory::Handle::removeChild(const std::string& name) {
Expand Down
32 changes: 17 additions & 15 deletions system/lib/wasmfs/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ class File : public std::enable_shared_from_this<File> {
// A mutex is needed for multiple accesses to the same file.
std::recursive_mutex mutex;

// May be called on files that have not been opened.
virtual size_t getSize() = 0;
// The the size in bytes of a file or return a negative error code. May be
// called on files that have not been opened.
virtual off_t getSize() = 0;

mode_t mode = 0; // User and group mode bits for access permission.

Expand Down Expand Up @@ -144,7 +145,7 @@ class DataFile : public File {
// Sets the size of the file to a specific size. If new space is allocated, it
// should be zero-initialized. May be called on files that have not been
// opened. Returns 0 on success or a negative error code.
virtual int setSize(size_t size) = 0;
virtual int setSize(off_t size) = 0;

// Sync the file data to the underlying persistent storage, if any. Returns 0
// on success or a negative error code.
Expand Down Expand Up @@ -202,10 +203,10 @@ class Directory : public File {
// Move the file represented by `file` from its current directory to this
// directory with the new `name`, possibly overwriting another file that
// already exists with that name. The old directory may be the same as this
// directory. On success, return `true`. Otherwise return `false` without
// changing any underlying state.
virtual bool insertMove(const std::string& name,
std::shared_ptr<File> file) = 0;
// directory. On success return 0 and otherwise return a negative error code
// without changing any underlying state.
virtual int insertMove(const std::string& name,
std::shared_ptr<File> file) = 0;

// Remove the file with the given name, returning `true` on success or if the
// child has already been removed or returning `false` if the child cannot be
Expand Down Expand Up @@ -254,7 +255,7 @@ class Directory : public File {
protected:
// 4096 bytes is the size of a block in ext4.
// This value was also copied from the JS file system.
size_t getSize() override { return 4096; }
off_t getSize() override { return 4096; }
};

class Symlink : public File {
Expand All @@ -270,7 +271,7 @@ class Symlink : public File {
virtual std::string getTarget() const = 0;

protected:
size_t getSize() override { return getTarget().size(); }
off_t getSize() override { return getTarget().size(); }
};

class File::Handle {
Expand All @@ -286,7 +287,7 @@ class File::Handle {
Handle(std::shared_ptr<File> file) : file(file), lock(file->mutex) {}
Handle(std::shared_ptr<File> file, std::defer_lock_t)
: file(file), lock(file->mutex, std::defer_lock) {}
size_t getSize() { return file->getSize(); }
off_t getSize() { return file->getSize(); }
mode_t getMode() { return file->mode; }
void setMode(mode_t mode) {
// The type bits can never be changed (whether something is a file or a
Expand Down Expand Up @@ -325,7 +326,7 @@ class DataFile::Handle : public File::Handle {
return getFile()->write(buf, len, offset);
}

[[nodiscard]] int setSize(size_t size) { return getFile()->setSize(size); }
[[nodiscard]] int setSize(off_t size) { return getFile()->setSize(size); }

// TODO: Design a proper API for flushing files.
[[nodiscard]] int flush() { return getFile()->flush(); }
Expand Down Expand Up @@ -370,10 +371,11 @@ class Directory::Handle : public File::Handle {
// Move the file represented by `file` from its current directory to this
// directory with the new `name`, possibly overwriting another file that
// already exists with that name. The old directory may be the same as this
// directory. On success, return `true`. Otherwise return `false` without
// changing any underlying state. This should only be called from renameat
// with the locks on the old and new parents already held.
bool insertMove(const std::string& name, std::shared_ptr<File> file);
// directory. On success return 0 and otherwise return a negative error code
// without changing any underlying state. This should only be called from
// renameat with the locks on the old and new parents already held.
[[nodiscard]] int insertMove(const std::string& name,
std::shared_ptr<File> file);

// Remove the file with the given name, returning `true` on success or if the
// vhild has already been removed or returning `false` if the child cannot be
Expand Down
4 changes: 2 additions & 2 deletions system/lib/wasmfs/js_impl_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ class JSImplFile : public DataFile {

int flush() override { return 0; }

size_t getSize() override {
off_t getSize() override {
return _wasmfs_jsimpl_get_size(getBackendIndex(), getFileIndex());
}

int setSize(size_t size) override {
int setSize(off_t size) override {
WASMFS_UNREACHABLE("TODO: JSImpl setSize");
}

Expand Down
6 changes: 3 additions & 3 deletions system/lib/wasmfs/memory_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ class MemoryFile : public DataFile {
ssize_t write(const uint8_t* buf, size_t len, off_t offset) override;
ssize_t read(uint8_t* buf, size_t len, off_t offset) override;
int flush() override { return 0; }
size_t getSize() override { return buffer.size(); }
int setSize(size_t size) override {
off_t getSize() override { return buffer.size(); }
int setSize(off_t size) override {
buffer.resize(size);
return 0;
}
Expand Down Expand Up @@ -86,7 +86,7 @@ class MemoryDirectory : public Directory {
return child;
}

bool insertMove(const std::string& name, std::shared_ptr<File> file) override;
int insertMove(const std::string& name, std::shared_ptr<File> file) override;

size_t getNumEntries() override { return entries.size(); }
std::vector<Directory::Entry> getEntries() override;
Expand Down
Loading