[PVFS2-developers] patches: st_nlink and rename fixes
Rob Ross
rross at mcs.anl.gov
Thu Jun 30 18:01:37 EDT 2005
Hey,
So this one does add two potential getattrs, right?
Looks ok to me; just making sure that I understand it correctly.
How are you guys testing all this stuff? Are there tests that we can be
running?
Thanks!
Rob
Phil Carns wrote:
>
> dir-nlink.patch
> ---------------------
> This forces the st_nlink field to stay fixed at 2 for all pvfs2
> directories. You can view this field with stat(). The "normal"
> semantic is for st_nlink to provide a count of how many subdirectories
> are within a directory, including "." and "..", but not including files.
> Prior to this patch, pvfs2 tried to maintain this, but the count was
> a little off, and it couldn't keep up with subdirs added or removed by
> other clients anyway. This led to negative st_nlink counts, with some
> possibly odd side effects.
>
> Making st_nlink track the "right" value would be very expensive in pvfs2
> because you might cross meta servers to get the information, and is of
> dubious practical use anyway :) For historical reference, PVFS1 kept
> this value fixed as well, though it went with st_nlink=1 instead of
> st_nlink=2.
>
> dirent-count.patch
> ----------------------
> This adds a new directory attribute field that indicates how many
> entries (of any type) are in a given directory. The server already had
> the ability to gather this information, but it just wasn't being
> reported. A client can read this with getattr(), but it can't be
> modified directly. This may have a couple of uses, one example (used in
> patch below) is to be able to detect quickly if a directory is empty or
> not.
>
> safer-rename.patch
> -----------------------
> This may take a minute to explain, so bear with me :) The summary is
> that the rename() operation in pvfs2 is broken in some cases. For
> example, you can rename over the top of a non-empty directory, and you
> can rename a file to a directory or vice versa. Neither of those should
> be allowed. The core of the problem is that pvfs2 can't easily detect
> those kinds of failures until late in the rename() process, and it
> doesn't have any recovery steps to undo a partial rename once the
> failure is detected. As a side note, most of these problems are an
> issue mainly from the system interface. The kernel vfs adds some sanity
> checking if you access the FS that way.
>
> Rename is the trickiest pvfs2 operation because it requires consistent
> metadata updates on as many as 4 objects, which can be scattered across
> servers: source object, target object, source parent, and target parent.
> So, to get all of this right and avoid races you have a couple of options:
>
> 1) Shared locks. Bad for hopefully obvious reasons.
> 2) Server to server communication to coordinate updates- probably the
> best long term solution, but hard to do cleanly in the near term
> 3) Checking preconditions on the client before starting rename(). This
> leaves a potential race window, but is enough to get the general case
> rename() working.
> 4) Let failure happen late in the rename process, and try to put all the
> objects back in the right place to recover. This has a race like 3),
> but more dangerous.
>
> So... this patch implements option 3. It basically does a getattr on
> both the source and target objects (if applicable, the target is
> optional) before starting a rename(). From this it can check:
> permissions, empti-ness of target if it is a directory, type of both
> objects, etc. It then errors out locally if needed.
>
> I'm interested in seeing 2) happen eventually, but for now this is
> enough to at least make single client rename() operations work safely
> without too much fuss.
>
> -Phil
>
>
>
> ------------------------------------------------------------------------
>
> ---------------------
> PatchSet 64
> Date: 2005/05/20 17:13:40
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> force nlink to remain constant at 2 for all directories. This is not correct
> semantics, but it would be too expensive to keep nlink in sync across
> clients. The previous approach tracking nlink only worked for a single
> client and would lead to bogus counts if multiple clients modified a
> directory.
> [associate:artf15935]
>
> Members:
> src/kernel/linux-2.6/namei.c:1.4->1.5
> src/kernel/linux-2.6/pvfs2-utils.c:1.5->1.6
>
> Index: src/kernel/linux-2.6/namei.c
> diff -u src/kernel/linux-2.6/namei.c:1.4 src/kernel/linux-2.6/namei.c:1.5
> --- src/kernel/linux-2.6/namei.c:1.4 Fri Apr 29 14:27:42 2005
> +++ src/kernel/linux-2.6/namei.c Fri May 20 10:13:40 2005
> @@ -304,7 +304,12 @@
>
> if (inode)
> {
> +#if 0
> + /* NOTE: we have no good way to keep nlink consistent for directories
> + * across clients; keep constant at 2 -Phil
> + */
> dir->i_nlink++;
> +#endif
> pvfs2_update_inode_time(dir);
> ret = 0;
> }
> @@ -321,8 +326,13 @@
> ret = pvfs2_unlink(dir, dentry);
> if (ret == 0)
> {
> - inode->i_nlink--;
> + inode->i_nlink--;
> +#if 0
> + /* NOTE: we have no good way to keep nlink consistent for directories
> + * across clients; keep constant at 2 -Phil
> + */
> dir->i_nlink--;
> +#endif
> pvfs2_update_inode_time(dir);
> }
> return ret;
> @@ -346,6 +356,10 @@
> atomic_read(&new_dentry->d_count));
>
> are_directories = S_ISDIR(old_dentry->d_inode->i_mode);
> +#if 0
> + /* NOTE: we have no good way to keep nlink consistent for directories
> + * across clients; keep constant at 2 -Phil
> + */
> if (are_directories && (new_dir->i_nlink >= PVFS2_LINK_MAX))
> {
> pvfs2_error("pvfs2_rename: directory %s surpassed "
> @@ -353,6 +367,7 @@
> new_dentry->d_name.name);
> return -EMLINK;
> }
> +#endif
>
> new_op = op_alloc();
> if (!new_op)
> @@ -412,16 +427,26 @@
> if (new_dentry->d_inode)
> {
> new_dentry->d_inode->i_ctime = CURRENT_TIME;
> +#if 0
> + /* NOTE: we have no good way to keep nlink consistent for directories
> + * across clients; keep constant at 2 -Phil
> + */
> if (are_directories)
> {
> new_dentry->d_inode->i_nlink--;
> }
> +#endif
> }
> +#if 0
> + /* NOTE: we have no good way to keep nlink consistent for directories
> + * across clients; keep constant at 2 -Phil
> + */
> else if (are_directories)
> {
> new_dir->i_nlink++;
> old_dir->i_nlink--;
> }
> +#endif
>
> error_exit:
> translate_error_if_wait_failed(ret, 0, 0);
> Index: src/kernel/linux-2.6/pvfs2-utils.c
> diff -u src/kernel/linux-2.6/pvfs2-utils.c:1.5 src/kernel/linux-2.6/pvfs2-utils.c:1.6
> --- src/kernel/linux-2.6/pvfs2-utils.c:1.5 Fri Apr 29 13:53:14 2005
> +++ src/kernel/linux-2.6/pvfs2-utils.c Fri May 20 10:13:40 2005
> @@ -174,6 +174,10 @@
> inode->i_mode |= S_IFDIR;
> inode->i_op = &pvfs2_dir_inode_operations;
> inode->i_fop = &pvfs2_dir_operations;
> + /* NOTE: we have no good way to keep nlink consistent for
> + * directories across clients; keep constant at 2 -Phil
> + */
> + inode->i_nlink = 2;
> ret = 0;
> break;
> case PVFS_TYPE_SYMLINK:
>
>
> ------------------------------------------------------------------------
>
> ---------------------
> PatchSet 59
> Date: 2005/05/18 17:20:15
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> Added new "dirent_count" field to attributes structure for directories;
> corresponds to the number of directory entries within a directory. Can be
> read by getattr, but _cannot_ be modified directly with setattr
> [associate:task4192]
>
> Members:
> include/pvfs2-sysint.h:1.1->1.2
> include/pvfs2-types.h:1.4->1.5
> src/client/sysint/sys-getattr.sm:1.1->1.2
> src/proto/pvfs2-attr.h:1.2->1.3
> src/server/get-attr.sm:1.1->1.2
> src/server/pvfs2-server.h:1.2->1.3
>
> Index: include/pvfs2-sysint.h
> diff -u include/pvfs2-sysint.h:1.1 include/pvfs2-sysint.h:1.2
> --- include/pvfs2-sysint.h:1.1 Mon Feb 7 12:58:46 2005
> +++ include/pvfs2-sysint.h Wed May 18 10:20:15 2005
> @@ -42,6 +42,7 @@
> PVFS_size size;
> char *link_target; /* NOTE: caller must free if valid */
> int dfile_count;
> + PVFS_size dirent_count;
> PVFS_ds_type objtype;
> uint32_t mask;
> };
> Index: include/pvfs2-types.h
> diff -u include/pvfs2-types.h:1.4 include/pvfs2-types.h:1.5
> --- include/pvfs2-types.h:1.4 Fri Apr 29 13:53:07 2005
> +++ include/pvfs2-types.h Wed May 18 10:20:15 2005
> @@ -209,10 +209,15 @@
> #define PVFS_ATTR_SYMLNK_TARGET (1 << 18)
> #define PVFS_ATTR_SYMLNK_ALL PVFS_ATTR_SYMLNK_TARGET
>
> +/* internal attribute masks for directory objects */
> +#define PVFS_ATTR_DIR_DIRENT_COUNT (1 << 19)
> +#define PVFS_ATTR_DIR_ALL PVFS_ATTR_DIR_TARGET
> +
> /* attribute masks used by system interface callers */
> #define PVFS_ATTR_SYS_SIZE (1 << 20)
> #define PVFS_ATTR_SYS_LNK_TARGET (1 << 24)
> #define PVFS_ATTR_SYS_DFILE_COUNT (1 << 25)
> +#define PVFS_ATTR_SYS_DIRENT_COUNT (1 << 26)
> #define PVFS_ATTR_SYS_UID PVFS_ATTR_COMMON_UID
> #define PVFS_ATTR_SYS_GID PVFS_ATTR_COMMON_GID
> #define PVFS_ATTR_SYS_PERM PVFS_ATTR_COMMON_PERM
> @@ -222,10 +227,11 @@
> #define PVFS_ATTR_SYS_TYPE PVFS_ATTR_COMMON_TYPE
> #define PVFS_ATTR_SYS_ALL \
> (PVFS_ATTR_COMMON_ALL | PVFS_ATTR_SYS_SIZE | \
> - PVFS_ATTR_SYS_LNK_TARGET | PVFS_ATTR_SYS_DFILE_COUNT)
> + PVFS_ATTR_SYS_LNK_TARGET | PVFS_ATTR_SYS_DFILE_COUNT | \
> + PVFS_ATTR_SYS_DIRENT_COUNT)
> #define PVFS_ATTR_SYS_ALL_NOSIZE \
> (PVFS_ATTR_COMMON_ALL | PVFS_ATTR_SYS_LNK_TARGET | \
> - PVFS_ATTR_SYS_DFILE_COUNT)
> + PVFS_ATTR_SYS_DFILE_COUNT | PVFS_ATTR_SYS_DIRENT_COUNT)
> #define PVFS_ATTR_SYS_ALL_SETABLE \
> (PVFS_ATTR_COMMON_ALL-PVFS_ATTR_COMMON_TYPE)
>
> Index: src/client/sysint/sys-getattr.sm
> diff -u src/client/sysint/sys-getattr.sm:1.1 src/client/sysint/sys-getattr.sm:1.2
> --- src/client/sysint/sys-getattr.sm:1.1 Mon Feb 7 12:57:31 2005
> +++ src/client/sysint/sys-getattr.sm Wed May 18 10:20:18 2005
> @@ -240,6 +253,13 @@
> attrmask |= PVFS_ATTR_SYMLNK_TARGET;
> }
>
> + if (attrmask & PVFS_ATTR_SYS_DIRENT_COUNT)
> + {
> + attrmask &= ~PVFS_ATTR_SYS_DIRENT_COUNT;
> + attrmask |= PVFS_ATTR_DIR_DIRENT_COUNT;
> + }
> +
> +
> gossip_debug(GOSSIP_GETATTR_DEBUG,
> "attrmask being passed to server: ");
> PINT_attrmask_print(GOSSIP_GETATTR_DEBUG, attrmask);
> @@ -348,6 +368,14 @@
> attr->u.meta.dfile_count;
> }
>
> + /* special case for when users ask for dirent count */
> + if ((attr->objtype == PVFS_TYPE_DIRECTORY) &&
> + (sm_p->u.getattr.attrmask & PVFS_ATTR_DIR_DIRENT_COUNT))
> + {
> + sm_p->u.getattr.getattr_resp_p->attr.dirent_count =
> + attr->u.dir.dirent_count;
> + }
> +
> /* copy outgoing sys_attr fields from returned object_attr */
> sm_p->u.getattr.getattr_resp_p->attr.owner = attr->owner;
> sm_p->u.getattr.getattr_resp_p->attr.group = attr->group;
> Index: src/proto/pvfs2-attr.h
> diff -u src/proto/pvfs2-attr.h:1.2 src/proto/pvfs2-attr.h:1.3
> --- src/proto/pvfs2-attr.h:1.2 Tue May 17 12:55:47 2005
> +++ src/proto/pvfs2-attr.h Wed May 18 10:20:20 2005
> @@ -58,9 +58,10 @@
> /* attributes specific to directory objects */
> struct PVFS_directory_attr_s
> {
> - /* undefined */
> + PVFS_size dirent_count;
> };
> typedef struct PVFS_directory_attr_s PVFS_directory_attr;
> +endecode_fields_1(PVFS_directory_attr, PVFS_size, dirent_count)
>
> /* attributes specific to symlinks */
> struct PVFS_symlink_attr_s
> @@ -114,6 +115,8 @@
> encode_PVFS_datafile_attr(pptr, &(x)->u.data); \
> if ((x)->mask & PVFS_ATTR_SYMLNK_TARGET) \
> encode_PVFS_symlink_attr(pptr, &(x)->u.sym); \
> + if ((x)->mask & PVFS_ATTR_DIR_DIRENT_COUNT) \
> + encode_PVFS_directory_attr(pptr, &(x)->u.dir); \
> } while (0)
> #define decode_PVFS_object_attr(pptr,x) do { \
> decode_PVFS_uid(pptr, &(x)->owner); \
> @@ -133,6 +136,8 @@
> decode_PVFS_datafile_attr(pptr, &(x)->u.data); \
> if ((x)->mask & PVFS_ATTR_SYMLNK_TARGET) \
> decode_PVFS_symlink_attr(pptr, &(x)->u.sym); \
> + if ((x)->mask & PVFS_ATTR_DIR_DIRENT_COUNT) \
> + decode_PVFS_directory_attr(pptr, &(x)->u.dir); \
> } while (0)
> #endif
> /* attr buffer needs room for larger of symlink path or meta fields */
> Index: src/server/get-attr.sm
> diff -u src/server/get-attr.sm:1.1 src/server/get-attr.sm:1.2
> --- src/server/get-attr.sm:1.1 Mon Feb 7 12:58:40 2005
> +++ src/server/get-attr.sm Wed May 18 10:20:23 2005
> @@ -2,6 +2,9 @@
> * (C) 2001 Clemson University and The University of Chicago
> *
> * See COPYING in top-level directory.
> + *
> + * Changes by Acxiom Corporation to add dirent_count field to attributes
> + * Copyright © Acxiom Corporation, 2005.
> */
>
> /* pvfs2_get_attr_sm
> @@ -28,7 +31,8 @@
> enum
> {
> STATE_METAFILE = 7,
> - STATE_SYMLINK = 9
> + STATE_SYMLINK = 9,
> + STATE_DIR = 10
> };
>
> static inline char *get_object_type(int objtype)
> @@ -63,6 +67,12 @@
> PINT_server_op *s_op, job_status_s *js_p);
> static int getattr_read_symlink_target(
> PINT_server_op *s_op, job_status_s *js_p);
> +static int getattr_read_dirdata_if_required(
> + PINT_server_op *s_op, job_status_s *js_p);
> +static int getattr_get_dirent_count(
> + PINT_server_op *s_op, job_status_s *js_p);
> +static int getattr_interpret_dirent_count(
> + PINT_server_op *s_op, job_status_s *js_p);
> static int getattr_read_metafile_datafile_handles_if_required(
> PINT_server_op *s_op, job_status_s *js_p);
> static int getattr_read_metafile_distribution_if_required(
> @@ -81,6 +91,9 @@
> read_symlink_target,
> read_metafile_datafile_handles_if_required,
> read_metafile_distribution_if_required,
> + read_dirdata_if_required,
> + get_dirent_count,
> + interpret_dirent_count,
> setup_resp)
> {
> state verify_attribs
> @@ -88,6 +101,7 @@
> run getattr_verify_attribs;
> STATE_SYMLINK => read_symlink_target;
> STATE_METAFILE => read_metafile_datafile_handles_if_required;
> + STATE_DIR => read_dirdata_if_required;
> default => setup_resp;
> }
>
> @@ -110,6 +124,26 @@
> default => setup_resp;
> }
>
> + state read_dirdata_if_required
> + {
> + run getattr_read_dirdata_if_required;
> + success => get_dirent_count;
> + default => setup_resp;
> + }
> +
> + state get_dirent_count
> + {
> + run getattr_get_dirent_count;
> + success => interpret_dirent_count;
> + default => setup_resp;
> + }
> +
> + state interpret_dirent_count
> + {
> + run getattr_interpret_dirent_count;
> + default => setup_resp;
> + }
> +
> state setup_resp
> {
> run getattr_setup_resp;
> @@ -306,11 +340,21 @@
> }
> else if (resp_attr->objtype == PVFS_TYPE_DIRECTORY)
> {
> - gossip_debug(
> - GOSSIP_GETATTR_DEBUG, " handle %Lu refers to "
> - "a directory. doing nothing special\n",
> - Lu(s_op->u.getattr.handle));
> - assert(resp_attr->mask & PVFS_ATTR_COMMON_ALL);
> + if (s_op->u.getattr.attrmask & PVFS_ATTR_DIR_DIRENT_COUNT)
> + {
> + gossip_debug(GOSSIP_GETATTR_DEBUG,
> + " getattr: dirent_count needed.\n");
> + assert(resp_attr->mask & PVFS_ATTR_COMMON_ALL);
> + resp_attr->mask |= PVFS_ATTR_DIR_DIRENT_COUNT;
> + js_p->error_code = STATE_DIR;
> + }
> + else
> + {
> + gossip_debug(GOSSIP_GETATTR_DEBUG,
> + " getattr: dirent_count not needed.\n");
> + js_p->error_code = 0;
> + assert(resp_attr->mask & PVFS_ATTR_COMMON_ALL);
> + }
> }
> else if (resp_attr->objtype == PVFS_TYPE_DIRDATA)
> {
> @@ -664,6 +708,63 @@
> return(1);
> }
>
> +static int getattr_read_dirdata_if_required(
> + PINT_server_op *s_op, job_status_s *js_p)
> +{
> + int ret = -PVFS_EINVAL;
> + job_id_t i;
> +
> + PINT_STATE_DEBUG("read_dirdata_if_required");
> +
> + s_op->key.buffer = Trove_Common_Keys[DIR_ENT_KEY].key;
> + s_op->key.buffer_sz = Trove_Common_Keys[DIR_ENT_KEY].size;
> +
> + s_op->val.buffer = &s_op->u.getattr.dirdata_handle;
> + s_op->val.buffer_sz = sizeof(PVFS_handle);
> +
> + ret = job_trove_keyval_read(
> + s_op->req->u.getattr.fs_id, s_op->req->u.getattr.handle,
> + &s_op->key, &s_op->val, 0, NULL, s_op, 0, js_p, &i,
> + server_job_context);
> +
> + return ret;
> +}
> +
> +static int getattr_get_dirent_count(
> + PINT_server_op *s_op, job_status_s *js_p)
> +{
> + int ret;
> + job_id_t tmp_id;
> +
> + PINT_STATE_DEBUG("get_dirent_count");
> +
> + gossip_debug(GOSSIP_SERVER_DEBUG, " trying to getattr of "
> + "dirdata handle (coll_id = %d, handle = %Lu\n",
> + s_op->u.getattr.fs_id, Lu(s_op->u.getattr.dirdata_handle));
> +
> + ret = job_trove_dspace_getattr(
> + s_op->u.getattr.fs_id, s_op->u.getattr.dirdata_handle, s_op,
> + &s_op->u.getattr.dirdata_ds_attr, 0, js_p, &tmp_id,
> + server_job_context);
> +
> + return ret;
> +}
> +
> +static int getattr_interpret_dirent_count(
> + PINT_server_op *s_op, job_status_s *js_p)
> +{
> + PINT_STATE_DEBUG("interpret_dirent_count");
> +
> + s_op->resp.u.getattr.attr.u.dir.dirent_count =
> + s_op->u.getattr.dirdata_ds_attr.k_size;
> +
> + gossip_debug(GOSSIP_GETATTR_DEBUG, "getattr: dirent_count: %Ld\n",
> + Ld(s_op->resp.u.getattr.attr.u.dir.dirent_count));
> +
> + js_p->error_code = 0;
> + return 1;
> +}
> +
> /*
> * Local variables:
> * mode: c
> Index: src/server/pvfs2-server.h
> diff -u src/server/pvfs2-server.h:1.2 src/server/pvfs2-server.h:1.3
> --- src/server/pvfs2-server.h:1.2 Thu Mar 17 10:52:53 2005
> +++ src/server/pvfs2-server.h Wed May 18 10:20:23 2005
> @@ -255,7 +255,9 @@
> struct PINT_server_getattr_op
> {
> PVFS_handle handle;
> + PVFS_handle dirdata_handle;
> PVFS_fs_id fs_id;
> + PVFS_ds_attributes dirdata_ds_attr;
> uint32_t attrmask;
> };
>
> ---------------------
> PatchSet 61
> Date: 2005/05/18 23:34:39
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> corrected earlier dirent_count patch for getattr; bad #define
> [associate:task4192]
>
> Members:
> include/pvfs2-types.h:1.5->1.6
>
> Index: include/pvfs2-types.h
> diff -u include/pvfs2-types.h:1.5 include/pvfs2-types.h:1.6
> --- include/pvfs2-types.h:1.5 Wed May 18 10:20:15 2005
> +++ include/pvfs2-types.h Wed May 18 16:34:39 2005
> @@ -211,7 +211,7 @@
>
> /* internal attribute masks for directory objects */
> #define PVFS_ATTR_DIR_DIRENT_COUNT (1 << 19)
> -#define PVFS_ATTR_DIR_ALL PVFS_ATTR_DIR_TARGET
> +#define PVFS_ATTR_DIR_ALL PVFS_ATTR_DIR_DIRENT_COUNT
>
> /* attribute masks used by system interface callers */
> #define PVFS_ATTR_SYS_SIZE (1 << 20)
> ---------------------
> PatchSet 62
> Date: 2005/05/20 16:10:08
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> further refinements to getattr dirent_count addition: make sure that it is
> retrieved by default in the attr cache, and that the copy attributes function
> honors it
> [associate:task4192]
>
> Members:
> src/client/sysint/shared-state-methods.c:1.1->1.2
> src/common/misc/pint-util.c:1.6->1.7
>
> Index: src/client/sysint/shared-state-methods.c
> diff -u src/client/sysint/shared-state-methods.c:1.1 src/client/sysint/shared-state-methods.c:1.2
> --- src/client/sysint/shared-state-methods.c:1.1 Mon Feb 7 12:57:29 2005
> +++ src/client/sysint/shared-state-methods.c Fri May 20 09:10:08 2005
> @@ -43,7 +43,7 @@
> *sm_p->cred_p,
> sm_p->parent_ref.fs_id,
> sm_p->parent_ref.handle,
> - PVFS_ATTR_COMMON_ALL);
> + PVFS_ATTR_COMMON_ALL | PVFS_ATTR_DIR_ALL);
>
> sm_p->msgpair.fs_id = sm_p->parent_ref.fs_id;
> sm_p->msgpair.handle = sm_p->parent_ref.handle;
> @@ -96,7 +96,7 @@
> *sm_p->cred_p,
> sm_p->object_ref.fs_id,
> sm_p->object_ref.handle,
> - (PVFS_ATTR_COMMON_ALL | PVFS_ATTR_META_ALL));
> + (PVFS_ATTR_COMMON_ALL | PVFS_ATTR_META_ALL | PVFS_ATTR_DIR_ALL));
>
> sm_p->msgpair.fs_id = sm_p->object_ref.fs_id;
> sm_p->msgpair.handle = sm_p->object_ref.handle;
> Index: src/common/misc/pint-util.c
> diff -u src/common/misc/pint-util.c:1.6 src/common/misc/pint-util.c:1.7
> --- src/common/misc/pint-util.c:1.6 Wed May 11 14:44:26 2005
> +++ src/common/misc/pint-util.c Fri May 20 09:10:10 2005
> @@ -127,6 +127,12 @@
> dest->objtype = src->objtype;
> }
>
> + if (src->mask & PVFS_ATTR_DIR_DIRENT_COUNT)
> + {
> + dest->u.dir.dirent_count =
> + src->u.dir.dirent_count;
> + }
> +
> /*
> NOTE:
> we only copy the size out if we're actually a
>
>
> ------------------------------------------------------------------------
>
> ---------------------
> PatchSet 63
> Date: 2005/05/20 16:27:00
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> several changes to sys_rename:
> - fixed bug in which error on lookup up src directory could be ignored if
> lookup on dest directory was successful
> - added getattr of src and dest in cases where both exist. This allows
> safety checks for:
> - type match between src and dest
> - directory empty test for dest (if directory)
> - fsid match between src and dest
> [associate:artf15934]
>
> Members:
> src/client/sysint/client-state-machine.h:1.1->1.2
> src/client/sysint/sys-rename.sm:1.1->1.2
>
> Index: src/client/sysint/client-state-machine.h
> diff -u src/client/sysint/client-state-machine.h:1.1 src/client/sysint/client-state-machine.h:1.2
> --- src/client/sysint/client-state-machine.h:1.1 Mon Feb 7 12:57:26 2005
> +++ src/client/sysint/client-state-machine.h Fri May 20 09:27:00 2005
> @@ -270,6 +274,7 @@
> PVFS_object_ref parent_refns[2]; /* old/new input parent refns */
>
> PVFS_object_ref refns[2]; /* old/new object refns */
> + PVFS_ds_type types[2]; /* old/new object types */
> int retry_count;
> int stored_error_code;
> int rmdirent_index;
> Index: src/client/sysint/sys-rename.sm
> diff -u src/client/sysint/sys-rename.sm:1.1 src/client/sysint/sys-rename.sm:1.2
> --- src/client/sysint/sys-rename.sm:1.1 Mon Feb 7 12:57:36 2005
> +++ src/client/sysint/sys-rename.sm Fri May 20 09:27:00 2005
> @@ -56,6 +63,8 @@
> PINT_client_sm *sm_p, job_status_s *js_p);
> static int rename_rmdirent_retry_or_fail(
> PINT_client_sm *sm_p, job_status_s *js_p);
> +static int rename_getattr_src_interpret(
> + PINT_client_sm *sm_p, job_status_s *js_p);
>
> static int rename_lookups_comp_fn(
> void *v_p, struct PVFS_server_resp *resp_p, int index);
> @@ -73,6 +82,13 @@
> rename_lookups_setup_msgpair_array,
> rename_lookups_xfer_msgpair_array,
> rename_lookups_failure,
> + rename_getattr_src_setup,
> + rename_getattr_src_xfer,
> + rename_getattr_src_interpret,
> + rename_getattr_dest_setup,
> + rename_getattr_dest_xfer,
> + rename_getattr_dest_interpret,
> + rename_getattr_cleanup,
> rename_crdirent_setup_msgpair,
> rename_crdirent_xfer_msgpair,
> rename_crdirent_retry_or_fail,
> @@ -107,10 +123,50 @@
> {
> jump pvfs2_msgpairarray_sm;
> success => rename_crdirent_setup_msgpair;
> - RENAME_CHDIRENT => rename_chdirent_setup_msgpair;
> + RENAME_CHDIRENT => rename_getattr_src_setup;
> default => rename_lookups_failure;
> }
>
> + state rename_getattr_src_setup
> + {
> + run PINT_sm_common_object_getattr_setup_msgpair;
> + success => rename_getattr_src_xfer;
> + default => rename_getattr_cleanup;
> + }
> +
> + state rename_getattr_src_xfer
> + {
> + jump pvfs2_client_getattr_acache_sm;
> + success => rename_getattr_src_interpret;
> + default => rename_getattr_cleanup;
> + }
> +
> + state rename_getattr_src_interpret
> + {
> + run rename_getattr_src_interpret;
> + default => rename_getattr_dest_setup;
> + }
> +
> + state rename_getattr_dest_setup
> + {
> + run PINT_sm_common_object_getattr_setup_msgpair;
> + success => rename_getattr_dest_xfer;
> + default => rename_getattr_cleanup;
> + }
> +
> + state rename_getattr_dest_xfer
> + {
> + jump pvfs2_client_getattr_acache_sm;
> + success => rename_chdirent_setup_msgpair;
> + default => rename_getattr_cleanup;
> + }
> +
> + state rename_getattr_cleanup
> + {
> + run PINT_sm_common_object_getattr_failure;
> + default => cleanup;
> + }
> +
> state rename_lookups_failure
> {
> run rename_lookups_failure;
> @@ -411,7 +479,19 @@
> properly verified before changing anything
> */
> sm_p->u.rename.target_dirent_exists = 1;
> - return RENAME_CHDIRENT;
> + /* set fs_id and handle for getattr nested sm */
> + sm_p->object_ref = sm_p->u.rename.refns[0];
> +
> + if(sm_p->msgarray[0].op_status == 0)
> + {
> + /* if both lookups succeeded, then we need chdirent */
> + return RENAME_CHDIRENT;
> + }
> + else
> + {
> + /* if the first one failed, maintain its error code */
> + return(sm_p->msgarray[0].op_status);
> + }
> }
> return 0;
> }
> @@ -686,10 +766,63 @@
> {
> int ret = -PVFS_EINVAL;
> PINT_sm_msgpair_state *msg_p = NULL;
> + PVFS_object_attr *attr = NULL;
>
> gossip_debug(GOSSIP_CLIENT_DEBUG, "rename state: "
> "rename_chdirent_setup_msgpair\n");
>
> + /* look at the result of the dest getattr and make sure everything looks
> + * ok before we continue
> + */
> + attr = (sm_p->acache_hit ?
> + &sm_p->pinode->attr :
> + &sm_p->acache_attr);
> + assert(attr);
> + sm_p->u.rename.types[1] = attr->objtype;
> +
> + gossip_debug(GOSSIP_CLIENT_DEBUG,
> + "rename dest type: %d\n", attr->objtype);
> + if(attr->objtype == PVFS_TYPE_DIRECTORY)
> + {
> + gossip_debug(GOSSIP_CLIENT_DEBUG,
> + "rename dest dirent_count: %Ld\n",
> + Ld(attr->u.dir.dirent_count));
> + gossip_debug(GOSSIP_CLIENT_DEBUG,
> + "rename dest attr mask: %d\n", (int)attr->mask);
> + }
> +
> + /* if the destination is a directory, is it empty? */
> + if(attr->objtype == PVFS_TYPE_DIRECTORY && attr->u.dir.dirent_count != 0)
> + {
> + js_p->error_code = -PVFS_ENOTEMPTY;
> + return(1);
> + }
> +
> + /* do the types match? */
> + if(sm_p->u.rename.types[0] != sm_p->u.rename.types[1])
> + {
> + if(sm_p->u.rename.types[1] == PVFS_TYPE_DIRECTORY)
> + {
> + js_p->error_code = -PVFS_EISDIR;
> + return(1);
> + }
> + if(sm_p->u.rename.types[1] == PVFS_TYPE_DIRECTORY)
> + {
> + js_p->error_code = -PVFS_ENOTDIR;
> + return(1);
> + }
> + /* TODO: what about other cases? */
> + js_p->error_code = -PVFS_EINVAL;
> + return(1);
> + }
> +
> + /* do the fsid's match? */
> + if(sm_p->u.rename.refns[0].fs_id != sm_p->u.rename.refns[1].fs_id)
> + {
> + js_p->error_code = -PVFS_ENODEV;
> + return(1);
> + }
> +
> js_p->error_code = 0;
>
> gossip_debug(GOSSIP_CLIENT_DEBUG," rename: posting chdirent req\n");
> @@ -863,6 +996,31 @@
> return 1;
> }
>
> +static int rename_getattr_src_interpret(
> + PINT_client_sm *sm_p, job_status_s *js_p)
> +{
> + PVFS_object_attr *attr = NULL;
> +
> + gossip_debug(GOSSIP_CLIENT_DEBUG,
> + "rename state: getattr_src_interpret\n");
> +
> + attr = (sm_p->acache_hit ?
> + &sm_p->pinode->attr :
> + &sm_p->acache_attr);
> +
> + assert(attr);
> +
> + sm_p->u.rename.types[0] = attr->objtype;
> + gossip_debug(GOSSIP_CLIENT_DEBUG,
> + "rename src type: %d\n", attr->objtype);
> +
> + /* setup for destination getattr */
> + sm_p->object_ref = sm_p->u.rename.refns[1];
> +
> + js_p->error_code = 0;
> + return(1);
> +}
> +
> /*
> * Local variables:
> * mode: c
> ---------------------
> PatchSet 87
> Date: 2005/06/13 22:14:36
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> added copyright notice for sys-rename() changes
> [associate:artf15934]
>
> Members:
> src/client/sysint/sys-rename.sm:1.2->1.3
>
> Index: src/client/sysint/sys-rename.sm
> diff -u src/client/sysint/sys-rename.sm:1.2 src/client/sysint/sys-rename.sm:1.3
> --- src/client/sysint/sys-rename.sm:1.2 Fri May 20 09:27:00 2005
> +++ src/client/sysint/sys-rename.sm Mon Jun 13 15:14:36 2005
> @@ -1,6 +1,9 @@
> /*
> * (C) 2003 Clemson University and The University of Chicago
> *
> + * Changes by Acxiom Corporation to add extra safety checks via getattr
> + * Copyright © Acxiom Corporation, 2005.
> + *
> * See COPYING in top-level directory.
> */
>
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> PVFS2-developers mailing list
> PVFS2-developers at beowulf-underground.org
> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
More information about the PVFS2-developers
mailing list