gluster-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Gluster-devel] REVERT: Change in glusterfs[master]: fuse: auxiliary gfi


From: Anand Avati
Subject: [Gluster-devel] REVERT: Change in glusterfs[master]: fuse: auxiliary gfid mount support
Date: Wed, 31 Jul 2013 05:42:30 -0700
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130620 Thunderbird/17.0.7

On 7/19/13 1:14 AM, Vijay Bellur (Code Review) wrote:
Vijay Bellur has submitted this change and it was merged.

Change subject: fuse: auxiliary gfid mount support
......................................................................


fuse: auxiliary gfid mount support

* files can be accessed directly through their gfid and not just
   through their paths. For eg., if the gfid of a file is
   f3142503-c75e-45b1-b92a-463cf4c01f99, that file can be accessed
   using <gluster-mount>/.gfid/f3142503-c75e-45b1-b92a-463cf4c01f99

   .gfid is a virtual directory used to seperate out the namespace
   for accessing files through gfid. This way, we do not conflict with
   filenames which can be qualified as uuids.

* A new file/directory/symlink can be created with a pre-specified
   gfid. A setxattr done on parent directory with fuse_auxgfid_newfile_args_t
   initialized with appropriate fields as value to key "glusterfs.gfid.newfile"
   results in the entry <parent>/bname whose gfid is set to args.gfid. The
   contents of the structure should be in network byte order.

   struct auxfuse_symlink_in {
         char     linkpath[]; /* linkpath is a null terminated string */
   } __attribute__ ((__packed__));

   struct auxfuse_mknod_in {
         unsigned int   mode;
         unsigned int   rdev;
         unsigned int   umask;
   } __attribute__ ((__packed__));

   struct auxfuse_mkdir_in {
         unsigned int   mode;
         unsigned int   umask;
   } __attribute__ ((__packed__));

   typedef struct {
         unsigned int  uid;
         unsigned int  gid;
         char          gfid[UUID_CANONICAL_FORM_LEN + 1]; /* a null terminated 
gfid string
                                                       * in canonical form.
                                                       */
         unsigned int  st_mode;
         char          bname[];     /* bname is a null terminated string */

         union {
                 struct auxfuse_mkdir_in   mkdir;
                 struct auxfuse_mknod_in   mknod;
                 struct auxfuse_symlink_in symlink;
         } __attribute__ ((__packed__)) args;
   } __attribute__ ((__packed__)) fuse_auxgfid_newfile_args_t;

   An initial consumer of this feature would be geo-replication to
   create files on slave mount with same gfids as that on master.
   It will also help gsyncd to access files directly through their
   gfids. gsyncd in its newer version will be consuming a changelog
   (of master) containing operations on gfids and sync corresponding
   files to slave.

* Also, bring in support to heal gfids with a specific value.
   fuse-bridge sends across a gfid during a lookup, which storage
   translators assign to an inode (file/directory etc) if there is
   no gfid associated it. This patch brings in support
   to specify that gfid value from an application, instead of relying
   on random gfid generated by fuse-bridge.

   gfids can be healed through setxattr interface. setxattr should be
   done on parent directory. The key used is "glusterfs.gfid.heal"
   and the value should be the following structure whose contents
   should be in network byte order.

   typedef struct {
         char      gfid[UUID_CANONICAL_FORM_LEN + 1]; /* a null terminated gfid
                                                       * string in canonical 
form
                                                       */
         char      bname[]; /* a null terminated basename */
   } __attribute__((__packed__)) fuse_auxgfid_heal_args_t;

   This feature can be used for upgrading older geo-rep setups where gfids
   of files are different on master and slave to newer setups where they
   should be same. One can delete gfids on slave using setxattr -x and
   .glusterfs and issue stat on all the files with gfids from master.

Thanks to "Amar Tumballi" <address@hidden> and "Csaba Henk"
<address@hidden> for their inputs.

Signed-off-by: Raghavendra G <address@hidden>
Change-Id: Ie8ddc0fb3732732315c7ec49eab850c16d905e4e
BUG: 952029
Reviewed-on: http://review.gluster.com/#/c/4702
Reviewed-by: Amar Tumballi <address@hidden>
Tested-by: Amar Tumballi <address@hidden>
Reviewed-on: http://review.gluster.org/4702
Reviewed-by: Xavier Hernandez <address@hidden>
Tested-by: Gluster Build System <address@hidden>
Reviewed-by: Vijay Bellur <address@hidden>
---
M glusterfsd/src/glusterfsd.c
M glusterfsd/src/glusterfsd.h
M libglusterfs/src/glusterfs.h
M libglusterfs/src/inode.c
M libglusterfs/src/inode.h
M xlators/cluster/dht/src/dht-common.c
M xlators/mount/fuse/src/Makefile.am
M xlators/mount/fuse/src/fuse-bridge.c
M xlators/mount/fuse/src/fuse-bridge.h
M xlators/mount/fuse/src/fuse-helpers.c
A xlators/mount/fuse/src/glfs-fuse-bridge.h
M xlators/mount/fuse/utils/mount.glusterfs.in
M xlators/storage/posix/src/posix.c
13 files changed, 1,317 insertions(+), 136 deletions(-)

Approvals:
   Xavier Hernandez: Looks good to me, but someone else must approve
   Amar Tumballi: Looks good to me, approved
   Vijay Bellur: Looks good to me, approved
   Gluster Build System: Verified




Raghavendra,

This patch needs significant rework in fuse-bridge.c. Particular concerns are (all of which can be fixed):

- fuse_nodeid_t - this a very confusing structure. It is building relations between inodes which messes up the ref model. Are inode pointers stored there with refs? or without refs? If inode pointers are stored with refs, then we are in a situation where we potentially never forget the inode (who initiates the extra unref and how?). If inode pointers are stored without ref, then there is no guarantee that the pointer deref is not going to land you on an object which was destroyed just a moment ago. Not only is the ref model very murky, it is extremely confusing to visualize the working during concurrent access and forgets.

- Per inode allocation of a new structure. Do we really need a new inode context? While we try to minimize per-inode allocations as much as possible, this is adding another structure for a reason which can be addressed in a different way (see below)

- Extra level of pointer indirection for fuse handle to inode conversion in every operation. Not only are we allocating per inode, the fuse handle is now a double deref through a _member_ of the ctx structure. Not only does this worsen memory locality, it is adding complexity for no good reason.

- Multiple types of inode contexts depending on situation. This is a problem which is arising with http://review.gluster.org/5267. We are now forced into a situation where the inode ctx is of different _types_ depending on whether the inode is for the gfid view or the path view. Having multipe _types_ of inode ctxes is a nightmare for maintenance, and we have already seen a crash where the code confuses it to be of one type instead of another in a place.

Instead, I propose this:

- Revert all of the changes done to fuse-bridge.c, bring it back to the state how it was before the patch (carefully considering other changes which have happened beyond this patch)
- Introduce a new translator to overlay a virtual gfid view
- Normal inodes can continue to return original gfids as-is.
- Virtual inodes can create a random on-the fly gfid (which need not be persisted), and identified by a numerical flag in inode ctx without allocating a per-inode object. - Upon access of the virtual inodes (which can be identified with an integer flag in the inode context without a new structure), redirect the operation to the inode which has the gfid whose canonical form is the dentry name of the virtual inode.

Let fuse-bridge be a pure fuse <--> xlator converter. Adding a new (gfid) view is clearly a separate concern, best implemented as a separate translator.

Thanks,
Avati




reply via email to

[Prev in Thread] Current Thread [Next in Thread]