From c8d5aa14e50be2a85491783f169a8f4e646b9594 Mon Sep 17 00:00:00 2001 From: wangdi Date: Sat, 30 Nov 2013 10:13:40 -0800 Subject: [PATCH] LU-2804 mdd: move ACL mode handling to MDD Move ACL mode handling from OSD to MDD, so both ldiskfs and zfs can be set mode correctly, and also by this, it can avoid to transfer the local mask to the remote MDT for fixing the mode. Move sanity 103 out of sanity SLOW list, so it can be run in the normal review/landing check, so to avoid regression about ACL. And also it only take 15 seconds in my local VM run, which probably not be put to the SLOW list at all. Signed-off-by: wang di Change-Id: I73146425baa7d8b712ce46e18955ecaa2a3fd9a4 Reviewed-on: http://review.whamcloud.com/5421 Tested-by: Hudson Reviewed-by: Alex Zhuravlev Reviewed-by: Lai Siyao Reviewed-by: Nathaniel Clark Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/mdd/mdd_dir.c | 100 ++++++++++++++++++++++++++------------- lustre/mdd/mdd_internal.h | 27 +++-------- lustre/mdd/mdd_object.c | 2 +- lustre/mdd/mdd_permission.c | 65 ++++++++----------------- lustre/osd-ldiskfs/osd_handler.c | 11 ++++- lustre/tests/acl/2561.test | 2 +- lustre/tests/acl/974_remote.test | 21 ++++++++ lustre/tests/sanity.sh | 10 ++-- 8 files changed, 135 insertions(+), 103 deletions(-) create mode 100644 lustre/tests/acl/974_remote.test diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c index 9a9d26a..c383d4f 100644 --- a/lustre/mdd/mdd_dir.c +++ b/lustre/mdd/mdd_dir.c @@ -1480,7 +1480,7 @@ static int mdd_declare_object_initialize(const struct lu_env *env, int mdd_object_initialize(const struct lu_env *env, const struct lu_fid *pfid, const struct lu_name *lname, struct mdd_object *child, - struct lu_attr *attr, struct thandle *handle, + const struct lu_attr *attr, struct thandle *handle, const struct md_op_spec *spec) { int rc; @@ -1494,20 +1494,8 @@ int mdd_object_initialize(const struct lu_env *env, const struct lu_fid *pfid, * (2) maybe, the child attributes should be set in OSD when creation. */ - /* - * inode mode has been set in creation time, and it's based on umask, - * la_mode and acl, don't set here again! (which will go wrong - * because below function doesn't consider umask). - * I'd suggest set all object attributes in creation time, see above. - */ - LASSERT(attr->la_valid & (LA_MODE | LA_TYPE)); - attr->la_valid &= ~(LA_MODE | LA_TYPE); rc = mdd_attr_set_internal(env, child, attr, handle, 0); /* arguments are supposed to stay the same */ - attr->la_valid |= LA_MODE | LA_TYPE; - if (rc != 0) - RETURN(rc); - if (S_ISDIR(attr->la_mode)) { /* Add "." and ".." for newly created dir */ mdo_ref_add(env, child, handle); @@ -1699,6 +1687,38 @@ out: return rc; } +static int mdd_acl_init(const struct lu_env *env, struct mdd_object *pobj, + struct lu_attr *la, struct lu_buf *acl_buf, + int *got_def_acl, int *reset_acl) +{ + int rc; + ENTRY; + + if (S_ISLNK(la->la_mode)) + RETURN(0); + + mdd_read_lock(env, pobj, MOR_TGT_PARENT); + rc = mdo_xattr_get(env, pobj, acl_buf, + XATTR_NAME_ACL_DEFAULT, BYPASS_CAPA); + mdd_read_unlock(env, pobj); + if (rc > 0) { + /* If there are default ACL, fix mode by default ACL */ + *got_def_acl = rc; + acl_buf->lb_len = rc; + rc = __mdd_fix_mode_acl(env, acl_buf, &la->la_mode); + if (rc < 0) + RETURN(rc); + *reset_acl = rc; + } else if (rc == -ENODATA || rc == -EOPNOTSUPP) { + /* If there are no default ACL, fix mode by mask */ + struct lu_ucred *uc = lu_ucred(env); + la->la_mode &= ~uc->uc_umask; + rc = 0; + } + + RETURN(rc); +} + /* * Create object and insert it into namespace. */ @@ -1714,10 +1734,12 @@ static int mdd_create(const struct lu_env *env, struct md_object *pobj, struct lu_attr *attr = &ma->ma_attr; struct thandle *handle; struct lu_attr *pattr = &info->mti_pattr; + struct lu_buf acl_buf; struct dynlock_handle *dlh; const char *name = lname->ln_name; int rc, created = 0, initialized = 0, inserted = 0; int got_def_acl = 0; + int reset_acl = 0; ENTRY; /* @@ -1768,20 +1790,12 @@ static int mdd_create(const struct lu_env *env, struct md_object *pobj, if (OBD_FAIL_CHECK(OBD_FAIL_MDS_DQACQ_NET)) GOTO(out_free, rc = -EINPROGRESS); - if (!S_ISLNK(attr->la_mode)) { - struct lu_buf *acl_buf; - - acl_buf = mdd_buf_get(env, info->mti_xattr_buf, - sizeof(info->mti_xattr_buf)); - mdd_read_lock(env, mdd_pobj, MOR_TGT_PARENT); - rc = mdo_xattr_get(env, mdd_pobj, acl_buf, - XATTR_NAME_ACL_DEFAULT, BYPASS_CAPA); - mdd_read_unlock(env, mdd_pobj); - if (rc > 0) - got_def_acl = rc; - else if (rc < 0 && rc != -EOPNOTSUPP && rc != -ENODATA) - GOTO(out_free, rc); - } + acl_buf.lb_buf = info->mti_xattr_buf; + acl_buf.lb_len = sizeof(info->mti_xattr_buf); + rc = mdd_acl_init(env, mdd_pobj, attr, &acl_buf, &got_def_acl, + &reset_acl); + if (rc < 0) + GOTO(out_free, rc); mdd_object_make_hint(env, mdd_pobj, son, attr); @@ -1813,13 +1827,33 @@ static int mdd_create(const struct lu_env *env, struct md_object *pobj, #ifdef CONFIG_FS_POSIX_ACL if (got_def_acl) { - struct lu_buf *acl_buf; + /* set default acl */ + if (S_ISDIR(attr->la_mode)) { + LASSERTF(acl_buf.lb_len == got_def_acl, + "invalid acl_buf: %p:%d got_def %d\n", + acl_buf.lb_buf, (int)acl_buf.lb_len, + got_def_acl); + rc = mdo_xattr_set(env, son, &acl_buf, + XATTR_NAME_ACL_DEFAULT, 0, + handle, BYPASS_CAPA); + if (rc) { + mdd_write_unlock(env, son); + GOTO(cleanup, rc); + } + } - acl_buf = mdd_buf_get(env, info->mti_xattr_buf, got_def_acl); - rc = __mdd_acl_init(env, son, acl_buf, &attr->la_mode, handle); - if (rc) { - mdd_write_unlock(env, son); - GOTO(cleanup, rc); + /* set its own acl */ + if (reset_acl) { + LASSERTF(acl_buf.lb_buf != NULL && acl_buf.lb_len != 0, + "invalid acl_buf %p:%d\n", acl_buf.lb_buf, + (int)acl_buf.lb_len); + rc = mdo_xattr_set(env, son, &acl_buf, + XATTR_NAME_ACL_ACCESS, + 0, handle, BYPASS_CAPA); + if (rc) { + mdd_write_unlock(env, son); + GOTO(cleanup, rc); + } } } #endif diff --git a/lustre/mdd/mdd_internal.h b/lustre/mdd/mdd_internal.h index 97f6cb6..d72737d 100644 --- a/lustre/mdd/mdd_internal.h +++ b/lustre/mdd/mdd_internal.h @@ -240,10 +240,10 @@ int mdd_attr_get(const struct lu_env *env, struct md_object *obj, int mdd_attr_set(const struct lu_env *env, struct md_object *obj, const struct md_attr *ma); int mdd_attr_set_internal(const struct lu_env *env, - struct mdd_object *obj, - struct lu_attr *attr, - struct thandle *handle, - int needacl); + struct mdd_object *obj, + const struct lu_attr *attr, + struct thandle *handle, + int needacl); int mdd_attr_check_set_internal(const struct lu_env *env, struct mdd_object *obj, struct lu_attr *attr, @@ -304,7 +304,7 @@ int mdd_finish_unlink(const struct lu_env *env, struct mdd_object *obj, struct md_attr *ma, struct thandle *th); int mdd_object_initialize(const struct lu_env *env, const struct lu_fid *pfid, const struct lu_name *lname, struct mdd_object *child, - struct lu_attr *attr, struct thandle *handle, + const struct lu_attr *attr, struct thandle *handle, const struct md_op_spec *spec); int mdd_link_sanity_check(const struct lu_env *env, struct mdd_object *tgt_obj, const struct lu_name *lname, struct mdd_object *src_obj); @@ -469,8 +469,8 @@ int __mdd_declare_acl_init(const struct lu_env *env, struct mdd_object *obj, int is_dir, struct thandle *handle); int mdd_acl_set(const struct lu_env *env, struct mdd_object *obj, const struct lu_buf *buf, int fl); -int __mdd_acl_init(const struct lu_env *env, struct mdd_object *obj, - struct lu_buf *buf, __u32 *mode, struct thandle *handle); +int __mdd_fix_mode_acl(const struct lu_env *env, struct lu_buf *buf, + __u32 *mode); int __mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, struct lu_attr *la, int mask, int role); int mdd_permission(const struct lu_env *env, @@ -865,23 +865,10 @@ int mdo_create_obj(const struct lu_env *env, struct mdd_object *o, struct thandle *handle) { struct dt_object *next = mdd_object_child(o); - struct lu_ucred *uc = lu_ucred(env); - __u32 saved; int rc; - /* - * LU-974 enforce client umask in creation. - * TODO: CMD needs to handle this for remote object. - */ - if (likely(uc != NULL)) - saved = xchg(¤t->fs->umask, uc->uc_umask & S_IRWXUGO); - rc = next->do_ops->do_create(env, next, attr, hint, dof, handle); - /* restore previous umask value */ - if (likely(uc != NULL)) - current->fs->umask = saved; - return rc; } diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index 5844799..fe796c2 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -696,7 +696,7 @@ static inline int mdd_attr_check(const struct lu_env *env, } int mdd_attr_set_internal(const struct lu_env *env, struct mdd_object *obj, - struct lu_attr *attr, struct thandle *handle, + const struct lu_attr *attr, struct thandle *handle, int needacl) { int rc; diff --git a/lustre/mdd/mdd_permission.c b/lustre/mdd/mdd_permission.c index 86e779e..cea5d3b 100644 --- a/lustre/mdd/mdd_permission.c +++ b/lustre/mdd/mdd_permission.c @@ -173,57 +173,34 @@ stop: RETURN(rc); } -/* - * Hold write_lock for obj. - */ -int __mdd_acl_init(const struct lu_env *env, struct mdd_object *obj, - struct lu_buf *buf, __u32 *mode, struct thandle *handle) +/** + * Fix mode and ACL according to the default ACL(buf) + * \retval = 0 ACL does not need to be reset. + * \retval = 1 ACL needs to be reset. + * \retval < 0 error. + **/ +int __mdd_fix_mode_acl(const struct lu_env *env, struct lu_buf *buf, + __u32 *mode) { - posix_acl_xattr_header *head; - posix_acl_xattr_entry *entry; - int entry_count; - __u32 old = *mode; - int rc, rc2; - - ENTRY; + posix_acl_xattr_header *head; + posix_acl_xattr_entry *entry; + int entry_count; + int rc; - head = (posix_acl_xattr_header *)(buf->lb_buf); - entry = head->a_entries; - entry_count = (buf->lb_len - sizeof(head->a_version)) / - sizeof(posix_acl_xattr_entry); - if (entry_count <= 0) - RETURN(0); + ENTRY; - if (S_ISDIR(*mode)) { - rc = mdo_xattr_set(env, obj, buf, XATTR_NAME_ACL_DEFAULT, 0, - handle, BYPASS_CAPA); - if (rc) - RETURN(rc); - } + head = (posix_acl_xattr_header *)(buf->lb_buf); + entry = head->a_entries; + entry_count = (buf->lb_len - sizeof(head->a_version)) / + sizeof(posix_acl_xattr_entry); + if (entry_count <= 0) + RETURN(0); - rc = lustre_posix_acl_create_masq(entry, mode, entry_count); - if (rc < 0) - RETURN(rc); - - /* part of ACL went into i_mode */ - if (*mode != old) { - struct mdd_thread_info *info = mdd_env_info(env); - struct lu_attr *pattr = &info->mti_pattr; - - /* mode was initialized within object creation, - * so we need explict ->attr_set() to update it */ - pattr->la_valid = LA_MODE; - pattr->la_mode = *mode; - rc2 = mdo_attr_set(env, obj, pattr, handle, BYPASS_CAPA); - if (rc2 < 0) - rc = rc2; - } + rc = lustre_posix_acl_create_masq(entry, mode, entry_count); - if (rc > 0) - rc = mdo_xattr_set(env, obj, buf, XATTR_NAME_ACL_ACCESS, 0, - handle, BYPASS_CAPA); RETURN(rc); } + #endif /* diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index 9932320..7374b06 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -1947,6 +1947,11 @@ static int __osd_object_create(struct osd_thread_info *info, struct thandle *th) { int result; + __u32 umask; + + /* we drop umask so that permissions we pass are not affected */ + umask = current->fs->umask; + current->fs->umask = 0; result = osd_create_type_f(dof->dof_type)(info, obj, attr, hint, dof, th); @@ -1958,7 +1963,10 @@ static int __osd_object_create(struct osd_thread_info *info, unlock_new_inode(obj->oo_inode); } - return result; + /* restore previous umask value */ + current->fs->umask = umask; + + return result; } /** @@ -2631,6 +2639,7 @@ static int osd_xattr_set(const struct lu_env *env, struct dt_object *dt, struct inode *inode = obj->oo_inode; struct osd_thread_info *info = osd_oti_get(env); int fs_flags = 0; + ENTRY; LASSERT(handle != NULL); diff --git a/lustre/tests/acl/2561.test b/lustre/tests/acl/2561.test index b112dba..67de760 100644 --- a/lustre/tests/acl/2561.test +++ b/lustre/tests/acl/2561.test @@ -1,6 +1,6 @@ LU-2561 newly created file is same size as directory - $ mkdir 2561 + $ mkdir -p 2561 $ cd 2561 $ getfacl --access . | setfacl -d -M- . $ touch f1 diff --git a/lustre/tests/acl/974_remote.test b/lustre/tests/acl/974_remote.test new file mode 100644 index 0000000..cea27c6 --- /dev/null +++ b/lustre/tests/acl/974_remote.test @@ -0,0 +1,21 @@ +LU-974 ignore umask when default acl with mask is set + + $ umask 022 + $ lfs mkdir -i 1 974 + + $ touch 974/f1 + $ ls -dl 974/f1 | awk '{ print $1 }' + > -rw-r--r-- + + $ setfacl -R -d -m mask:007 974 + $ touch 974/f2 + $ ls -dl 974/f2 | awk '{ print $1 }' + > -rw-rw-r--+ + + $ umask 077 + $ touch f3 + $ ls -dl f3 | awk '{ print $1 }' + > -rw------- + + $ rm -rf 974 + diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 6ab66ad..e9753ca 100644 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -58,7 +58,7 @@ init_test_env $@ . ${CONFIG:=$LUSTRE/tests/cfg/${NAME}.sh} init_logging -[ "$SLOW" = "no" ] && EXCEPT_SLOW="24o 24v 27m 36f 36g 36h 51b 60c 63 64b 68 71 73 77f 78 101a 103 115 120g 124b" +[ "$SLOW" = "no" ] && EXCEPT_SLOW="24o 24v 27m 36f 36g 36h 51b 60c 63 64b 68 71 73 77f 78 101a 115 120g 124b" [ $(facet_fstype $SINGLEMDS) = "zfs" ] && # bug number for skipped test: LU-2834 LU-1593 LU-2610 LU-2833 LU-1957 LU-2805 @@ -6164,8 +6164,12 @@ test_103 () { run_acl_subtest inheritance || error "inheritance test failed" rm -f make-tree - echo "LU-974 ignore umask when acl is enabled..." - run_acl_subtest 974 || error "LU-974 test failed" + echo "LU-974 ignore umask when acl is enabled..." + run_acl_subtest 974 || error "LU-974 test failed" + if [ $MDSCOUNT -ge 2 ]; then + run_acl_subtest 974_remote || + error "LU-974 test failed under remote dir" + fi echo "LU-2561 newly created file is same size as directory..." run_acl_subtest 2561 || error "LU-2561 test failed" -- 1.8.3.1