[vfs] Plumb a hash table around to track koids of tokens
This is a reland of 0f31d965b40a34d3da7447c6494fe4b641793dd1, now that
https://fuchsia-review.googlesource.com/c/garnet/+/248677 has landed
and all clients of libfs have correct header dependencies.
Original change's description:
> [vfs] Plumb a hash table around to track koids of tokens
>
> Rather than rely on set/get_cookie
>
> Test: fs unit tests
> Change-Id: I03d6b5de4eae90e8bbcef09958fb983b84e86e64
Test: fs unit tests
Change-Id: If1632111373282bd82be113e5760c255fc3ef0f0
diff --git a/system/ulib/fs/include/fs/mount_channel.h b/system/ulib/fs/include/fs/mount_channel.h
new file mode 100644
index 0000000..65a7551
--- /dev/null
+++ b/system/ulib/fs/include/fs/mount_channel.h
@@ -0,0 +1,51 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#pragma once
+
+#include <zircon/types.h>
+
+#ifdef __Fuchsia__
+#include <lib/zx/channel.h>
+#include <fs/client.h>
+#endif // __Fuchsia__
+
+#include <utility>
+
+namespace fs {
+
+#ifdef __Fuchsia__
+
+// MountChannel functions exactly the same as a channel, except that it
+// intentionally destructs by sending a clean "shutdown" signal to the
+// underlying filesystem. Up until the point that a remote handle is
+// attached to a vnode, this wrapper guarantees not only that the
+// underlying handle gets closed on error, but also that the sub-filesystem
+// is released (which cleans up the underlying connection to the block
+// device).
+class MountChannel {
+public:
+ constexpr MountChannel() = default;
+ explicit MountChannel(zx_handle_t handle)
+ : channel_(handle) {}
+ explicit MountChannel(zx::channel channel)
+ : channel_(std::move(channel)) {}
+ MountChannel(MountChannel&& other)
+ : channel_(std::move(other.channel_)) {}
+
+ zx::channel TakeChannel() { return std::move(channel_); }
+
+ ~MountChannel() {
+ if (channel_.is_valid()) {
+ vfs_unmount_handle(channel_.release(), 0);
+ }
+ }
+
+private:
+ zx::channel channel_;
+};
+
+#endif // __Fuchsia__
+
+} // namespace fs
diff --git a/system/ulib/fs/include/fs/vfs.h b/system/ulib/fs/include/fs/vfs.h
index 26898ba..3e4e271 100644
--- a/system/ulib/fs/include/fs/vfs.h
+++ b/system/ulib/fs/include/fs/vfs.h
@@ -22,8 +22,11 @@
#include <lib/zx/channel.h>
#include <lib/zx/event.h>
#include <lib/zx/vmo.h>
+#include <fbl/intrusive_hash_table.h>
#include <fbl/mutex.h>
#include <fs/client.h>
+#include <fs/mount_channel.h>
+#include <fs/vnode.h>
#endif // __Fuchsia__
#include <fbl/function.h>
@@ -69,39 +72,6 @@
void* p;
} vdircookie_t;
-#ifdef __Fuchsia__
-
-// MountChannel functions exactly the same as a channel, except that it
-// intentionally destructs by sending a clean "shutdown" signal to the
-// underlying filesystem. Up until the point that a remote handle is
-// attached to a vnode, this wrapper guarantees not only that the
-// underlying handle gets closed on error, but also that the sub-filesystem
-// is released (which cleans up the underlying connection to the block
-// device).
-class MountChannel {
-public:
- constexpr MountChannel() = default;
- explicit MountChannel(zx_handle_t handle)
- : channel_(handle) {}
- explicit MountChannel(zx::channel channel)
- : channel_(std::move(channel)) {}
- MountChannel(MountChannel&& other)
- : channel_(std::move(other.channel_)) {}
-
- zx::channel TakeChannel() { return std::move(channel_); }
-
- ~MountChannel() {
- if (channel_.is_valid()) {
- vfs_unmount_handle(channel_.release(), 0);
- }
- }
-
-private:
- zx::channel channel_;
-};
-
-#endif // __Fuchsia__
-
// The Vfs object contains global per-filesystem state, which
// may be valid across a collection of Vnodes.
//
@@ -216,6 +186,8 @@
zx_status_t UninstallRemoteLocked(fbl::RefPtr<Vnode> vn,
zx::channel* h) FS_TA_REQUIRES(vfs_lock_);
+ fbl::HashTable<zx_koid_t, std::unique_ptr<VnodeToken>> vnode_tokens_;
+
// Non-intrusive node in linked list of vnodes acting as mount points
class MountNode final : public fbl::DoublyLinkedListable<fbl::unique_ptr<MountNode>> {
public:
diff --git a/system/ulib/fs/include/fs/vnode.h b/system/ulib/fs/include/fs/vnode.h
index 20d7e00..299a2b1 100644
--- a/system/ulib/fs/include/fs/vnode.h
+++ b/system/ulib/fs/include/fs/vnode.h
@@ -10,12 +10,14 @@
#include <sys/types.h>
#include <fbl/function.h>
+#include <fbl/intrusive_single_list.h>
#include <fbl/intrusive_double_list.h>
#include <fbl/macros.h>
#include <fbl/ref_counted_internal.h>
#include <fbl/ref_ptr.h>
+#include <fbl/string_piece.h>
+#include <fs/mount_channel.h>
#include <fs/ref_counted.h>
-#include <fs/vfs.h>
#include <lib/fdio/io.h>
#include <lib/fdio/vfs.h>
#include <zircon/assert.h>
@@ -33,6 +35,9 @@
namespace fs {
+class Vfs;
+typedef struct vdircookie vdircookie_t;
+
inline bool vfs_valid_name(fbl::StringPiece name) {
return name.length() <= NAME_MAX &&
memchr(name.data(), '/', name.length()) == nullptr &&
@@ -243,4 +248,24 @@
const size_t len_;
};
+// Helper class to track outstanding operations associated to a
+// particular Vnode.
+class VnodeToken : public fbl::SinglyLinkedListable<std::unique_ptr<VnodeToken>> {
+public:
+ VnodeToken(zx_koid_t koid, fbl::RefPtr<Vnode> vnode) :
+ koid_(koid), vnode_(std::move(vnode)) {
+ }
+
+ zx_koid_t get_koid() const { return koid_; }
+ fbl::RefPtr<Vnode> get_vnode() const { return vnode_; }
+
+ // Trait implementation for fbl::HashTable
+ zx_koid_t GetKey() const { return koid_; }
+ static size_t GetHash(zx_koid_t koid) { return koid; }
+
+private:
+ zx_koid_t koid_;
+ fbl::RefPtr<Vnode> vnode_;
+};
+
} // namespace fs
diff --git a/system/ulib/fs/lazy-dir.cpp b/system/ulib/fs/lazy-dir.cpp
index a5846e6..1b5d750 100644
--- a/system/ulib/fs/lazy-dir.cpp
+++ b/system/ulib/fs/lazy-dir.cpp
@@ -3,6 +3,9 @@
// found in the LICENSE file.
#include <fs/lazy-dir.h>
+
+#include <fs/vfs.h>
+#include <fs/vnode.h>
#include <fuchsia/io/c/fidl.h>
namespace fs {
diff --git a/system/ulib/fs/pseudo-file.cpp b/system/ulib/fs/pseudo-file.cpp
index 3d88090..741e938 100644
--- a/system/ulib/fs/pseudo-file.cpp
+++ b/system/ulib/fs/pseudo-file.cpp
@@ -6,6 +6,9 @@
#include <utility>
+#include <fs/vfs.h>
+#include <zircon/device/vfs.h>
+
namespace fs {
PseudoFile::PseudoFile(ReadHandler read_handler, WriteHandler write_handler)
diff --git a/system/ulib/fs/service.cpp b/system/ulib/fs/service.cpp
index 6135311..aca03d9 100644
--- a/system/ulib/fs/service.cpp
+++ b/system/ulib/fs/service.cpp
@@ -6,6 +6,8 @@
#include <utility>
+#include <zircon/device/vfs.h>
+
namespace fs {
Service::Service(Connector connector)
diff --git a/system/ulib/fs/vfs.cpp b/system/ulib/fs/vfs.cpp
index d3de1b1..c066081 100644
--- a/system/ulib/fs/vfs.cpp
+++ b/system/ulib/fs/vfs.cpp
@@ -244,6 +244,14 @@
#define TOKEN_RIGHTS (ZX_RIGHTS_BASIC)
+namespace {
+zx_koid_t GetTokenKoid(const zx::event& token) {
+ zx_info_handle_basic_t info = {};
+ token.get_info(ZX_INFO_HANDLE_BASIC, &info, sizeof(info), nullptr, nullptr);
+ return info.koid;
+}
+} // namespace
+
void Vfs::TokenDiscard(zx::event ios_token) {
fbl::AutoLock lock(&vfs_lock_);
if (ios_token) {
@@ -256,13 +264,12 @@
//
// By cleared the token cookie, any remaining handles to the event will
// be ignored by the filesystem server.
- ios_token.set_cookie(*zx::process::self(), 0);
+ auto rename_request = vnode_tokens_.erase(GetTokenKoid(ios_token));
}
}
zx_status_t Vfs::VnodeToToken(fbl::RefPtr<Vnode> vn, zx::event* ios_token,
zx::event* out) {
- uint64_t vnode_cookie = reinterpret_cast<uint64_t>(vn.get());
zx_status_t r;
fbl::AutoLock lock(&vfs_lock_);
@@ -280,28 +287,22 @@
return r;
} else if ((r = new_ios_token.duplicate(TOKEN_RIGHTS, &new_token) != ZX_OK)) {
return r;
- } else if ((r = new_ios_token.set_cookie(*zx::process::self(), vnode_cookie)) != ZX_OK) {
- return r;
}
+ auto koid = GetTokenKoid(new_ios_token);
+ vnode_tokens_.insert(std::make_unique<VnodeToken>(koid, std::move(vn)));
*ios_token = std::move(new_ios_token);
*out = std::move(new_token);
return ZX_OK;
}
zx_status_t Vfs::TokenToVnode(zx::event token, fbl::RefPtr<Vnode>* out) {
- uint64_t vcookie;
- zx_status_t r;
- if ((r = token.get_cookie(*zx::process::self(), &vcookie)) < 0) {
+ const auto& vnode_token = vnode_tokens_.find(GetTokenKoid(token));
+ if (vnode_token == vnode_tokens_.end()) {
// TODO(smklein): Return a more specific error code for "token not from this server"
return ZX_ERR_INVALID_ARGS;
}
- if (vcookie == 0) {
- // Client closed the channel associated with the token
- return ZX_ERR_INVALID_ARGS;
- }
-
- *out = fbl::RefPtr<fs::Vnode>(reinterpret_cast<fs::Vnode*>(vcookie));
+ *out = vnode_token->get_vnode();
return ZX_OK;
}
diff --git a/system/ulib/fs/vmo-file.cpp b/system/ulib/fs/vmo-file.cpp
index 8c8acc1..33dae23 100644
--- a/system/ulib/fs/vmo-file.cpp
+++ b/system/ulib/fs/vmo-file.cpp
@@ -9,8 +9,10 @@
#include <fbl/algorithm.h>
#include <fbl/auto_lock.h>
+#include <fs/vfs.h>
#include <fuchsia/io/c/fidl.h>
#include <zircon/assert.h>
+#include <zircon/device/vfs.h>
#include <zircon/syscalls.h>
namespace fs {
diff --git a/system/utest/fs-vnode/lazy-dir-tests.cpp b/system/utest/fs-vnode/lazy-dir-tests.cpp
index bcf8034..105d79e 100644
--- a/system/utest/fs-vnode/lazy-dir-tests.cpp
+++ b/system/utest/fs-vnode/lazy-dir-tests.cpp
@@ -5,6 +5,7 @@
#include <fbl/vector.h>
#include <fs/lazy-dir.h>
#include <fs/pseudo-file.h>
+#include <fs/vfs.h>
#include <unittest/unittest.h>
#include <utility>
diff --git a/system/utest/fs-vnode/pseudo-file-tests.cpp b/system/utest/fs-vnode/pseudo-file-tests.cpp
index 6157488..f4a24f4 100644
--- a/system/utest/fs-vnode/pseudo-file-tests.cpp
+++ b/system/utest/fs-vnode/pseudo-file-tests.cpp
@@ -7,6 +7,7 @@
#include <fbl/vector.h>
#include <initializer_list>
#include <unittest/unittest.h>
+#include <zircon/device/vfs.h>
#define EXPECT_FSTR_EQ(expected, actual) \
EXPECT_BYTES_EQ(reinterpret_cast<const uint8_t*>(expected.c_str()), \
diff --git a/system/utest/fs-vnode/vmo-file-tests.cpp b/system/utest/fs-vnode/vmo-file-tests.cpp
index 16cd497..b2c11a7 100644
--- a/system/utest/fs-vnode/vmo-file-tests.cpp
+++ b/system/utest/fs-vnode/vmo-file-tests.cpp
@@ -7,6 +7,7 @@
#include <limits.h>
#include <fuchsia/io/c/fidl.h>
+#include <zircon/device/vfs.h>
#include <zircon/syscalls.h>
#include <zircon/syscalls/object.h>
