Update Getxattr and Removexattr methods to return ENODATA for non-existent attributes

This commit modifies the Getxattr and Removexattr methods in the OrcasNode to return ENODATA when attributes are not found, improving clarity in the response for cache hits and preventing infinite loops during removals. Debug logging is enhanced to reflect these changes, ensuring better traceability of attribute handling within the VFS.

Author: orca-zhang

vfs/fs.go

@@ -4517,8 +4517,8 @@ func (n *OrcasNode) Getxattr(ctx context.Context, attr string, dest []byte) (uin
 			if exists {
 				// Check if this is a "not found" sentinel (nil)
 				if value == nil {
-					DebugLog("[VFS Getxattr] Cache hit (sentinel): objID=%d, attr=%s, returning 0", n.objID, attr)
-					return 0, 0
+					DebugLog("[VFS Getxattr] Cache hit (sentinel): objID=%d, attr=%s, returning ENODATA", n.objID, attr)
+					return 0, syscall.ENODATA
 				}
 				if len(value) > len(dest) {
 					DebugLog("[VFS Getxattr] Cache hit but buffer too small: objID=%d, attr=%s, valueLen=%d, destLen=%d, returning ERANGE", n.objID, attr, len(value), len(dest))
@@ -4572,7 +4572,7 @@ func (n *OrcasNode) Getxattr(ctx context.Context, attr string, dest []byte) (uin
 					entry.attrs[attr] = nil // Use nil as sentinel
 					entry.mu.Unlock()
 					attrCache.Put(cacheKey, entry)
-					return 0, 0
+					return 0, syscall.ENODATA
 				}
 				// For other errors (database errors, etc.), return EIO instead of ENODATA
 				DebugLog("[VFS Getxattr] ERROR: Failed to get attribute: objID=%d, attr=%s, error=%v", n.objID, attr, err)
@@ -4678,14 +4678,29 @@ func (n *OrcasNode) Removexattr(ctx context.Context, attr string) syscall.Errno
 	if lh, ok := n.fs.h.(*core.LocalHandler); ok {
 		ma := lh.MetadataAdapter()
 		if ma != nil {
+			// Check cache first to see if attribute already doesn't exist
+			cacheKey := n.objID
+			if cached, ok := attrCache.Get(cacheKey); ok {
+				if entry, ok := cached.(*attrCacheEntry); ok && entry != nil {
+					entry.mu.RLock()
+					value, exists := entry.attrs[attr]
+					entry.mu.RUnlock()
+					if exists && value == nil {
+						// Cache already has sentinel (attribute doesn't exist)
+						// Return ENODATA to prevent infinite loop
+						DebugLog("[VFS Removexattr] Attribute already marked as non-existent in cache: objID=%d, attr=%s, returning ENODATA", n.objID, attr)
+						return syscall.ENODATA
+					}
+				}
+			}
+
 			DebugLog("[VFS Removexattr] Removing attribute from database: objID=%d, attr=%s", n.objID, attr)
 			err := ma.RemoveAttr(n.fs.c, n.fs.bktID, n.objID, attr)
 			if err != nil {
 				// Check if this is a "not found" error (attribute doesn't exist)
 				if strings.Contains(err.Error(), "attribute not found") || strings.Contains(err.Error(), "not found") {
 					DebugLog("[VFS Removexattr] Attribute not found: objID=%d, attr=%s, setting sentinel in cache", n.objID, attr)
 					// Attribute doesn't exist, set sentinel in cache to prevent repeated queries
-					cacheKey := n.objID
 					var entry *attrCacheEntry
 					if cached, ok := attrCache.Get(cacheKey); ok {
 						if e, ok := cached.(*attrCacheEntry); ok && e != nil {
@@ -4701,15 +4716,16 @@ func (n *OrcasNode) Removexattr(ctx context.Context, attr string) syscall.Errno
 					entry.attrs[attr] = nil // Set sentinel to indicate attribute doesn't exist
 					entry.mu.Unlock()
 					attrCache.Put(cacheKey, entry)
-					return 0
+					// Return ENODATA to indicate attribute doesn't exist and prevent infinite loop
+					return syscall.ENODATA
 				}
 				// For other errors (database errors, etc.), return EIO
 				DebugLog("[VFS Removexattr] ERROR: Failed to remove attribute: objID=%d, attr=%s, error=%v", n.objID, attr, err)
 				return syscall.EIO
 			}
 
 			// Update cache: set sentinel (nil) to indicate attribute doesn't exist
-			cacheKey := n.objID
+			// cacheKey is already defined above
 			var entry *attrCacheEntry
 			if cached, ok := attrCache.Get(cacheKey); ok {
 				if e, ok := cached.(*attrCacheEntry); ok && e != nil {

vfs/vfs_test.go

@@ -4319,13 +4319,17 @@ func TestXattrSetGetRemove(t *testing.T) {
 			errno = fileNode.Removexattr(ctx, attrName)
 			So(errno, ShouldEqual, syscall.Errno(0))
 
-			// Get attribute again - should not exist (return 0, 0)
-			// This is the key test: after removal, Getxattr should return 0, 0
-			// instead of ENODATA, indicating the attribute doesn't exist
+			// Get attribute again - should not exist (return ENODATA)
+			// This is the key test: after removal, Getxattr should return ENODATA
+			// indicating the attribute doesn't exist
 			dest2 := make([]byte, 1024)
 			size2, errno2 := fileNode.Getxattr(ctx, attrName, dest2)
-			So(errno2, ShouldEqual, syscall.Errno(0))
+			So(errno2, ShouldEqual, syscall.ENODATA)
 			So(size2, ShouldEqual, uint32(0))
+
+			// Try to remove again - should return ENODATA to prevent infinite loop
+			errno3 := fileNode.Removexattr(ctx, attrName)
+			So(errno3, ShouldEqual, syscall.ENODATA)
 		})
 	})
 }