Improve lookup implementation

Cargo.lock

@@ -418,6 +418,7 @@ dependencies = [
  "libc",
  "serde",
  "tarpc",
+ "thiserror",
  "tokio",
  "tracing",
 ]

gdriver-client/Cargo.toml

@@ -16,6 +16,7 @@ fuser = "0.14.0"
 bimap = "0.6"
 libc = "0.2.152"
 futures = "0.3"
+thiserror = "1.0.56"
 
 [dependencies.gdriver-common]
 path = "../gdriver-common"

gdriver-client/src/filesystem.rs

@@ -1,5 +1,5 @@
 use std::collections::HashMap;
-use std::ffi::OsStr;
+use std::ffi::{OsStr, OsString};
 use std::os::raw::c_int;
 use std::time::Duration;
 
@@ -9,28 +9,38 @@ use fuser::{KernelConfig, ReplyEntry, Request};
 use tracing::*;
 
 use gdriver_common::drive_structure::drive_id::{DriveId, ROOT_ID};
+use gdriver_common::drive_structure::meta::read_metadata_file;
 use gdriver_common::ipc::gdriver_service::{
     errors::GDriverServiceError, GDriverServiceClient, GDriverSettings,
 };
 
+use crate::filesystem::attributes::read_inode_attributes_from_meta_file;
+use crate::filesystem::errors::FilesystemError;
+use crate::prelude::macros::*;
 use crate::prelude::*;
-use crate::{
+use tarpc::context::current as current_context;
-    reply_error_e, reply_error_o, send_request, send_request_handled, send_request_handled2,
-    send_request_handled2_consuming,
-};
 
 mod macros;
 
 //TODO2: Decide if this is a good TTL
 const TTL: Duration = Duration::from_secs(2);
 
+type Inode = u64;
+
+#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+struct FileIdentifier {
+    parent: Inode,
+    name: OsString,
+}
+
 pub struct Filesystem {
     gdriver_client: GDriverServiceClient,
 
-    entry_ids: BiMap<u64, DriveId>,
+    entry_ids: BiMap<Inode, DriveId>,
-    ino_to_file_handles: HashMap<u64, Vec<u64>>,
+    ino_to_file_handles: HashMap<Inode, Vec<u64>>,
     next_ino: u64,
     gdriver_settings: GDriverSettings,
+    entry_name_parent_to_ino: BiMap<FileIdentifier, Inode>,
 }
 
 impl Filesystem {
@@ -41,9 +51,10 @@ impl Filesystem {
             ino_to_file_handles: HashMap::new(),
             next_ino: 222,
             gdriver_settings: GDriverSettings::default(),
+            entry_name_parent_to_ino: BiMap::new(),
         }
     }
