On 2014-06-17 18:47, Anders Blomdell wrote:
> On 2014-06-17 17:49, Shyamsundar Ranganathan wrote:
>> You maybe looking at the problem being fixed here, [1].
>>
>> On a lookup attribute mismatch was not being healed across
>> directories, and this patch attempts to address the same. Currently
>> the version of the patch does not heal the S_ISUID and S_ISGID bits,
>> which is work in progress (but easy enough to incorporate and test
>> based on the patch at [1]).
> Thanks, will look into it tomorrow.
> 
>> On a separate note, add-brick just adds a brick to the cluster, the
>> lookup is where the heal (or creation of the directory across all sub
>> volumes in DHT xlator) is being done.
> Thanks for the clarification (I guess that a rebalance would trigger it as 
> well?)
Attached slightly modified version of patch [1] seems to work correctly after a 
rebalance
that is allowed to run to completion on its own, if directories are traversed 
during rebalance, 
some 07777 dirs show spurious 01777, 00000 and sometimes ends up with the wrong 
permission.

Continuing debug tomorrow...
> 
>>
>> Shyam
>>
>> [1] http://review.gluster.org/#/c/6983/
>>
>> ----- Original Message -----
>>> From: "Anders Blomdell" <anders.blomd...@control.lth.se> To:
>>> "Gluster Devel" <gluster-devel@gluster.org> Sent: Tuesday, June 17,
>>> 2014 10:53:52 AM Subject: [Gluster-devel] 3.5.1-beta2 Problems with
>>> suid and sgid bits on       directories
>>>
>> With a glusterfs-3.5.1-0.3.beta2.fc20.x86_64 with a reverted 
>> 3dc56cbd16b1074d7ca1a4fe4c5bf44400eb63ff (due to local lack of IPv4 
>> addresses), I get weird behavior if I:
>>
>> 1. Create a directory with suid/sgid/sticky bit set
>> (/mnt/gluster/test) 2. Make a subdirectory of #1
>> (/mnt/gluster/test/dir1) 3. Do an add-brick
>>
>> Before add-brick
>>
>> 755 /mnt/gluster 7775 /mnt/gluster/test 2755 /mnt/gluster/test/dir1
>>
>> After add-brick
>>
>> 755 /mnt/gluster 1775 /mnt/gluster/test 755 /mnt/gluster/test/dir1
>>
>> On the server it looks like this:
>>
>> 7775 /data/disk1/gluster/test 2755 /data/disk1/gluster/test/dir1 1775
>> /data/disk2/gluster/test 755 /data/disk2/gluster/test/dir1
>>
>> Filed as bug:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1110262
>>
>> If somebody can point me to where the logic of add-brick is placed, I
>> can give it a shot (a find/grep on mkdir didn't immediately point me
>> to the right place).
>>
>>
> /Anders
> 
/Anders

-- 
Anders Blomdell                  Email: anders.blomd...@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118                     Fax:      +46 46 138118
SE-221 00 Lund, Sweden

diff -urb glusterfs-3.5.1beta2/xlators/cluster/dht/src/dht-common.c glusterfs-3.5.1.orig/xlators/cluster/dht/src/dht-common.c
--- glusterfs-3.5.1beta2/xlators/cluster/dht/src/dht-common.c	2014-06-10 18:55:22.000000000 +0200
+++ glusterfs-3.5.1.orig/xlators/cluster/dht/src/dht-common.c	2014-06-17 22:46:28.710636632 +0200
@@ -523,6 +523,28 @@
 }
 
 int
+permission_changed (ia_prot_t *local, ia_prot_t *stbuf)
+{
+    if( (local->owner.read != stbuf->owner.read) ||
+        (local->owner.write != stbuf->owner.write) ||
+        (local->owner.exec != stbuf->owner.exec) ||
+        (local->group.read != stbuf->group.read) ||
+        (local->group.write != stbuf->group.write) ||
+        (local->group.exec != stbuf->group.exec) ||
+        (local->other.read != stbuf->other.read) ||
+        (local->other.write != stbuf->other.write) ||
+        (local->other.exec != stbuf->other.exec ) ||
+        (local->suid != stbuf->suid ) ||
+        (local->sgid != stbuf->sgid ) ||
+        (local->sticky != stbuf->sticky ))
+    {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
+int
 dht_revalidate_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                     int op_ret, int op_errno,
                     inode_t *inode, struct iatt *stbuf, dict_t *xattr,
@@ -617,12 +639,16 @@
                                                     stbuf->ia_ctime_nsec)) {
                                         local->prebuf.ia_gid = stbuf->ia_gid;
                                         local->prebuf.ia_uid = stbuf->ia_uid;
+                                        local->prebuf.ia_prot = stbuf->ia_prot;
                                 }
                         }
                         if (local->stbuf.ia_type != IA_INVAL)
                         {
                                 if ((local->stbuf.ia_gid != stbuf->ia_gid) ||
-                                    (local->stbuf.ia_uid != stbuf->ia_uid)) {
+                                    (local->stbuf.ia_uid != stbuf->ia_uid) ||
+                                    (permission_changed (&(local->stbuf.ia_prot)
+                                                        , (&(stbuf->ia_prot)))))
+                                {
                                         local->need_selfheal = 1;
                                 }
                         }
@@ -669,6 +695,8 @@
                         uuid_copy (local->gfid, local->stbuf.ia_gfid);
                         local->stbuf.ia_gid = local->prebuf.ia_gid;
                         local->stbuf.ia_uid = local->prebuf.ia_uid;
+                        local->stbuf.ia_prot = local->prebuf.ia_prot;
+
                         copy = create_frame (this, this->ctx->pool);
                         if (copy) {
                                 copy_local = dht_local_init (copy, &local->loc,
diff -urb glusterfs-3.5.1beta2/xlators/cluster/dht/src/dht-selfheal.c glusterfs-3.5.1.orig/xlators/cluster/dht/src/dht-selfheal.c
--- glusterfs-3.5.1beta2/xlators/cluster/dht/src/dht-selfheal.c	2014-06-10 18:55:22.000000000 +0200
+++ glusterfs-3.5.1.orig/xlators/cluster/dht/src/dht-selfheal.c	2014-06-17 22:53:18.809616163 +0200
@@ -1010,8 +1010,8 @@
                 if (!subvol || (subvol == dht_first_up_subvol (this)))
                         continue;
                 ret = syncop_setattr (subvol, &local->loc, &local->stbuf,
-                                      (GF_SET_ATTR_UID | GF_SET_ATTR_GID),
-                                      NULL, NULL);
+                                      (GF_SET_ATTR_UID | GF_SET_ATTR_GID |
+				       GF_SET_ATTR_MODE), NULL, NULL);
                 if (ret)
                         gf_log ("dht", GF_LOG_ERROR, "Failed to set uid/gid on"
                                 " %s on %s subvol (%s)", local->loc.path,
_______________________________________________
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel

Reply via email to