From 4b1fdb2a6c56f7dbfdab4f12fe8320d29260d7f4 Mon Sep 17 00:00:00 2001 From: Jiffin Tony Thottan Date: Sun, 11 Jun 2017 07:33:52 +0530 Subject: [PATCH 522/525] gfapi : Resolve "." and ".." only for named lookups The patch https://review.gluster.org/#/c/17177 resolves "." and ".." to corrosponding inodes and names before sending the request to the backend server. But this will only work if inode and its parent is linked properly. Incase of nameless lookup(applications like ganesha) the inode of parent can be NULL(only gfid is send). So this patch will resolve "." and ".." only if proper parent is available Upstream reference : >Change-Id: I4c50258b0d896dabf000a547ab180b57df308a0b >BUG: 1460514 >Signed-off-by: Jiffin Tony Thottan >Reviewed-on: https://review.gluster.org/17502 >Smoke: Gluster Build System >NetBSD-regression: NetBSD Build System >Reviewed-by: Poornima G >CentOS-regression: Gluster Build System >Reviewed-by: soumya k >Reviewed-by: Jeff Darcy >Signed-off-by: Jiffin Tony Thottan Change-Id: I4c50258b0d896dabf000a547ab180b57df308a0b BUG: 1457183 Signed-off-by: Jiffin Tony Thottan Reviewed-on: https://code.engineering.redhat.com/gerrit/109520 Reviewed-by: Atin Mukherjee --- api/src/glfs-resolve.c | 72 +++++++++++++++++++++++++---- libglusterfs/src/inode.c | 118 ++++++++--------------------------------------- libglusterfs/src/inode.h | 17 +------ 3 files changed, 83 insertions(+), 124 deletions(-) diff --git a/api/src/glfs-resolve.c b/api/src/glfs-resolve.c index 44d013c..8e91cd7 100644 --- a/api/src/glfs-resolve.c +++ b/api/src/glfs-resolve.c @@ -268,6 +268,7 @@ glfs_resolve_component (struct glfs *fs, xlator_t *subvol, inode_t *parent, { loc_t loc = {0, }; inode_t *inode = NULL; + inode_t *temp_parent = NULL; int reval = 0; int ret = -1; int glret = -1; @@ -276,14 +277,12 @@ glfs_resolve_component (struct glfs *fs, xlator_t *subvol, inode_t *parent, dict_t *xattr_req = NULL; uint64_t ctx_value = LOOKUP_NOT_NEEDED; - loc.name = component; - loc.parent = inode_ref (parent); gf_uuid_copy (loc.pargfid, parent->gfid); - /* At this point we should never have '.' or ".." in path */ if (__is_root_gfid (parent->gfid) && - (strcmp (component, "/") == 0)) { + ((strcmp (component, ".") == 0) || + (strcmp (component, "..") == 0))) { if (!force_lookup) { inode = inode_ref (parent); } else { @@ -293,8 +292,68 @@ glfs_resolve_component (struct glfs *fs, xlator_t *subvol, inode_t *parent, } goto found; } + /* * + * if the component name is either "." or "..", it will try to + * resolve that if inode has a proper parent (named lookup). + * + * Below condition works like this + * + * Example 1 : + * Path /out_dir/dir/in_dir/. + * In put values : + * parent = in_dir + * component : "." + * + * Out put values: + * parent : dir + * component : "in_dir" + * + * Example 2 : + * Path /out_dir/dir/in_dir/.. + * In put values : + * parent = in_dir + * component : ".." + * + * Out put values: + * parent : output_dir + * component : "dir" + * + * Incase of nameless lookup, both "." and ".." retained + */ + + if (strcmp (component, ".") == 0) { + loc.inode = inode_ref (parent); + temp_parent = inode_parent (loc.inode, 0, 0); + if (temp_parent) { + inode_unref (loc.parent); + loc.parent = temp_parent; + inode_find_directory_name (loc.inode, &loc.name); + } + + } else if (strcmp (component, "..") == 0) { + loc.inode = inode_parent (parent, 0, 0); + if (loc.inode) { + temp_parent = inode_parent (loc.inode, 0, 0); + if (temp_parent) { + inode_unref (loc.parent); + loc.parent = temp_parent; + inode_find_directory_name (loc.inode, &loc.name); + } else if (__is_root_gfid (loc.inode->gfid)) { + inode_unref (loc.parent); + loc.parent = inode_ref (loc.inode); + loc.name = ""; + } else { + inode_unref (loc.inode); + loc.inode = NULL; + } + + } + } else + loc.inode = inode_grep (parent->table, parent, component); + + if (!loc.name) + loc.name = component; - loc.inode = inode_grep (parent->table, parent, component); if (loc.inode) { gf_uuid_copy (loc.gfid, loc.inode->gfid); reval = 1; @@ -389,7 +448,6 @@ found: out: if (xattr_req) dict_unref (xattr_req); - loc_wipe (&loc); return inode; @@ -409,7 +467,6 @@ priv_glfs_resolve_at (struct glfs *fs, xlator_t *subvol, inode_t *at, char *next_component = NULL; int ret = -1; struct iatt ciatt = {0, }; - char dentry_name[PATH_MAX] = {0, }; DECLARE_OLD_THIS; __GLFS_ENTRY_VALIDATE_FS(fs, invalid_fs); @@ -441,7 +498,6 @@ priv_glfs_resolve_at (struct glfs *fs, xlator_t *subvol, inode_t *at, if (parent) inode_unref (parent); parent = inode; - glusterfs_normalize_dentry (&parent, &component, dentry_name); inode = glfs_resolve_component (fs, subvol, parent, component, &ciatt, /* force hard lookup on the last diff --git a/libglusterfs/src/inode.c b/libglusterfs/src/inode.c index 90a5608..0353825 100644 --- a/libglusterfs/src/inode.c +++ b/libglusterfs/src/inode.c @@ -2549,112 +2549,30 @@ out: return size; } -static void -inode_parent_null_check(inode_t **parent, inode_t *inode, char **component) -{ - GF_VALIDATE_OR_GOTO ("inode", inode, out); - GF_VALIDATE_OR_GOTO ("inode", (*component), out); - - if (!(*parent) && __is_root_gfid (inode->gfid)) { - *parent = inode_ref (inode); - *component = "/"; - } -out: - return; -} - -/* - * This function changes component name and parent inode - * if the component name is either "." or ".." - * - * @Paramas: - * Parent : Parent inode of current dentry - * component : component name that we need to test - * dentry_name : Address for memory if need to change component. - * The caller has to preallocate this memory with - * PATH_MAX as the size. - * - * We return the updated parent inode and component in the - * respective structures. - * - * Basic Idea of the function: - * - * Example 1 : - * Path /out_dir/dir/in_dir/. - * In put values : - * parent = in_dir - * component : "." - * - * Out put values: - * parent : dir - * component : "in_dir" - * - * Example 2 : - * Path /out_dir/dir/in_dir/.. - * In put values : - * parent = in_dir - * component : ".." - * - * Out put values: - * parent : output_dir - * component : "dir" - */ +/* * + * This function finds name of the inode, if it has dentry. The dentry will be + * created only if inode_link happens with valid parent and name. And this + * function is only applicable for directories because multiple dentries are + * not possible(no hardlinks) + * */ void -glusterfs_normalize_dentry (inode_t **parent, char **component, - char *dentry_name) -{ - inode_t *temp_inode = NULL; - dentry_t *dentry = NULL; +inode_find_directory_name (inode_t *inode, const char **name) { + dentry_t *dentry = NULL; - GF_VALIDATE_OR_GOTO ("inode", (*parent), out); - GF_VALIDATE_OR_GOTO ("inode", (*component), out); - GF_VALIDATE_OR_GOTO ("inode", (dentry_name), out); + GF_VALIDATE_OR_GOTO ("inode", inode, out); + GF_VALIDATE_OR_GOTO ("inode", name, out); - /* After this point, there should not be "." or ".." - * in the path. Dot and double dots are replaced with - * appropriate base name and parent inode. - */ + if (!IA_ISDIR (inode->ia_type)) + return; - /* During the resolving, if it goes beyond the mount point - * we do lookup on the mount itself like "/.. " will be - * converted as "/" - */ - if (strcmp (*component, ".") == 0) { - temp_inode = *parent; - *parent = inode_parent (*parent, 0, 0); - inode_parent_null_check (parent, temp_inode, component); - pthread_mutex_lock (&temp_inode->table->lock); - { - dentry = __dentry_search_arbit (temp_inode); - if (dentry) { - snprintf (dentry_name, PATH_MAX, "%s", - dentry->name); - *component = dentry_name; - } - } - pthread_mutex_unlock (&temp_inode->table->lock); - inode_unref (temp_inode); - } else if (strcmp (*component, "..") == 0) { - temp_inode = *parent; - *parent = inode_parent (*parent, 0, 0); - inode_parent_null_check (parent, temp_inode, component); - inode_unref (temp_inode); - - temp_inode = *parent; - *parent = inode_parent (*parent, 0, 0); - inode_parent_null_check (parent, temp_inode, component); - pthread_mutex_lock (&temp_inode->table->lock); - { - dentry = __dentry_search_arbit (temp_inode); - if (dentry) { - snprintf (dentry_name, PATH_MAX, "%s", - dentry->name); - *component = dentry_name; - } + pthread_mutex_lock (&inode->table->lock); + { + dentry = __dentry_search_arbit (inode); + if (dentry) { + *name = dentry->name; } - pthread_mutex_unlock (&temp_inode->table->lock); - inode_unref (temp_inode); } + pthread_mutex_unlock (&inode->table->lock); out: return; } diff --git a/libglusterfs/src/inode.h b/libglusterfs/src/inode.h index e3c38bb..e4ad046 100644 --- a/libglusterfs/src/inode.h +++ b/libglusterfs/src/inode.h @@ -282,21 +282,6 @@ inode_has_dentry (inode_t *inode); size_t inode_ctx_size (inode_t *inode); -/* - * This function is used to change the dentry from a path - * if it contains any "." or ".." . - * - * It replaces "." and ".." to proper bname after resolving - * and will change the component accordingly. - * - * This fucntion also replaces the parent inode based on the - * bname. - * - * We should give a allocated memory as a third argument to store - * the component in case if we are modifying it. - */ - void -glusterfs_normalize_dentry (inode_t **parent, char **component, - char *dentry_name); +inode_find_directory_name (inode_t *inode, const char **name); #endif /* _INODE_H */ -- 1.8.3.1