[Gluster-devel] Fixing dht rename of directory symlink
Anand Avati
anand.avati at gmail.com
Sun Jul 31 12:26:42 UTC 2011
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 <manu at netbsd.org> 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
> manu at netbsd.org
>
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at nongnu.org
> https://lists.nongnu.org/mailman/listinfo/gluster-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://supercolony.gluster.org/pipermail/gluster-devel/attachments/20110731/488a4032/attachment-0003.html>
More information about the Gluster-devel
mailing list