feat: xattr name index and ordered insertion

ext4_test/src/main.rs

@@ -117,21 +117,27 @@ fn xattr_test(ext4: &mut Ext4) {
     let file = ext4
         .generic_create(ROOT_INO, "f2", file_mode)
         .expect("Create failed");
-    ext4.setxattr(file, "user.test1", "hello world".as_bytes())
+    ext4.setxattr(file, "user.testone", "hello world".as_bytes())
         .expect("setxattr failed");
-    ext4.setxattr(file, "user.test2", "world hello".as_bytes())
+    ext4.setxattr(file, "user.testtwo", "world hello".as_bytes())
         .expect("setxattr failed");
-    let value = ext4.getxattr(file, "user.test1").expect("getxattr failed");
+
+    let names = ext4.listxattr(file).expect("listxattr failed");
+    assert_eq!(names, vec!["user.testone", "user.testtwo"]);
+    
+    let value = ext4.getxattr(file, "user.testone").expect("getxattr failed");
     assert_eq!(value, "hello world".as_bytes());
-    let value = ext4.getxattr(file, "user.test2").expect("getxattr failed");
+    let value = ext4.getxattr(file, "user.testtwo").expect("getxattr failed");
     assert_eq!(value, "world hello".as_bytes());
+    
     let names = ext4.listxattr(file).expect("listxattr failed");
-    assert_eq!(names, vec!["user.test1", "user.test2"]);
-    ext4.removexattr(file, "user.test1").expect("removexattr failed");
-    ext4.getxattr(file, "user.test1")
+    assert_eq!(names, vec!["user.testone", "user.testtwo"]);
+    
+    ext4.removexattr(file, "user.testone").expect("removexattr failed");
+    ext4.getxattr(file, "user.testone")
         .expect_err("getxattr failed");
     let names = ext4.listxattr(file).expect("listxattr failed");
-    assert_eq!(names, vec!["user.test2"]);
+    assert_eq!(names, vec!["user.testtwo"]);
 }
 
 fn main() {
@@ -151,6 +157,7 @@ fn main() {
     println!("large read write test done");
     remove_file_test(&mut ext4);
     println!("remove file test done");
+    log::set_max_level(log::LevelFilter::Trace);
     xattr_test(&mut ext4);
     println!("xattr test done");
 }

src/ext4_defs/xattr.rs

@@ -11,6 +11,7 @@
 use super::{AsBytes, Block};
 use crate::constants::*;
 use crate::prelude::*;
+use core::cmp::Ordering;
 
 /// The beginning of an extended attribute block.
 #[repr(C)]
@@ -53,7 +54,12 @@ impl XattrHeader {
 pub struct XattrEntry {
     /// Length of name.
     name_len: u8,
-    /// Attribute name index (UNUSED by now)
+    /// Attribute name index.
+    ///
+    /// To reduce the amount of on-disk space that the keys consume, the
+    /// beginningof the key string is matched against the attribute name
+    /// index. If a match is found, the attribute name index field is set,
+    /// and matching string is removed from the key name.
     name_index: u8,
     /// Location of this attribute's value on the disk block where
     /// it is stored. For a block this value is relative to the start
@@ -117,11 +123,12 @@ impl XattrEntry {
     /// Create a new xattr entry.
     pub fn new(name: &str, value_size: usize, value_offset: usize) -> Self {
         let mut name_bytes = [0u8; 255];
+        let (name_index, name) = Self::match_name(name);
         let name_len = name.as_bytes().len();
         name_bytes[..name_len].copy_from_slice(name.as_bytes());
         Self {
             name_len: name.len() as u8,
-            name_index: 0,
+            name_index,
             value_offset: value_offset as u16,
             value_inum: 0,
             value_size: value_size as u32,
@@ -132,24 +139,64 @@ impl XattrEntry {
 
     /// Get the name of the xattr entry
     pub fn name(&self) -> String {
-        let name = &self.name[..self.name_len as usize];
-        unsafe { String::from_utf8_unchecked(name.to_vec()) }
-    }
-
-    /// Compare the name of the xattr entry with a given name
-    pub fn compare_name(&self, name: &str) -> bool {
-        &self.name[..name.len()] == name.as_bytes()
+        let prefix = match self.name_index {
+            1 => "user.",
+            2 => "system.posix_acl_access.",
+            3 => "system.posix_acl_default.",
+            4 => "trusted.",
+            6 => "security.",
+            7 => "system.",
+            _ => "",
+        };
+        let name_bytes = &self.name[..self.name_len as usize];
+        let name = unsafe { String::from_utf8_unchecked(name_bytes.to_vec()) };
+        prefix.to_string() + &name
     }
 
     /// Get the required size to save a xattr entry, 4-byte aligned
-    pub fn required_size(name_len: usize) -> usize {
-        // u32 + u16 + u8 + Ext4DirEnInner + name -> align to 4
+    pub fn required_size(name: &str) -> usize {
+        let (_, name) = Self::match_name(name);
+        let name_len = name.len();
+        // FakeXattrEntry + name -> align to 4
         (core::mem::size_of::<FakeXattrEntry>() + name_len + 3) / 4 * 4
     }
 
     /// Get the used size of this xattr entry, 4-bytes alighed
     pub fn used_size(&self) -> usize {
-        Self::required_size(self.name_len as usize)
+        (core::mem::size_of::<FakeXattrEntry>() + self.name_len as usize + 3) / 4 * 4
+    }
+
+    /// Compare the name of the xattr entry with a given name
+    pub fn compare_name(&self, name: &str) -> Ordering {
+        let (name_index, name) = Self::match_name(name);
+        match self.name_index.cmp(&name_index) {
+            Ordering::Equal => {}
+            ordering => return ordering,
+        };
+        match self.name_len.cmp(&(name.len() as u8)) {
+            Ordering::Equal => {}
+            ordering => return ordering,
+        }
+        self.name[..self.name_len as usize].cmp(name.as_bytes())
+    }
+
+    /// Match the attribute name prefix to get name index. If one is found,
+    /// return the name index and the string with the prefix removed.
+    fn match_name(name: &str) -> (u8, &str) {
+        let prefixes = [
+            ("user.", 1),
+            ("system.posix_acl_access.", 2),
+            ("system.posix_acl_default.", 3),
+            ("trusted.", 4),
+            ("security.", 6),
+            ("system.", 7),
+        ];
+        for (prefix, index) in prefixes {
+            if name.starts_with(prefix) {
+                return (index, &name[prefix.len()..]);
+            }
+        }
+        (0, name)
     }
 }
 
@@ -192,12 +239,10 @@ impl XattrBlock {
                 break;
             }
             let entry: XattrEntry = self.0.read_offset_as(entry_start);
-            if entry.compare_name(name) {
-                return Some(
-                    &self
-                        .0
-                        .read_offset(entry.value_offset as usize, entry.value_size as usize),
-                );
+            if entry.compare_name(name).is_eq() {
+                let offset = entry.value_offset as usize;
+                let size = entry.value_size as usize;
+                return Some(&self.0.data[offset..offset + size]);
             }
             entry_start += entry.used_size();
         }
@@ -223,87 +268,150 @@ impl XattrBlock {
 
     /// Insert a xattr entry into the block. Return true if success.
     pub fn insert(&mut self, name: &str, value: &[u8]) -> bool {
-        let mut entry_start = size_of::<XattrHeader>();
-        let mut value_end = BLOCK_SIZE;
+        let mut p_entry = size_of::<XattrHeader>();
+        let mut p_value = BLOCK_SIZE;
+
+        let mut is_ins_pos_found = false;
+        let mut ins_entry_pos = p_entry;
+        let mut ins_value_pos = p_value;
+        let ins_entry_size = XattrEntry::required_size(name);
+        let ins_value_size = value.len();
+
         // Iterate over entry table, find the position to insert entry
-        while entry_start < BLOCK_SIZE {
+        // and the end of entry table
+        while p_entry < BLOCK_SIZE {
             // Check `name_len`, 0 indicates the end of the entry table.
-            if self.0.data[entry_start] == 0 {
-                // Insert to the end of table
+            if self.0.data[p_entry] == 0 {
+                // Reach the end of table
                 break;
             }
-            let entry: XattrEntry = self.0.read_offset_as(entry_start);
-            entry_start += entry.used_size();
-            value_end = entry.value_offset as usize;
+            let entry: XattrEntry = self.0.read_offset_as(p_entry);
+            if !is_ins_pos_found && entry.compare_name(name).is_gt() {
+                // Insert before this entry
+                ins_entry_pos = p_entry;
+                ins_value_pos = p_value;
+                is_ins_pos_found = true;
+            }
+            p_value = entry.value_offset as usize;
+            p_entry += entry.used_size();
+        }
+        if !is_ins_pos_found {
+            // Insert at the end of table
+            ins_entry_pos = p_entry;
+            ins_value_pos = p_value;
         }
-        // `[entry_start, value_end)` is the empty space
-        // Check space
-        let required_size = XattrEntry::required_size(name.len()) + value.len() + 1;
-        if value_end - entry_start < required_size {
+
+        // `ins_entry_pos` points to the position to insert entry,
+        // `ins_value_pos - ins_value_size` points to the position to insert value.
+        // `p_entry` points to the start of blank area,
+        // `p_value` points to the last value,
+        // `[p_entry, p_value)` is the blank area.
+        
+        // Check space, '+1' is reserved for blank area
+        if p_value - p_entry < ins_entry_size + ins_value_size + 1 {
+            // Not enough space
             return false;
         }
-        // Insert entry
-        let value_offset = value_end - value.len();
-        let entry = XattrEntry::new(name, value.len(), value_offset);
-        self.0.write_offset_as(entry_start, &entry);
-        // Insert value
-        self.0.write_offset(value_offset, value);
+
+        // Move the entries from `ins_entry_pos` to `p_entry`
+        // Copy `[ins_entry_pos, p_entry)` to `[ins_entry_pos+ins_entry_size, p_entry+ins_entry_size)`
+        self.0
+            .data
+            .copy_within(ins_entry_pos..p_entry, ins_entry_pos + ins_entry_size);
+        // Set `[ins_entry_pos..ins_entry_pos+entry_req_size)` to 0
+        self.0.data[ins_entry_pos..ins_entry_pos + ins_entry_size].fill(0);
+
+        // Move the corresponding values
+        // Copy `[p_value, ins_value_pos)` to `[p_value-ins_value_size, ins_value_pos-ins_value_size)`
+        self.0
+            .data
+            .copy_within(p_value..ins_value_pos, p_value - ins_value_size);
+        // Set `[ins_value_pos-ins_value_size, ins_value_pos)` to 0
+        self.0.data[ins_value_pos - ins_value_size..ins_value_pos].fill(0);
+
+        // Update the value offset of the moved entries
+        let mut p_entry2 = ins_entry_pos + ins_entry_size;
+        while p_entry2 < p_entry + ins_entry_size {
+            let mut entry: XattrEntry = self.0.read_offset_as(p_entry2);
+            entry.value_offset -= ins_value_size as u16;
+            self.0.write_offset_as(p_entry2, &entry);
+            p_entry2 += entry.used_size();
+        }
+
+        // Insert entry to `[ins_entry_pos, ins_entry_pos+ins_entry_size)`
+        let entry = XattrEntry::new(name, value.len(), ins_value_pos - ins_value_size);
+        self.0.write_offset_as(ins_entry_pos, &entry);
+        // Insert value to `[ins_value_pos-ins_value_size, ins_value_pos)`
+        self.0.write_offset(ins_value_pos - ins_value_size, value);
+
         true
     }
 
     /// Remove a xattr entry from the block. Return true if success.
     pub fn remove(&mut self, name: &str) -> bool {
-        let mut entry_start = size_of::<XattrHeader>();
-        // Iterate over entry table, find the position to remove entry
-        while entry_start < BLOCK_SIZE {
+        let mut p_entry = size_of::<XattrHeader>();
+        let mut p_value = BLOCK_SIZE;
+
+        let mut is_rem_pos_found = false;
+        let mut rem_entry_pos = p_entry;
+        let mut rem_value_pos = p_value;
+        let mut rem_entry_size = 0;
+        let mut rem_value_size = 0;
+
+        // Iterate over entry table, find the entry to remove
+        while p_entry < BLOCK_SIZE {
             // Check `name_len`, 0 indicates the end of the entry table.
-            if self.0.data[entry_start] == 0 {
-                // Target xattr not found
-                return false;
+            if self.0.data[p_entry] == 0 {
+                break;
             }
-            let entry: XattrEntry = self.0.read_offset_as(entry_start);
+            let entry: XattrEntry = self.0.read_offset_as(p_entry);
+            p_value = entry.value_offset as usize;
             // Compare name
-            if entry.compare_name(name) {
-                break;
+            if !is_rem_pos_found && entry.compare_name(name).is_eq() {
+                rem_entry_pos = p_entry;
+                rem_value_pos = p_value;
+                rem_entry_size = entry.used_size();
+                rem_value_size = entry.value_size as usize;
+                is_rem_pos_found = true;
             }
-            entry_start += entry.used_size();
+            p_entry += entry.used_size();
         }
-        // `entry_start` now points to the removed entry.
-        let removed_entry: XattrEntry = self.0.read_offset_as(entry_start);
-        let removed_entry_size = removed_entry.used_size();
-        // `value_end` points to the end of removed value
-        let mut value_end = removed_entry.value_offset as usize + removed_entry.value_size as usize;
-
-        // Move the following entries and values
-        while entry_start + removed_entry_size < BLOCK_SIZE {
-            let next_entry_start = entry_start + removed_entry_size;
-            // Check `name_len`, 0 indicates the end of the entry table.
-            if self.0.data[next_entry_start] == 0 {
-                break;
-            }
-            // Get the entry to move
-            let mut next_entry: XattrEntry = self.0.read_offset_as(next_entry_start);
-            // Get its value
-            let next_value = self
-                .0
-                .read_offset(
-                    next_entry.value_offset as usize,
-                    next_entry.value_size as usize,
-                )
-                .to_owned();
-            // Move the value
-            let value_offset = value_end - next_value.len();
-            self.0.write_offset(value_offset, &next_value);
-            // Update entry
-            next_entry.value_offset = value_offset as u16;
-            // Write the entry to block
-            self.0.write_offset_as(entry_start, &next_entry);
-            // Update offset
-            value_end -= next_value.len();
-            entry_start += next_entry.used_size();
+        if !is_rem_pos_found {
+            return false;
         }
-        // Clear [entry_offset, value_offset)
-        self.0.data[entry_start..value_end].fill(0);
+
+        // `rem_entry_pos` points to the entry to remove,
+        // `rem_value_pos` points to the value to remove.
+        // `p_entry` points to the start of blank area,
+        // `p_value` points to the last value,
+        // `[p_entry, p_value)` is the blank area.
+
+        // Move the following entries
+        // Copy `[rem_entry_pos + rem_entry_size, p_entry)` to `[rem_entry_pos, p_entry - rem_entry_size)`
+        self.0
+            .data
+            .copy_within(rem_entry_pos + rem_entry_size..p_entry, rem_entry_pos);
+        // Set `[p_entry - rem_entry_size, p_entry)` to 0
+        self.0.data[p_entry - rem_entry_size..p_entry].fill(0);
+
+        // Move the corresponding values
+        // Copy `[p_value, rem_value_pos)` to `[p_value + rem_value_size, rem_value_pos + rem_value_size)`
+        self.0.data.copy_within(
+            p_value..rem_value_pos,
+            p_value + rem_value_size,
+        );
+        // Set `[p_value, p_value + rem_value_size)` to 0
+        self.0.data[p_value..p_value + rem_value_size].fill(0);
+
+        // Update the value offset of the moved entries
+        let mut p_entry2 = rem_entry_pos;
+        while p_entry2 < p_entry - rem_entry_size {
+            let mut entry: XattrEntry = self.0.read_offset_as(p_entry2);
+            entry.value_offset += rem_value_size as u16;
+            self.0.write_offset_as(p_entry2, &entry);
+            p_entry2 += entry.used_size();
+        }
+
         true
     }
 }