[Gluster-devel] Fixing dht rename of directory symlink
Emmanuel Dreyfus
manu at netbsd.org
Sun Jul 31 08:14:20 UTC 2011
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
More information about the Gluster-devel
mailing list