[Gluster-devel] Fixing dht rename of directory symlink

[Emmanuel Dreyfus]

This is a followup to my previous messages about dht and rename. Here is a
quick summary for the impatient:

dht rename works on non directory files by link/rename/unlink. As I understand
this is to make sure the file is accessible during the rename operation.
However this breaks on the standard front when we rename a symlink to a
directory.

Standards say that link(2) on a directory may not be supported. In is indeed
not supported on Linux ext3fs, nor on NetBSD FFS. Nothing is said in the
standards about link(2) on a symlink to a directory. Linux does it, making two
symlinks with same inode. NetBSD first resolves the symlink, discovers the
link(2) attempt on a directory, and raises EPERM.

Both approaches to link(2) on symlink to a directory are standared compliant,
but glusterfs relying on a given behavior here is a mistake. I talked to
NetBSD crowd about adding a linux_link(2) system call, but that proposal did
not reach consensus. Therefore I would like to fix this in glusterfs.

Here is a first try. It works fine when rename operates on the same subvolume.
It works but never gives control back to the calling process when it happens
of different subvolumes.

Help would be welcome. 

--- dht-rename.c.orig   2011-07-31 09:19:02.000000000 +0200
+++ dht-rename.c        2011-07-31 09:46:09.000000000 +0200
@@ -501,9 +501,10 @@
 
         if (local->call_cnt == 0)
                 goto unwind;
 
-        if (src_cached != dst_hashed && src_cached != dst_cached) {
+        if (!IA_ISLNK(local->loc.inode->ia_type) &&
+           src_cached != dst_hashed && src_cached != dst_cached) {
                 gf_log (this->name, GF_LOG_TRACE,
                         "deleting old src datafile %s @ %s",
                         local->loc.path, src_cached->name);
 
@@ -521,9 +522,9 @@
                             src_hashed, src_hashed->fops->unlink,
                             &local->loc);
         }
 
-        if (dst_cached
+        if (!IA_ISLNK(local->loc.inode->ia_type) && dst_cached
             && (dst_cached != dst_hashed)
             && (dst_cached != src_cached)) {
                 gf_log (this->name, GF_LOG_TRACE,
                         "deleting old dst datafile %s @ %s",
@@ -669,14 +670,25 @@
                                        src_cached, dst_hashed, &local->loc);
         }
 
         if (src_cached != dst_hashed) {
-                gf_log (this->name, GF_LOG_TRACE,
-                        "link %s => %s (%s)", local->loc.path,
-                        local->loc2.path, src_cached->name);
-                STACK_WIND (frame, dht_rename_links_cbk,
-                            src_cached, src_cached->fops->link,
-                            &local->loc, &local->loc2);
+               if (IA_ISLNK(local->loc.inode->ia_type)) {
+                       gf_log (this->name, GF_LOG_TRACE,
+                               "rename link %s => %s (%s)", local->loc.path,
+                               local->loc2.path, src_cached->name);
+
+                       STACK_WIND (frame, dht_rename_cbk,
+                                   src_cached, src_cached->fops->rename,
+                                   &local->loc, &local->loc2);
+               } else {
+                       gf_log (this->name, GF_LOG_TRACE,
+                               "link %s => %s (%s)", local->loc.path,
+                               local->loc2.path, src_cached->name);
+
+                       STACK_WIND (frame, dht_rename_links_cbk,
+                                   src_cached, src_cached->fops->link,
+                                   &local->loc, &local->loc2);
+               }
         }
 
 nolinks:
         if (!call_cnt) {
bacasable# q
bacasable# diff -U4 dht-rename.c.orig dht-rename.c     
--- dht-rename.c.orig   2011-07-31 09:19:02.000000000 +0200
+++ dht-rename.c        2011-07-31 09:46:09.000000000 +0200
@@ -501,9 +501,10 @@
 
         if (local->call_cnt == 0)
                 goto unwind;
 
-        if (src_cached != dst_hashed && src_cached != dst_cached) {
+        if (!IA_ISLNK(local->loc.inode->ia_type) &&
+           src_cached != dst_hashed && src_cached != dst_cached) {
                 gf_log (this->name, GF_LOG_TRACE,
                         "deleting old src datafile %s @ %s",
                         local->loc.path, src_cached->name);
 
@@ -521,9 +522,9 @@
                             src_hashed, src_hashed->fops->unlink,
                             &local->loc);
         }
 
-        if (dst_cached
+        if (!IA_ISLNK(local->loc.inode->ia_type) && dst_cached
             && (dst_cached != dst_hashed)
             && (dst_cached != src_cached)) {
                 gf_log (this->name, GF_LOG_TRACE,
                         "deleting old dst datafile %s @ %s",
@@ -669,14 +670,25 @@
                                        src_cached, dst_hashed, &local->loc);
         }
 
         if (src_cached != dst_hashed) {
-                gf_log (this->name, GF_LOG_TRACE,
-                        "link %s => %s (%s)", local->loc.path,
-                        local->loc2.path, src_cached->name);
-                STACK_WIND (frame, dht_rename_links_cbk,
-                            src_cached, src_cached->fops->link,
-                            &local->loc, &local->loc2);
+               if (IA_ISLNK(local->loc.inode->ia_type)) {
+                       gf_log (this->name, GF_LOG_TRACE,
+                               "rename link %s => %s (%s)", local->loc.path,
+                               local->loc2.path, src_cached->name);
+
+                       STACK_WIND (frame, dht_rename_cbk,
+                                   src_cached, src_cached->fops->rename,
+                                   &local->loc, &local->loc2);
+               } else {
+                       gf_log (this->name, GF_LOG_TRACE,
+                               "link %s => %s (%s)", local->loc.path,
+                               local->loc2.path, src_cached->name);
+
+                       STACK_WIND (frame, dht_rename_links_cbk,
+                                   src_cached, src_cached->fops->link,
+                                   &local->loc, &local->loc2);
+               }
         }
 
 nolinks:
         if (!call_cnt) {


-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu at netbsd.org