[fxfs] Convert FxNode to a trait.
Also fixes a bug where the root node was never added to the cache on
initializing a volume.
Change-Id: I36d74f0309e41efaebd5feb96981899d68a23874
Tested: unit.
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/513727
Commit-Queue: James Sullivan <jfsulliv@google.com>
Reviewed-by: Chris Suter <csuter@google.com>
diff --git a/src/storage/fxfs/src/server.rs b/src/storage/fxfs/src/server.rs
index 80f86ad..343429d 100644
--- a/src/storage/fxfs/src/server.rs
+++ b/src/storage/fxfs/src/server.rs
@@ -52,7 +52,8 @@
.open_or_create_volume(volume_name)
.await
.expect("Open or create volume failed"),
- );
+ )
+ .await;
Ok(Self { fs, volume })
}
diff --git a/src/storage/fxfs/src/server/directory.rs b/src/storage/fxfs/src/server/directory.rs
index e367d99..fa648cb 100644
--- a/src/storage/fxfs/src/server/directory.rs
+++ b/src/storage/fxfs/src/server/directory.rs
@@ -30,6 +30,7 @@
dirents_sink::{self, Sink},
entry::{DirectoryEntry, EntryInfo},
entry_container::{AsyncGetEntry, Directory, MutableDirectory},
+ mutable::connection::io1::MutableConnection,
traversal_position::TraversalPosition,
},
execution_scope::ExecutionScope,
@@ -53,19 +54,17 @@
flags: u32,
mode: u32,
mut path: Path,
- ) -> Result<FxNode, Error> {
+ ) -> Result<Arc<dyn FxNode>, Error> {
// TODO(csuter): There are races to fix here where a direectory or file could be removed
// whilst a lookup is taking place. The transaction locks only protect against multiple
// writers, but the readers don't have any locks, so there needs to be checks that nodes
// still exist at some point.
- let mut current_node = FxNode::Dir(self.clone());
let fs = self.volume.store().filesystem();
+ let mut current_node = self.clone() as Arc<dyn FxNode>;
while !path.is_empty() {
let last_segment = path.is_single_component();
- let current_dir = match current_node {
- FxNode::Dir(dir) => dir.clone(),
- FxNode::File(_file) => bail!(FxfsError::NotDir),
- };
+ let current_dir =
+ current_node.into_any().downcast::<FxDirectory>().map_err(|_| FxfsError::NotDir)?;
let name = path.next().unwrap();
// Create the transaction here if we might need to create the object so that we have a
// lock in place.
@@ -126,25 +125,34 @@
dir: &Arc<FxDirectory>,
name: &str,
mode: u32,
- ) -> Result<FxNode, Error> {
+ ) -> Result<Arc<dyn FxNode>, Error> {
let (object_id, node) = if mode & MODE_TYPE_DIRECTORY != 0 {
let dir = dir.directory.create_child_dir(&mut transaction, name).await?;
let object_id = dir.object_id();
- let node =
- FxNode::Dir(Arc::new(FxDirectory { volume: self.volume.clone(), directory: dir }));
+ let node = Arc::new(FxDirectory { volume: self.volume.clone(), directory: dir })
+ as Arc<dyn FxNode>;
(object_id, node)
} else {
let handle = dir.directory.create_child_file(&mut transaction, name).await?;
let object_id = handle.object_id();
- let node = FxNode::File(Arc::new(FxFile::new(handle)));
+ let node = Arc::new(FxFile::new(handle)) as Arc<dyn FxNode>;
(object_id, node)
};
- transaction.commit().await;
- self.volume.add_node(object_id, node.downgrade()).await;
+ self.volume.store().filesystem().commit_transaction(transaction).await;
+ self.volume.add_node(object_id, Arc::downgrade(&node)).await;
Ok(node)
}
}
+impl FxNode for FxDirectory {
+ fn object_id(&self) -> u64 {
+ self.directory.object_id()
+ }
+ fn into_any(self: Arc<Self>) -> Arc<dyn Any + Send + Sync + 'static> {
+ self
+ }
+}
+
#[async_trait]
impl MutableDirectory for FxDirectory {
async fn link(&self, _name: String, _entry: Arc<dyn DirectoryEntry>) -> Result<(), Status> {
@@ -187,19 +195,24 @@
) {
let cloned_scope = scope.clone();
scope.spawn(async move {
+ // TODO(jfsulliv): Factor this out into a visitor-pattern style method for FxNode, e.g.
+ // FxNode::visit(FileFn, DirFn).
match self.lookup(flags, mode, path).await {
Err(e) => send_on_open_with_error(flags, server_end, map_to_status(e)),
- Ok(FxNode::Dir(dir)) => {
- vfs::directory::mutable::connection::io1::MutableConnection::create_connection(
- cloned_scope,
- OpenDirectory::new(dir),
- flags,
- mode,
- server_end,
- );
- }
- Ok(FxNode::File(file)) => {
- file.clone().open(cloned_scope, flags, mode, Path::empty(), server_end);
+ Ok(node) => {
+ if let Ok(dir) = node.clone().into_any().downcast::<FxDirectory>() {
+ MutableConnection::create_connection(
+ cloned_scope,
+ OpenDirectory::new(dir),
+ flags,
+ mode,
+ server_end,
+ );
+ } else if let Ok(file) = node.into_any().downcast::<FxFile>() {
+ file.clone().open(cloned_scope, flags, mode, Path::empty(), server_end);
+ } else {
+ panic!("Unknown node type")
+ }
}
};
});
@@ -285,7 +298,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
{
let (dir_proxy, dir_server_end) = fidl::endpoints::create_proxy::<DirectoryMarker>()
.expect("Create proxy to succeed");
@@ -331,7 +344,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -357,12 +370,12 @@
let (filesystem, vol) = if i == 0 {
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
(filesystem, vol)
} else {
let filesystem = FxFilesystem::open(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.volume("vol").await?).await;
(filesystem, vol)
};
let dir = vol.root().clone();
@@ -394,7 +407,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -428,7 +441,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -463,7 +476,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -508,7 +521,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
diff --git a/src/storage/fxfs/src/server/file.rs b/src/storage/fxfs/src/server/file.rs
index 15e0947..c492eb3 100644
--- a/src/storage/fxfs/src/server/file.rs
+++ b/src/storage/fxfs/src/server/file.rs
@@ -6,14 +6,14 @@
crate::{
object_handle::{ObjectHandle, ObjectHandleExt},
object_store::StoreObjectHandle,
- server::errors::map_to_status,
+ server::{errors::map_to_status, node::FxNode},
},
async_trait::async_trait,
fidl::endpoints::ServerEnd,
fidl_fuchsia_io::{self as fio, NodeAttributes, NodeMarker},
fidl_fuchsia_mem::Buffer,
fuchsia_zircon::Status,
- std::sync::Arc,
+ std::{any::Any, sync::Arc},
vfs::{
common::send_on_open_with_error,
directory::entry::{DirectoryEntry, EntryInfo},
@@ -37,6 +37,15 @@
}
}
+impl FxNode for FxFile {
+ fn object_id(&self) -> u64 {
+ self.handle.object_id()
+ }
+ fn into_any(self: Arc<Self>) -> Arc<dyn Any + Send + Sync + 'static> {
+ self
+ }
+}
+
impl DirectoryEntry for FxFile {
fn open(
self: Arc<Self>,
@@ -164,7 +173,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -199,7 +208,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -245,12 +254,12 @@
let (filesystem, vol) = if i == 0 {
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
(filesystem, vol)
} else {
let filesystem = FxFilesystem::open(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.volume("vol").await?).await;
(filesystem, vol)
};
let dir = vol.root().clone();
@@ -294,7 +303,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -342,7 +351,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -410,7 +419,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -476,7 +485,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
@@ -548,7 +557,7 @@
let device = Arc::new(FakeDevice::new(2048, 512));
let filesystem = FxFilesystem::new_empty(device.clone()).await?;
let volume_directory = volume_directory(&filesystem).await?;
- let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?);
+ let vol = FxVolumeAndRoot::new(volume_directory.new_volume("vol").await?).await;
let dir = vol.root().clone();
let (dir_proxy, dir_server_end) =
diff --git a/src/storage/fxfs/src/server/node.rs b/src/storage/fxfs/src/server/node.rs
index 1657836..9b4c1f4 100644
--- a/src/storage/fxfs/src/server/node.rs
+++ b/src/storage/fxfs/src/server/node.rs
@@ -2,45 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-use {
- crate::server::{directory::FxDirectory, file::FxFile},
- std::sync::{Arc, Weak},
-};
+use {std::any::Any, std::sync::Arc};
-// TODO(jfsulliv): Should FxNode be a trait object instead of an enum, so that we can have common
-// functionality shared between directories and files?
-
-#[derive(Clone)]
-// FxNode is a strong reference to a node in the filesystem hierarchy (either a file or directory).
-pub enum FxNode {
- Dir(Arc<FxDirectory>),
- File(Arc<FxFile>),
-}
-
-impl FxNode {
- /// Creates a weak copy of self.
- pub fn downgrade(&self) -> WeakFxNode {
- match self {
- FxNode::Dir(dir) => WeakFxNode::Dir(Arc::downgrade(dir)),
- FxNode::File(file) => WeakFxNode::File(Arc::downgrade(file)),
- }
- }
-}
-
-#[derive(Clone)]
-// WeakFxNode is a weak version of FxNode.
-pub enum WeakFxNode {
- Dir(Weak<FxDirectory>),
- File(Weak<FxFile>),
-}
-
-impl WeakFxNode {
- /// Attempts to create a strong copy of self. If the underlying node has been deleted, returns
- /// None.
- pub fn upgrade(&self) -> Option<FxNode> {
- match self {
- WeakFxNode::Dir(dir) => Weak::upgrade(dir).map(|dir| FxNode::Dir(dir)),
- WeakFxNode::File(file) => Weak::upgrade(file).map(|file| FxNode::File(file)),
- }
- }
+// FxNode is a node in the filesystem hierarchy (either a file or directory).
+pub trait FxNode: Any + Send + Sync + 'static {
+ fn object_id(&self) -> u64;
+ fn into_any(self: Arc<Self>) -> Arc<dyn Any + Send + Sync + 'static>;
}
diff --git a/src/storage/fxfs/src/server/volume.rs b/src/storage/fxfs/src/server/volume.rs
index 1515eec..bfd34d4 100644
--- a/src/storage/fxfs/src/server/volume.rs
+++ b/src/storage/fxfs/src/server/volume.rs
@@ -6,32 +6,32 @@
crate::{
errors::FxfsError,
object_store::{directory::ObjectDescriptor, HandleOptions, ObjectStore},
- server::{
- directory::FxDirectory,
- file::FxFile,
- node::{FxNode, WeakFxNode},
- },
+ server::{directory::FxDirectory, file::FxFile, node::FxNode},
volume::Volume,
},
anyhow::{anyhow, bail, Error},
fuchsia_zircon::Status,
- std::{any::Any, collections::HashMap, sync::Arc},
+ std::{
+ any::Any,
+ collections::HashMap,
+ sync::{Arc, Weak},
+ },
vfs::{
filesystem::{Filesystem, FilesystemRename},
path::Path,
},
};
-struct NodeCache(HashMap<u64, WeakFxNode>);
+struct NodeCache(HashMap<u64, Weak<dyn FxNode>>);
impl NodeCache {
- fn add(&mut self, object_id: u64, node: WeakFxNode) {
+ fn add(&mut self, object_id: u64, node: Weak<dyn FxNode>) {
// This is a programming error, so panic here.
assert!(!self.0.contains_key(&object_id), "Duplicate node for {}", object_id);
self.0.insert(object_id, node);
}
- fn get(&mut self, object_id: u64) -> Result<FxNode, Error> {
+ fn get(&mut self, object_id: u64) -> Result<Arc<dyn FxNode>, Error> {
if let Some(node) = self.0.get(&object_id) {
if let Some(node) = node.upgrade() {
Ok(node)
@@ -62,19 +62,22 @@
&self.store
}
- pub async fn add_node(&self, object_id: u64, node: WeakFxNode) {
+ pub async fn add_node(&self, object_id: u64, node: Weak<dyn FxNode>) {
self.nodes.lock().await.add(object_id, node)
}
- pub async fn open_node(&self, object_id: u64) -> Result<FxNode, Error> {
+ pub async fn open_node(&self, object_id: u64) -> Result<Arc<dyn FxNode>, Error> {
self.nodes.lock().await.get(object_id)
}
+ /// Attempts to open a node in the node cache. If the node wasn't present in the cache, loads
+ /// the object from the object store, installing the returned node into the cache and returns
+ /// the newly created FxNode backed by the loaded object.
pub async fn open_or_load_node(
self: &Arc<Self>,
object_id: u64,
object_descriptor: ObjectDescriptor,
- ) -> Result<FxNode, Error> {
+ ) -> Result<Arc<dyn FxNode>, Error> {
let mut nodes = self.nodes.lock().await;
match nodes.get(object_id) {
Ok(node) => Ok(node),
@@ -83,15 +86,15 @@
ObjectDescriptor::File => {
let file =
self.store.open_object(object_id, HandleOptions::default()).await?;
- FxNode::File(Arc::new(FxFile::new(file)))
+ Arc::new(FxFile::new(file)) as Arc<dyn FxNode>
}
ObjectDescriptor::Directory => {
let directory = self.store.open_directory(object_id).await?;
- FxNode::Dir(Arc::new(FxDirectory::new(self.clone(), directory)))
+ Arc::new(FxDirectory::new(self.clone(), directory)) as Arc<dyn FxNode>
}
_ => bail!(FxfsError::Inconsistent),
};
- nodes.add(object_id, node.downgrade());
+ nodes.add(object_id, Arc::downgrade(&node));
Ok(node)
}
Err(e) => return Err(e),
@@ -115,17 +118,19 @@
pub struct FxVolumeAndRoot {
volume: Arc<FxVolume>,
- root: FxNode,
+ root: Arc<dyn FxNode>,
}
impl FxVolumeAndRoot {
- pub fn new(volume: Volume) -> Self {
+ pub async fn new(volume: Volume) -> Self {
let (store, root_directory) = volume.into();
+ let root_object_id = root_directory.object_id();
let volume = Arc::new(FxVolume {
nodes: futures::lock::Mutex::new(NodeCache(HashMap::new())),
store,
});
- let root = FxNode::Dir(Arc::new(FxDirectory::new(volume.clone(), root_directory)));
+ let root: Arc<dyn FxNode> = Arc::new(FxDirectory::new(volume.clone(), root_directory));
+ volume.add_node(root_object_id, Arc::downgrade(&root)).await;
Self { volume, root }
}
@@ -133,12 +138,8 @@
&self.volume
}
- pub fn root(&self) -> &Arc<FxDirectory> {
- if let FxNode::Dir(dir) = &self.root {
- dir
- } else {
- panic!("Invalid type for root");
- }
+ pub fn root(&self) -> Arc<FxDirectory> {
+ self.root.clone().into_any().downcast::<FxDirectory>().expect("Invalid type for root")
}
pub(super) fn into_volume(self) -> Arc<FxVolume> {
