Your summary is accurate. Currently distribute assumes link(2)ability of non-directories to provide POSIX semantics that the destination path should always be accessible if a file already existed at the time of rename(2). I will review this patch in more detail. In the mean time, a couple of points
1. Can you submit this patch in accordance to the development work flow outlined at http://www.gluster.com/community/documentation/index.php/Development_Work_Flow 2. Can you check if the other file types, like block/char devices, named pipes, named sockets etc are link(2)able? We might have to extend a non-link(2) based solution to the rest of the filetypes. Avati On Sun, Jul 31, 2011 at 1:44 PM, Emmanuel Dreyfus <[email protected]> wrote: > 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 > [email protected] > > _______________________________________________ > Gluster-devel mailing list > [email protected] > https://lists.nongnu.org/mailman/listinfo/gluster-devel >
_______________________________________________ Gluster-devel mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/gluster-devel
