From 07412234ec60de20cb8d8e45d755297fe6da2d61 Mon Sep 17 00:00:00 2001 From: Hongchao Zhang Date: Sun, 28 Jan 2018 05:21:35 +0800 Subject: [PATCH] LU-5152 quota: enforce block quota for chgrp When an unprivileged user calls chgrp to change the group of one of his files, the block quota limit of that new group should be checked to ensure it not exceeds the limit. The side effect of this patch could be, 1.The performance of chgrp from non-privileged user will be very slow, no matter if quota is enabled. Since we assume that chgrp issued from non-privileged user is very rare, the performance impact possibly is acceptable. 2.If MDT crash while performing chgrp, inconsistency (group ownership among MDT and OST objects) will be created. It should be acceptable. This patch has fixed the bug while calculating the disk space of some file for ldiskfs and zfs, the block unit is always 512. Lustre-change: https://review.whamcloud.com/30146 Lustre-commit: 8a71fd5061bd073e055e6cbba1d238305e6827bb Change-Id: I4b781e94493fe63c8cbd5700dc68293b2504c2ac Signed-off-by: Hongchao Zhang Reviewed-by: Fan Yong Reviewed-by: Niu Yawei Signed-off-by: Minh Diep Reviewed-on: https://review.whamcloud.com/31210 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: John L. Hammond --- lustre/include/lustre/lustre_idl.h | 1 + lustre/mdd/mdd_object.c | 59 +++++++++++++++++++++++++++----------- lustre/osd-ldiskfs/osd_handler.c | 31 +++++++++++++------- lustre/osd-zfs/osd_object.c | 20 ++++++++----- lustre/osp/osp_object.c | 33 +++++++++++++++++++-- lustre/quota/qsd_handler.c | 7 +++++ lustre/target/out_lib.c | 4 +++ 7 files changed, 119 insertions(+), 36 deletions(-) diff --git a/lustre/include/lustre/lustre_idl.h b/lustre/include/lustre/lustre_idl.h index a7d3424..f3173fe 100644 --- a/lustre/include/lustre/lustre_idl.h +++ b/lustre/include/lustre/lustre_idl.h @@ -1639,6 +1639,7 @@ enum { * 2. If these flags needs to be stored into inode, they will be * stored in LMA. see LMAI_XXXX */ LUSTRE_ORPHAN_FL = 0x00002000, + LUSTRE_SET_SYNC_FL = 0x00040000, /* Synchronous setattr on OSTs */ LUSTRE_LMA_FL_MASKS = LUSTRE_ORPHAN_FL, }; diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index 88069ca..41031dc 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -861,10 +861,11 @@ int mdd_attr_set(const struct lu_env *env, struct md_object *obj, { struct mdd_object *mdd_obj = md2mdd_obj(obj); struct mdd_device *mdd = mdo2mdd(obj); - struct thandle *handle; + struct thandle *handle = NULL; struct lu_attr *la_copy = &mdd_env_info(env)->mti_la_for_fix; struct lu_attr *attr = MDD_ENV_VAR(env, cattr); const struct lu_attr *la = &ma->ma_attr; + struct lu_ucred *uc; int rc; ENTRY; @@ -890,17 +891,38 @@ int mdd_attr_set(const struct lu_env *env, struct md_object *obj, RETURN(0); } - handle = mdd_trans_create(env, mdd); - if (IS_ERR(handle)) - RETURN(PTR_ERR(handle)); + /* If an unprivileged user changes group of some file, + * the setattr operation will be processed synchronously to + * honor the quota limit of the corresponding group. see LU-5152 */ + uc = lu_ucred_check(env); + if (S_ISREG(attr->la_mode) && la->la_valid & LA_GID && + la->la_gid != attr->la_gid && uc != NULL && uc->uc_fsuid != 0) { + la_copy->la_valid |= LA_FLAGS; + la_copy->la_flags |= LUSTRE_SET_SYNC_FL; + + /* Flush the possible existing sync requests to OSTs to + * keep the order of sync for the current setattr operation + * will be sent directly to OSTs. see LU-5152 */ + rc = dt_sync(env, mdd->mdd_child); + if (rc) + GOTO(out, rc); + } + + handle = mdd_trans_create(env, mdd); + if (IS_ERR(handle)) { + rc = PTR_ERR(handle); + handle = NULL; + + GOTO(out, rc); + } rc = mdd_declare_attr_set(env, mdd, mdd_obj, la_copy, handle); - if (rc) - GOTO(stop, rc); + if (rc) + GOTO(out, rc); - rc = mdd_trans_start(env, mdd, handle); - if (rc) - GOTO(stop, rc); + rc = mdd_trans_start(env, mdd, handle); + if (rc) + GOTO(out, rc); if (mdd->mdd_sync_permission && permission_needs_sync(attr, la)) handle->th_sync = 1; @@ -910,20 +932,25 @@ int mdd_attr_set(const struct lu_env *env, struct md_object *obj, la->la_mtime, la->la_ctime); mdd_write_lock(env, mdd_obj, MOR_TGT_CHILD); - if (la_copy->la_valid & LA_FLAGS) - rc = mdd_attr_set_internal(env, mdd_obj, la_copy, handle, 1); - else if (la_copy->la_valid) /* setattr */ + if (la_copy->la_valid) { rc = mdd_attr_set_internal(env, mdd_obj, la_copy, handle, 1); + + if (rc == -EDQUOT && la_copy->la_flags & LUSTRE_SET_SYNC_FL) { + /* rollback to the original gid */ + la_copy->la_flags &= ~LUSTRE_SET_SYNC_FL; + la_copy->la_gid = attr->la_gid; + mdd_attr_set_internal(env, mdd_obj, la_copy, handle, 1); + } + } mdd_write_unlock(env, mdd_obj); +out: if (rc == 0) rc = mdd_attr_set_changelog(env, obj, handle, la_copy->la_valid); - GOTO(stop, rc); - -stop: - rc = mdd_trans_stop(env, mdd, rc, handle); + if (handle != NULL) + rc = mdd_trans_stop(env, mdd, rc, handle); return rc; } diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index 6877fff..e8568dd 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -2435,7 +2435,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, struct osd_object *obj, struct osd_thandle *oh, long long bspace, qid_t old_id, qid_t new_id, bool enforce, - unsigned type) + unsigned type, bool ignore_edquot) { int rc; struct osd_thread_info *info = osd_oti_get(env); @@ -2450,7 +2450,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, qi->lqi_space = 1; /* Reserve credits for the new id */ rc = osd_declare_qid(env, oh, qi, NULL, enforce, NULL); - if (rc == -EDQUOT || rc == -EINPROGRESS) + if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) rc = 0; if (rc) RETURN(rc); @@ -2459,7 +2459,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, qi->lqi_id.qid_uid = old_id; qi->lqi_space = -1; rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); - if (rc == -EDQUOT || rc == -EINPROGRESS) + if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) rc = 0; if (rc) RETURN(rc); @@ -2475,7 +2475,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, * to save credit reservation. */ rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); - if (rc == -EDQUOT || rc == -EINPROGRESS) + if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) rc = 0; if (rc) RETURN(rc); @@ -2484,7 +2484,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, qi->lqi_id.qid_uid = old_id; qi->lqi_space = -bspace; rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); - if (rc == -EDQUOT || rc == -EINPROGRESS) + if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) rc = 0; RETURN(rc); @@ -2522,22 +2522,30 @@ static int osd_declare_attr_set(const struct lu_env *env, if (attr == NULL || obj->oo_inode == NULL) RETURN(rc); - bspace = obj->oo_inode->i_blocks; - bspace <<= obj->oo_inode->i_sb->s_blocksize_bits; + bspace = obj->oo_inode->i_blocks << 9; bspace = toqb(bspace); /* Changing ownership is always preformed by super user, it should not - * fail with EDQUOT. + * fail with EDQUOT unless required explicitly. * * We still need to call the osd_declare_qid() to calculate the journal * credits for updating quota accounting files and to trigger quota * space adjustment once the operation is completed.*/ if (attr->la_valid & LA_UID || attr->la_valid & LA_GID) { + bool ignore_edquot = !(attr->la_flags & LUSTRE_SET_SYNC_FL); + + if (!ignore_edquot) + CDEBUG(D_QUOTA, "%s: enforce quota on UID %u, GID %u" + "(the quota space is %lld)\n", + obj->oo_inode->i_sb->s_id, attr->la_uid, + attr->la_gid, bspace); + /* USERQUOTA */ uid = i_uid_read(obj->oo_inode); enforce = (attr->la_valid & LA_UID) && (attr->la_uid != uid); rc = osd_declare_attr_qid(env, obj, oh, bspace, uid, - attr->la_uid, enforce, USRQUOTA); + attr->la_uid, enforce, USRQUOTA, + true); if (rc) RETURN(rc); @@ -2545,7 +2553,8 @@ static int osd_declare_attr_set(const struct lu_env *env, enforce = (attr->la_valid & LA_GID) && (attr->la_gid != gid); rc = osd_declare_attr_qid(env, obj, oh, bspace, i_gid_read(obj->oo_inode), - attr->la_gid, enforce, GRPQUOTA); + attr->la_gid, enforce, GRPQUOTA, + ignore_edquot); if (rc) RETURN(rc); @@ -2557,7 +2566,7 @@ static int osd_declare_attr_set(const struct lu_env *env, (attr->la_projid != projid); rc = osd_declare_attr_qid(env, obj, oh, bspace, (qid_t)projid, (qid_t)attr->la_projid, - enforce, PRJQUOTA); + enforce, PRJQUOTA, true); if (rc) RETURN(rc); } diff --git a/lustre/osd-zfs/osd_object.c b/lustre/osd-zfs/osd_object.c index 6d51b63..0c7beca 100644 --- a/lustre/osd-zfs/osd_object.c +++ b/lustre/osd-zfs/osd_object.c @@ -825,7 +825,7 @@ static inline int qsd_transfer(const struct lu_env *env, struct qsd_instance *qsd, struct lquota_trans *trans, int qtype, __u64 orig_id, __u64 new_id, __u64 bspace, - struct lquota_id_info *qi) + struct lquota_id_info *qi, bool ignore_edquot) { int rc; @@ -842,7 +842,7 @@ static inline int qsd_transfer(const struct lu_env *env, qi->lqi_id.qid_uid = new_id; qi->lqi_space = 1; rc = qsd_op_begin(env, qsd, trans, qi, NULL); - if (rc == -EDQUOT || rc == -EINPROGRESS) + if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) rc = 0; if (rc) return rc; @@ -864,7 +864,7 @@ static inline int qsd_transfer(const struct lu_env *env, qi->lqi_id.qid_uid = new_id; qi->lqi_space = bspace; rc = qsd_op_begin(env, qsd, trans, qi, NULL); - if (rc == -EDQUOT || rc == -EINPROGRESS) + if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) rc = 0; if (rc) return rc; @@ -934,7 +934,11 @@ static int osd_declare_attr_set(const struct lu_env *env, if (attr && (attr->la_valid & (LA_UID | LA_GID | LA_PROJID))) { sa_object_size(obj->oo_sa_hdl, &blksize, &bspace); - bspace = toqb(bspace * blksize); + bspace = toqb(bspace * 512); + + CDEBUG(D_QUOTA, "%s: enforce quota on UID %u, GID %u," + "the quota space is %lld (%u)\n", osd->od_svname, + attr->la_uid, attr->la_gid, bspace, blksize); } if (attr && attr->la_valid & LA_UID) { @@ -943,7 +947,7 @@ static int osd_declare_attr_set(const struct lu_env *env, rc = qsd_transfer(env, osd->od_quota_slave, &oh->ot_quota_trans, USRQUOTA, obj->oo_attr.la_uid, attr->la_uid, - bspace, &info->oti_qi); + bspace, &info->oti_qi, true); if (rc) GOTO(out, rc); } @@ -954,7 +958,9 @@ static int osd_declare_attr_set(const struct lu_env *env, rc = qsd_transfer(env, osd->od_quota_slave, &oh->ot_quota_trans, GRPQUOTA, obj->oo_attr.la_gid, attr->la_gid, - bspace, &info->oti_qi); + bspace, &info->oti_qi, + !(attr->la_flags & + LUSTRE_SET_SYNC_FL)); if (rc) GOTO(out, rc); } @@ -985,7 +991,7 @@ static int osd_declare_attr_set(const struct lu_env *env, &oh->ot_quota_trans, PRJQUOTA, obj->oo_attr.la_projid, attr->la_projid, bspace, - &info->oti_qi); + &info->oti_qi, true); if (rc) GOTO(out, rc); } diff --git a/lustre/osp/osp_object.c b/lustre/osp/osp_object.c index 75c873a..0668475 100644 --- a/lustre/osp/osp_object.c +++ b/lustre/osp/osp_object.c @@ -709,8 +709,37 @@ static int osp_attr_set(const struct lu_env *env, struct dt_object *dt, RETURN(0); if (!is_only_remote_trans(th)) { - rc = osp_sync_add(env, o, MDS_SETATTR64_REC, th, attr); - /* XXX: send new uid/gid to OST ASAP? */ + if (attr->la_flags & LUSTRE_SET_SYNC_FL) { + struct ptlrpc_request *req = NULL; + struct osp_update_request *update = NULL; + struct osp_device *osp = lu2osp_dev(dt->do_lu.lo_dev); + + update = osp_update_request_create(&osp->opd_dt_dev); + if (IS_ERR(update)) + RETURN(PTR_ERR(update)); + + rc = osp_update_rpc_pack(env, attr_set, update, + OUT_ASTTR_SET, + lu_object_fid(&dt->do_lu), + attr); + if (rc != 0) { + CERROR("%s: update error "DFID": rc = %d\n", + osp->opd_obd->obd_name, + PFID(lu_object_fid(&dt->do_lu)), rc); + + osp_update_request_destroy(env, update); + RETURN(rc); + } + + rc = osp_remote_sync(env, osp, update, &req); + if (req != NULL) + ptlrpc_req_finished(req); + + osp_update_request_destroy(env, update); + } else { + rc = osp_sync_add(env, o, MDS_SETATTR64_REC, th, attr); + /* XXX: send new uid/gid to OST ASAP? */ + } } else { struct lu_attr *la; diff --git a/lustre/quota/qsd_handler.c b/lustre/quota/qsd_handler.c index a8d842d..5ccd6f8 100644 --- a/lustre/quota/qsd_handler.c +++ b/lustre/quota/qsd_handler.c @@ -652,6 +652,13 @@ static bool qsd_acquire(const struct lu_env *env, struct lquota_entry *lqe, * rc < 0, something bad happened */ break; + /* if we have gotten some quota and stil wait more quota, + * it's better to give QMT some time to reclaim from clients */ + if (count > 0) { + set_current_state(TASK_INTERRUPTIBLE); + schedule_timeout(cfs_time_seconds(1)); + } + /* need to acquire more quota space from master */ rc = qsd_acquire_remote(env, lqe); } diff --git a/lustre/target/out_lib.c b/lustre/target/out_lib.c index c267ed2..8e60dfff9 100644 --- a/lustre/target/out_lib.c +++ b/lustre/target/out_lib.c @@ -657,6 +657,10 @@ int out_attr_set_add_exec(const struct lu_env *env, struct dt_object *dt_obj, if (rc != 0) return rc; + if (attr->la_valid & LA_FLAGS && + attr->la_flags & LUSTRE_SET_SYNC_FL) + th->th_sync |= 1; + arg = tx_add_exec(ta, out_tx_attr_set_exec, out_tx_attr_set_undo, file, line); if (IS_ERR(arg)) -- 1.8.3.1