Whamcloud - gitweb
LU-5152 quota: enforce block quota for chgrp 46/30146/23
authorHongchao Zhang <hongchao.zhang@intel.com>
Sat, 27 Jan 2018 21:21:35 +0000 (05:21 +0800)
committerOleg Drokin <oleg.drokin@intel.com>
Tue, 6 Feb 2018 04:27:53 +0000 (04:27 +0000)
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.

Change-Id: I4b781e94493fe63c8cbd5700dc68293b2504c2ac
Signed-off-by: Hongchao Zhang <hongchao.zhang@intel.com>
Reviewed-on: https://review.whamcloud.com/30146
Reviewed-by: Fan Yong <fan.yong@intel.com>
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
lustre/include/uapi/linux/lustre/lustre_idl.h
lustre/mdd/mdd_object.c
lustre/osd-ldiskfs/osd_handler.c
lustre/osd-zfs/osd_object.c
lustre/osp/osp_object.c
lustre/quota/qsd_handler.c
lustre/target/out_lib.c
lustre/tests/sanity-quota.sh

index 3a65228..a158f70 100644 (file)
@@ -1674,6 +1674,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,
 };
index 2bc73a4..b2a6d5c 100644 (file)
@@ -879,10 +879,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;
 
@@ -908,17 +909,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;
@@ -928,20 +950,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;
 }
index 118bec0..ad77d4d 100644 (file)
@@ -2479,7 +2479,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);
@@ -2494,7 +2494,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);
@@ -2503,7 +2503,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);
@@ -2519,7 +2519,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);
@@ -2528,7 +2528,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);
@@ -2566,22 +2566,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);
 
@@ -2589,7 +2597,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);
 
@@ -2601,7 +2610,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);
        }
@@ -5618,7 +5627,7 @@ static int osd_index_declare_ea_insert(const struct lu_env *env,
                    i_projid_read(inode) != 0)
                        rc = osd_declare_attr_qid(env, osd_dt_obj(dt), oh,
                                                  0, i_projid_read(inode),
-                                                 0, false, PRJQUOTA);
+                                                 0, false, PRJQUOTA, true);
 #endif
        }
 
index 853ef27..c835e92 100644 (file)
@@ -1037,7 +1037,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;
 
@@ -1054,7 +1054,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;
@@ -1076,7 +1076,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;
@@ -1146,7 +1146,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) {
@@ -1155,7 +1159,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);
                }
@@ -1166,7 +1170,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);
                }
@@ -1197,7 +1203,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);
                }
index 0604892..4e316c4 100644 (file)
@@ -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;
 
index b3c43a0..eb0bddb 100644 (file)
@@ -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);
        }
index 14bc9bd..1f55422 100644 (file)
@@ -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))
index 524ceae..431c5d2 100755 (executable)
@@ -2983,6 +2983,47 @@ test_54() {
 }
 run_test 54 "basic lfs project interface test"
 
+test_55() {
+       setup_quota_test || error "setup quota failed with $?"
+
+       set_ost_qtype $QTYPE || error "enable ost quota failed"
+       quota_init
+
+       #add second group to TSTUSR
+       usermod -G $TSTUSR,$TSTUSR2 $TSTUSR
+
+       #prepare test file
+       $RUNAS dd if=/dev/zero of=$DIR/$tdir/$tfile bs=1024 count=100000 ||
+       error "failed to dd"
+
+       cancel_lru_locks osc
+       sync; sync_all_data || true
+
+       $LFS setquota -g $TSTUSR2 -b 0 -B 50M $DIR ||
+       error "failed to setquota on group $TSTUSR2"
+
+       $LFS quota -v -g $TSTUSR2 $DIR
+
+       runas -u $TSTUSR -g $TSTUSR2 chgrp $TSTUSR2 $DIR/$tdir/$tfile &&
+       error "chgrp should failed with -EDQUOT"
+
+       USED=$(getquota -g $TSTUSR2 global curspace)
+       echo "$USED"
+
+       $LFS setquota -g $TSTUSR2 -b 0 -B 300M $DIR ||
+       error "failed to setquota on group $TSTUSR2"
+
+       $LFS quota -v -g $TSTUSR2 $DIR
+
+       runas -u $TSTUSR -g $TSTUSR2 chgrp $TSTUSR2 $DIR/$tdir/$tfile ||
+       error "chgrp should succeed"
+
+       $LFS quota -v -g $TSTUSR2 $DIR
+
+       cleanup_quota_test
+}
+run_test 55 "Chgrp should be affected by group quota"
+
 quota_fini()
 {
        do_nodes $(comma_list $(nodes_list)) "lctl set_param debug=-quota"