-    fn generate_ino(&mut self) -> u64 {
+    fn generate_ino(&mut self) -> Inode {
         let ino = self.next_ino;
         self.next_ino += 1;
         ino
@@ -52,24 +63,24 @@ impl Filesystem {
 
 //region DriveFilesystem ino_to_id
 impl Filesystem {
-    fn get_id_from_ino(&self, ino: u64) -> Option<&DriveId> {
+    fn get_id_from_ino(&self, ino: Inode) -> Option<&DriveId> {
         self.entry_ids.get_by_left(&ino)
     }
-    fn get_ino_from_id(&mut self, id: DriveId) -> u64 {
+    fn get_ino_from_id(&mut self, id: DriveId) -> Inode {
         let x = self.entry_ids.get_by_right(&id);
         if let Some(ino) = x {
             return *ino;
         }
         self.add_id(id)
     }
-    fn remove_id(&mut self, id: DriveId) -> Result<u64> {
+    fn remove_id(&mut self, id: DriveId) -> Result<Inode> {
         if let Some((ino, _)) = self.entry_ids.remove_by_right(&id) {
             Ok(ino)
         } else {
             Err(Box::from(anyhow!("could not find id {}", id)))
         }
     }
-    fn add_id(&mut self, id: DriveId) -> u64 {
+    fn add_id(&mut self, id: DriveId) -> Inode {
         let ino = self.generate_ino();
         trace!("adding new ino for drive id: {} => {}", id, ino);
         self.entry_ids.insert(ino, id);
@@ -79,63 +90,131 @@ impl Filesystem {
 
 //endregion
 mod attributes;
+
 impl fuser::Filesystem for Filesystem {
     //region init
     fn init(&mut self, _req: &Request<'_>, _config: &mut KernelConfig) -> StdResult<(), c_int> {
         self.entry_ids.insert(1, ROOT_ID.clone());
-        self.gdriver_settings =
+        self.gdriver_settings = send_request!(self.gdriver_client.get_settings(current_context()))
-            send_request!(self.gdriver_client.get_settings(tarpc::context::current()))
+            .map_err(|e| {
-                .map_err(|e| {
+                error!("Got a connection error while fetching settings: {e}");
-                    error!("Got a connection error while fetching settings: {e}");
+                libc::ECONNREFUSED
-                    libc::ECONNREFUSED
+            })?
-                })?
+            .map_err(|e| {
-                .map_err(|e| {
+                error!("Got an error while fetching settings: {e}");
-                    error!("Got an error while fetching settings: {e}");
+                trace!("details: {e:?}");
-                    trace!("details: {e:?}");
+                libc::EBADMSG
-                    libc::EBADMSG
+            })?;
-                })?;
 
         Ok(())
     }
     //endregion
     //region lookup
     fn lookup(&mut self, _req: &Request<'_>, parent: u64, name: &OsStr, reply: ReplyEntry) {
-        let parent_id = self.entry_ids.get_by_left(&parent);
+        let metadata = utils::lookup::lookup(self, parent, name.to_os_string());
-        let parent_id = reply_error_o!(
+        match metadata {
-            parent_id,
+            Ok(metadata) => {
-            reply,
+                reply.entry(&TTL, &metadata.into(), 0);
-            libc::ENOENT,
+            }
-            "Failed to find drive_id for parent ino: {}",
+            Err(e) => {
-            parent
+                error!("Got an error during lookup: {e:?}");
-        );
+                match e {
-        trace!(
+                    FilesystemError::Rpc(_) => reply.error(libc::EREMOTEIO),
-            "looking for child of parent:{} with name: {:?}",
+                    FilesystemError::IO(_) => reply.error(libc::EIO),
-            parent_id,
+                    FilesystemError::Service(_) | FilesystemError::NotFound => {
-            name
+                        reply.error(libc::ENOENT)
-        );
+                    }
+                    FilesystemError::Other(e) => {
+                        dbg!(e);
+                        todo!("Handle other errors and decide what error code should be used here")
+                    }
+                }
+            }
+        }
+    }
+    //endregion
+}
+mod errors {
+    use gdriver_common::ipc::gdriver_service::errors::GDriverServiceError;
+    use std::error::Error;
+    use tarpc::client::RpcError;
 
-        let id = reply_error_e!(
+    #[derive(Debug, thiserror::Error)]
-            send_request_handled2!(
+    pub enum FilesystemError {
-                self.gdriver_client.get_file_by_name(
+        #[error("Error while executing RPC: {0}")]
-                    tarpc::context::current(),
+        Rpc(#[from] RpcError),
-                    name.to_os_string(),
+        #[error("Could not find entity specified")]
-                    parent_id.clone()
+        NotFound,
-                ),
+        #[error("IO Error")]
-                reply
+        IO(#[source] Box<dyn Error>),
-            ),
+        #[error("Service returned Error: {0}")]
-            reply,
+        Service(#[from] GDriverServiceError),
-            libc::ENOENT,
+        #[error("Some other error occurred: {0}")]
-            "Could not find file by name '{:?}' under parent: {}",
+        Other(#[source] Box<dyn Error>),
-            name,
+    }
-            parent_id
+}
-        );
+mod utils {
-        send_request_handled2_consuming!(
+    use super::*;
-            self.gdriver_client
+    pub mod lookup {
-                .get_metadata_for_file(tarpc::context::current(), id),
+        use super::*;
-            reply,
+        use crate::filesystem::attributes::InodeAttributes;
-            parent
+        use crate::filesystem::errors::FilesystemError;
-        );
+        use futures::TryFutureExt;
-
+        use gdriver_common::ipc::gdriver_service::errors::GetFileByPathError;
-        todo!()
+
+        pub fn lookup(
+            fs: &mut Filesystem,
+            parent: Inode,
+            name: OsString,
+        ) -> StdResult<InodeAttributes, FilesystemError> {
+            let id: DriveId;
+            let ino: Inode;
+
+            let name = name.to_os_string();
+            let ino_opt = fs.entry_name_parent_to_ino.get_by_left(&FileIdentifier {
+                parent,
+                name: name.clone(),
+            });
+
+            match ino_opt {
+                None => {
+                    //we don't know this name with this parent already, so we have to look it up
+                    let parent_id = fs
+                        .entry_ids
+                        .get_by_left(&parent)
+                        .ok_or(FilesystemError::NotFound)?;
+                    trace!(
+                        "looking for child of parent:{} with name: {:?}",
+                        parent_id,
+                        name
+                    );
+                    id = send_request!(fs.gdriver_client.get_file_by_name(
+                        current_context(),
+                        name.to_os_string(),
+                        parent_id.clone()
+                    ))?
+                    .map_err(GDriverServiceError::from)?;
+
+                    ino = fs.add_id(id.clone());
+                }
+                Some(i) => {
+                    ino = *i;
+                    id = fs
+                        .get_id_from_ino(*i)
+                        .ok_or(FilesystemError::NotFound)?
+                        .clone();
+                }
+            }
+            let open_file_handles =
+                fs.ino_to_file_handles.get(&ino).map(Vec::len).unwrap_or(0) as u64;
+            send_request!(fs
+                .gdriver_client
+                .get_metadata_for_file(current_context(), id.clone()))?
+            .map_err(GDriverServiceError::from)?;
+            let meta_path = fs.gdriver_settings.get_metadata_file_path(&id);
+            let metadata = read_inode_attributes_from_meta_file(&meta_path, ino, open_file_handles)
+                .map_err(FilesystemError::IO)?;
+            Ok(metadata)
+        }
     }
 }

gdriver-client/src/filesystem/attributes.rs

@@ -117,8 +117,8 @@ pub(crate) fn read_inode_attributes_from_metadata(
     metadata: Metadata,
     inode: Inode,
     open_file_handles: u64,
-) -> Result<InodeAttributes> {
+) -> InodeAttributes {
-    Ok(InodeAttributes {
+    InodeAttributes {
         inode,
         open_file_handles,
         size: metadata.size,
@@ -131,7 +131,7 @@ pub(crate) fn read_inode_attributes_from_metadata(
         uid: metadata.uid,
         gid: metadata.gid,
         xattrs: metadata.xattrs,
-    })
+    }
 }
 pub(crate) fn read_inode_attributes_from_meta_file(
     meta_path: &Path,
@@ -139,7 +139,11 @@ pub(crate) fn read_inode_attributes_from_meta_file(
     open_file_handles: u64,
 ) -> Result<InodeAttributes> {
     let metadata = read_metadata_file(meta_path)?;
-    read_inode_attributes_from_metadata(metadata, inode, open_file_handles)
+    Ok(read_inode_attributes_from_metadata(
+        metadata,
+        inode,
+        open_file_handles,
+    ))
 }
 
 impl From<InodeAttributes> for fuser::FileAttr {

gdriver-client/src/filesystem/macros.rs

@@ -43,19 +43,19 @@ mod send_requests {
         }};
     }
     #[macro_export]
-    macro_rules! send_request_handled {
+    macro_rules! send_request_handled_internal {
         ($func:expr ,$reply:ident, $error_code:expr, $error_msg:expr, $($arg:tt)*) => {{
             let x = send_request!($func);
             reply_error_e!(x, $reply, $error_code, $error_msg, $($arg)*)
         }};
     }
     #[macro_export]
-    macro_rules! send_request_handled2 {
+    macro_rules! send_request_handled {
         ($func:expr ,$reply:ident) => {
-            send_request_handled2!($func, $reply, "")
+            send_request_handled!($func, $reply, "")
         };
         ($func:expr ,$reply:ident, $error_msg:expr) => {
-            send_request_handled!(
+            send_request_handled_internal!(
                 $func,
                 $reply,
                 ::libc::EREMOTEIO,
@@ -66,12 +66,12 @@ mod send_requests {
     }
 
     #[macro_export]
-    macro_rules! send_request_handled2_consuming {
+    macro_rules! send_request_handled_consuming {
         ($func:expr ,$reply:ident) => {
-            send_request_handled2_consuming!($func, $reply, "");
+            send_request_handled_consuming!($func, $reply, "");
         };
         ($func:expr ,$reply:ident, $error_msg:expr) => {
-            let _ = send_request_handled2!($func, $reply, $error_msg);
+            let _ = send_request_handled!($func, $reply, $error_msg);
         };
     }
 }

gdriver-client/src/prelude.rs

@@ -1,2 +1,8 @@
 pub(crate) use gdriver_common::prelude::result::*;
 pub(crate) use gdriver_common::prelude::*;
+pub(crate) mod macros {
+    pub(crate) use crate::{
+        reply_error_e, reply_error_o, send_request, send_request_handled_internal, send_request_handled,
+        send_request_handled_consuming,
+    };
+}

gdriver-common/src/ipc/gdriver_service.rs

@@ -55,6 +55,16 @@ impl GDriverSettings {
     pub fn downloaded_path(&self) -> &Path {
         &self.downloaded_path
     }
+
+    pub fn get_metadata_file_path(&self, id: &DriveId) -> PathBuf {
+        self.metadata_path.join(id.as_ref()).with_extension("meta")
+    }
+    pub fn get_downloaded_file_path(&self, id: &DriveId) -> PathBuf {
+        self.downloaded_path.join(id.as_ref())
+    }
+    pub fn get_cache_file_path(&self, id: &DriveId) -> PathBuf {
+        self.cache_path.join(id.as_ref())
+    }
 }
 
 impl Default for GDriverSettings {