From aa92caa21fa2a4473dce5889de7fcd17e171c1a0 Mon Sep 17 00:00:00 2001 From: Mikhail Pershin Date: Sun, 28 Feb 2021 12:24:12 +0300 Subject: [PATCH] LU-14430 mdt: fix maximum ACL handling Having maximum ACL cause big reply buffer and in that case server could return -ERANGE in mdt_pack_acl2body() expecting a client to resend RPC with bigger buffer. The problem is that even in that case server can return -ERANGE causing userspace tool to get this error after all. Instead of estimating reply sizes in mdt_pack_acl2body() let's just rely on mdt_fix_reply() code which does buffer grow when it is needed - add more credits for osd_create in ldiskfs because it copies also default ACLs during create - remove code returning -ERANGE in mdt_pack_acl2body() and rely on mdt_fix_reply() reply buffers grow - test is added to create as many ACLs as possible Test-Parameters: env=ONLY=103e testlist=sanity Signed-off-by: Mikhail Pershin Change-Id: If7af5c61f89ee1220d7982d4c61a7357051a811c Reviewed-on: https://review.whamcloud.com/42013 Tested-by: jenkins Reviewed-by: Andreas Dilger Tested-by: Maloo Reviewed-by: Alex Zhuravlev --- lustre/include/dt_object.h | 1 + lustre/mdd/mdd_dir.c | 5 ++++ lustre/mdt/mdt_handler.c | 50 ++++++++-------------------------- lustre/osd-ldiskfs/osd_handler.c | 22 ++++++++++++--- lustre/tests/sanity.sh | 59 ++++++++++++++++++++++++++++++++++------ 5 files changed, 86 insertions(+), 51 deletions(-) diff --git a/lustre/include/dt_object.h b/lustre/include/dt_object.h index b730151..d36e1c7 100644 --- a/lustre/include/dt_object.h +++ b/lustre/include/dt_object.h @@ -388,6 +388,7 @@ struct dt_allocation_hint { struct dt_object *dah_parent; const void *dah_eadata; int dah_eadata_len; + int dah_acl_len; __u32 dah_mode; int dah_append_stripes; bool dah_can_block; diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c index fe0c927..a81de64 100644 --- a/lustre/mdd/mdd_dir.c +++ b/lustre/mdd/mdd_dir.c @@ -2148,6 +2148,11 @@ static int mdd_declare_create_object(const struct lu_env *env, const struct lu_buf *buf; int rc; +#ifdef CONFIG_LUSTRE_FS_POSIX_ACL + /* ldiskfs OSD needs this information for credit allocation */ + if (def_acl_buf) + hint->dah_acl_len = def_acl_buf->lb_len; +#endif rc = mdd_declare_create_object_internal(env, p, c, attr, handle, spec, hint); if (rc) diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index 6f404db..fa160bc 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -657,6 +657,7 @@ int mdt_pack_acl2body(struct mdt_thread_info *info, struct mdt_body *repbody, if (buf->lb_len == 0) RETURN(0); + LASSERT(!info->mti_big_acl_used); again: rc = mo_xattr_get(env, next, buf, XATTR_NAME_ACL_ACCESS); if (rc < 0) { @@ -666,10 +667,9 @@ again: rc = 0; } else if (rc == -EOPNOTSUPP) { rc = 0; - } else { - if (rc == -ERANGE && - exp_connect_large_acl(info->mti_exp) && - buf->lb_buf != info->mti_big_acl) { + } else if (rc == -ERANGE) { + if (exp_connect_large_acl(info->mti_exp) && + !info->mti_big_acl_used) { if (info->mti_big_acl == NULL) { info->mti_big_aclsize = min_t(unsigned int, @@ -695,47 +695,19 @@ again: buf->lb_buf = info->mti_big_acl; buf->lb_len = info->mti_big_aclsize; - + info->mti_big_acl_used = 1; goto again; } - + /* FS has ACL bigger that our limits */ + CDEBUG(D_INODE, "%s: "DFID" ACL can't fit into %d\n", + mdt_obd_name(mdt), PFID(mdt_object_fid(o)), + info->mti_big_aclsize); + rc = -E2BIG; + } else { CERROR("%s: unable to read "DFID" ACL: rc = %d\n", mdt_obd_name(mdt), PFID(mdt_object_fid(o)), rc); } } else { - int client; - int server; - int acl_buflen; - int lmm_buflen = 0; - int lmmsize = 0; - - acl_buflen = req_capsule_get_size(pill, &RMF_ACL, RCL_SERVER); - if (acl_buflen >= rc) - goto map; - - /* If LOV/LMA EA is small, we can reuse part of their buffer */ - client = ptlrpc_req_get_repsize(pill->rc_req); - server = lustre_packed_msg_size(pill->rc_req->rq_repmsg); - if (req_capsule_has_field(pill, &RMF_MDT_MD, RCL_SERVER)) { - lmm_buflen = req_capsule_get_size(pill, &RMF_MDT_MD, - RCL_SERVER); - lmmsize = repbody->mbo_eadatasize; - } - - if (client < server - acl_buflen - lmm_buflen + rc + lmmsize) { - CDEBUG(D_INODE, "%s: client prepared buffer size %d " - "is not big enough with the ACL size %d (%d)\n", - mdt_obd_name(mdt), client, rc, - server - acl_buflen - lmm_buflen + rc + lmmsize); - repbody->mbo_aclsize = 0; - repbody->mbo_valid &= ~OBD_MD_FLACL; - RETURN(-ERANGE); - } - -map: - if (buf->lb_buf == info->mti_big_acl) - info->mti_big_acl_used = 1; - rc = nodemap_map_acl(nodemap, buf->lb_buf, rc, NODEMAP_FS_TO_CLIENT); /* if all ACLs mapped out, rc is still >= 0 */ diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index fa80fde..03dee69 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -3435,7 +3435,6 @@ static osd_obj_type_f osd_create_type_f(enum dt_format_type type) return result; } - static void osd_ah_init(const struct lu_env *env, struct dt_allocation_hint *ah, struct dt_object *parent, struct dt_object *child, umode_t child_mode) @@ -3614,6 +3613,8 @@ static int osd_declare_create(const struct lu_env *env, struct dt_object *dt, struct thandle *handle) { struct osd_thandle *oh; + struct super_block *sb = osd_sb(osd_dev(dt->do_lu.lo_dev)); + int credits; int rc; ENTRY; @@ -3628,10 +3629,23 @@ static int osd_declare_create(const struct lu_env *env, struct dt_object *dt, * vs. osd_mkreg: osd_mk_index will create 2 blocks for root_node and * leaf_node, could involves the block, block bitmap, groups, GDT * change for each block, so add 4 * 2 credits in that case. + * + * The default ACL initialization may consume an additional 16 blocks */ - osd_trans_declare_op(env, oh, OSD_OT_CREATE, - osd_dto_credits_noquota[DTO_OBJECT_CREATE] + - (dof->dof_type == DFT_INDEX) ? 4 * 2 : 0); + credits = osd_dto_credits_noquota[DTO_OBJECT_CREATE] + + ((dof->dof_type == DFT_INDEX) ? 4 * 2 : 0); + + /** + * While ldiskfs_new_inode() calls ldiskfs_init_acl() we have to add + * credits for possible default ACL creation in new inode + */ + if (hint && hint->dah_acl_len) + credits += osd_calc_bkmap_credits(sb, NULL, 0, -1, + (hint->dah_acl_len + sb->s_blocksize - 1) >> + sb->s_blocksize_bits); + + osd_trans_declare_op(env, oh, OSD_OT_CREATE, credits); + /* * Reuse idle OI block may cause additional one OI block * to be changed. diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 695d4ce..c4e305d 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -10805,17 +10805,60 @@ test_103c() { run_test 103c "'cp -rp' won't set empty acl" test_103e() { - (( $MDS1_VERSION >= $(version_code 2.13.59) )) || - skip "MDS needs to be at least 2.13.59" + local numacl + local fileacl + local saved_debug=$($LCTL get_param -n debug) + + (( $MDS1_VERSION >= $(version_code 2.14.0) )) || + skip "MDS needs to be at least 2.14.0" + + large_xattr_enabled || skip_env "ea_inode feature disabled" mkdir -p $DIR/$tdir - # one default ACL will be created for the file owner - for U in {2..256}; do - setfacl -m default:user:$U:rwx $DIR/$tdir - numacl=$(getfacl $DIR/$tdir |& grep -c "default:user") - touch $DIR/$tdir/$tfile.$U || - error "failed to create $tfile.$U with $numacl ACLs" + # add big LOV EA to cause reply buffer overflow earlier + $LFS setstripe -C 1000 $DIR/$tdir + lctl set_param mdc.*-mdc*.stats=clear + + $LCTL set_param debug=0 + stack_trap "$LCTL set_param debug=\"$saved_debug\"" EXIT + stack_trap "$LCTL get_param mdc.*-mdc*.stats" EXIT + + # add a large number of default ACLs (expect 8000+ for 2.13+) + for U in {2..7000}; do + setfacl -d -m user:$U:rwx $DIR/$tdir || + error "Able to add just $U default ACLs" done + numacl=$(getfacl $DIR/$tdir |& grep -c "default:user") + echo "$numacl default ACLs created" + + stat $DIR/$tdir || error "Cannot stat directory" + # check file creation + touch $DIR/$tdir/$tfile || + error "failed to create $tfile with $numacl default ACLs" + stat $DIR/$tdir/$tfile || error "Cannot stat file" + fileacl=$(getfacl $DIR/$tdir/$tfile |& grep -c "user:") + echo "$fileacl ACLs were inherited" + (( $fileacl == $numacl )) || + error "Not all default ACLs were inherited: $numacl != $fileacl" + # check that new ACLs creation adds new ACLs to inherited ACLs + setfacl -m user:19000:rwx $DIR/$tdir/$tfile || + error "Cannot set new ACL" + numacl=$((numacl + 1)) + fileacl=$(getfacl $DIR/$tdir/$tfile |& grep -c "user:") + (( $fileacl == $numacl )) || + error "failed to add new ACL: $fileacl != $numacl as expected" + # adds more ACLs to a file to reach their maximum at 8000+ + numacl=0 + for U in {20000..25000}; do + setfacl -m user:$U:rwx $DIR/$tdir/$tfile || break + numacl=$((numacl + 1)) + done + echo "Added $numacl more ACLs to the file" + fileacl=$(getfacl $DIR/$tdir/$tfile |& grep -c "user:") + echo "Total $fileacl ACLs in file" + stat $DIR/$tdir/$tfile > /dev/null || error "Cannot stat file" + rm -f $DIR/$tdir/$tfile || error "Cannot remove file" + rmdir $DIR/$tdir || error "Cannot remove directory" } run_test 103e "inheritance of big amount of default ACLs" -- 1.8.3.1