From 8fe9d033868ed6221d0b25c37b1e1bfa225ea1eb Mon Sep 17 00:00:00 2001 From: fanyong Date: Wed, 22 Nov 2006 03:31:53 +0000 Subject: [PATCH] (1) Drop unnecessary permission check for name_{insert,remove}. (2) Do local permission check for name_{insert,remove} before remote ops. (3) Add some comment. --- lustre/cmm/cmm_object.c | 31 +++++++++++++++++++++++++++++++ lustre/cmm/cmm_split.c | 1 + lustre/mdd/mdd_dir.c | 27 ++++++++++++++++++++++----- lustre/mdt/mdt_handler.c | 1 + lustre/mdt/mdt_reint.c | 6 ++++++ 5 files changed, 61 insertions(+), 5 deletions(-) diff --git a/lustre/cmm/cmm_object.c b/lustre/cmm/cmm_object.c index 1877680..ab6f552 100644 --- a/lustre/cmm/cmm_object.c +++ b/lustre/cmm/cmm_object.c @@ -548,6 +548,11 @@ static int cml_rename(const struct lu_env *env, struct md_object *mo_po, if (mo_t && lu_object_exists(&mo_t->mo_lu) < 0) { /* mo_t is remote object and there is RPC to unlink it */ + /* + * XXX: before remote unlink, maybe need local sanity check + * for mdo_rename first, or do some revocation for remote + * unlink if mdo_rename failed. + */ rc = mo_ref_del(env, md_object_next(mo_t), ma); if (rc) RETURN(rc); @@ -887,6 +892,11 @@ static int cmr_create(const struct lu_env *env, struct md_object *mo_p, } #endif + /* Local permission check for name_insert before remote ops. */ + rc = mo_permission(env, md_object_next(mo_p), MAY_WRITE); + if (rc) + RETURN(rc); + /* Remote object creation and local name insert. */ rc = mo_object_create(env, md_object_next(mo_c), spec, ma); if (rc == 0) { @@ -911,6 +921,11 @@ static int cmr_link(const struct lu_env *env, struct md_object *mo_p, if (rc == 0) { rc = -EEXIST; } else if (rc == -ENOENT) { + /* Local permission check for name_insert before remote ops. */ + rc = mo_permission(env, md_object_next(mo_p), MAY_WRITE); + if (rc) + RETURN(rc); + rc = mo_ref_add(env, md_object_next(mo_s)); if (rc == 0) { rc = mdo_name_insert(env, md_object_next(mo_p), name, @@ -927,6 +942,11 @@ static int cmr_unlink(const struct lu_env *env, struct md_object *mo_p, int rc; ENTRY; + /* Local permission check for name_remove before remote ops. */ + rc = mo_permission(env, md_object_next(mo_p), MAY_WRITE); + if (rc) + RETURN(rc); + rc = mo_ref_del(env, md_object_next(mo_c), ma); if (rc == 0) { rc = mdo_name_remove(env, md_object_next(mo_p), name, @@ -951,6 +971,12 @@ static int cmr_rename(const struct lu_env *env, RETURN(rc); LASSERT(mo_t == NULL); + + /* Local permission check for name_remove before remote ops. */ + rc = mo_permission(env, md_object_next(mo_po), MAY_WRITE); + if (rc) + RETURN(rc); + /* the mo_pn is remote directory, so we cannot even know if there is * mo_t or not. Therefore mo_t is NULL here but remote server should do * lookup and process this further */ @@ -974,6 +1000,11 @@ static int cmr_rename_tgt(const struct lu_env *env, int rc; ENTRY; /* target object is remote one */ + /* + * XXX: before remote unlink, maybe need local sanity check + * for mdo_rename_tgt first, or do some revocation for remote + * unlink if mdo_rename_tgt failed. + */ rc = mo_ref_del(env, md_object_next(mo_t), ma); /* continue locally with name handling only */ if (rc == 0) diff --git a/lustre/cmm/cmm_split.c b/lustre/cmm/cmm_split.c index 9db11d5..8951f58 100644 --- a/lustre/cmm/cmm_split.c +++ b/lustre/cmm/cmm_split.c @@ -408,6 +408,7 @@ static int cmm_split_remove_entry(const struct lu_env *env, GOTO(cleanup, rc = -ENOMEM); memcpy(name, ent->lde_name, le16_to_cpu(ent->lde_namelen)); + /* No permission check for name_remove when split */ rc = mdo_name_remove(env, md_object_next(mo), name, is_dir); OBD_FREE(name, le16_to_cpu(ent->lde_namelen) + 1); diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c index 280afc9..d16d29e 100644 --- a/lustre/mdd/mdd_dir.c +++ b/lustre/mdd/mdd_dir.c @@ -623,6 +623,7 @@ out_trans: return rc; } +#if 0 static int mdd_ni_sanity_check(const struct lu_env *env, struct md_object *pobj, const char *name, @@ -639,6 +640,7 @@ static int mdd_ni_sanity_check(const struct lu_env *env, RETURN(mdd_permission_internal_locked(env, obj, NULL, MAY_WRITE | MAY_EXEC)); } +#endif /* * Partial operation. @@ -663,17 +665,24 @@ static int mdd_name_insert(const struct lu_env *env, struct md_object *pobj, dlh = mdd_pdo_write_lock(env, mdd_obj, name); if (dlh == NULL) GOTO(out_trans, rc = -ENOMEM); +#if 0 + /* + * For some case, no need permission check, e.g. split_dir. + * When need permission check, do it before name_insert. + */ rc = mdd_ni_sanity_check(env, pobj, name, fid); if (rc) GOTO(out_unlock, rc); +#endif rc = __mdd_index_insert(env, mdd_obj, fid, name, is_dir, handle, BYPASS_CAPA); - if (rc == 0) { - la->la_ctime = la->la_atime = CURRENT_SECONDS; - la->la_valid = LA_ATIME | LA_CTIME; - rc = mdd_attr_set_internal_locked(env, mdd_obj, la, handle, 0); - } + if (rc) + GOTO(out_unlock, rc); + + la->la_ctime = la->la_atime = CURRENT_SECONDS; + la->la_valid = LA_ATIME | LA_CTIME; + rc = mdd_attr_set_internal_locked(env, mdd_obj, la, handle, 0); EXIT; out_unlock: mdd_pdo_write_unlock(env, mdd_obj, dlh); @@ -682,6 +691,7 @@ out_trans: return rc; } +#if 0 static int mdd_nr_sanity_check(const struct lu_env *env, struct md_object *pobj, const char *name) @@ -699,6 +709,7 @@ static int mdd_nr_sanity_check(const struct lu_env *env, RETURN(mdd_permission_internal_locked(env, obj, NULL, MAY_WRITE | MAY_EXEC)); } +#endif /* * Partial operation. @@ -723,9 +734,15 @@ static int mdd_name_remove(const struct lu_env *env, dlh = mdd_pdo_write_lock(env, mdd_obj, name); if (dlh == NULL) GOTO(out_trans, rc = -ENOMEM); +#if 0 + /* + * For some case, no need permission check, e.g. split_dir. + * When need permission check, do it before name_remove. + */ rc = mdd_nr_sanity_check(env, pobj, name); if (rc) GOTO(out_unlock, rc); +#endif rc = __mdd_index_delete(env, mdd_obj, name, is_dir, handle, BYPASS_CAPA); diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index 5e239f9..737877a 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -1117,6 +1117,7 @@ static int mdt_write_dir_page(struct mdt_thread_info *info, struct page *page, GOTO(out, rc = -ENOMEM); memcpy(name, ent->lde_name, le16_to_cpu(ent->lde_namelen)); + /* No permission check for name_insert when write_dir_page */ rc = mdo_name_insert(info->mti_env, md_object_next(&object->mot_obj), name, lf, is_dir); diff --git a/lustre/mdt/mdt_reint.c b/lustre/mdt/mdt_reint.c index f35d2b8..f5b08fc 100644 --- a/lustre/mdt/mdt_reint.c +++ b/lustre/mdt/mdt_reint.c @@ -613,6 +613,12 @@ static int mdt_reint_rename_tgt(struct mdt_thread_info *info) mdt_object_child(mtgt), rr->rr_fid2, rr->rr_tgt, ma); } else /* -ENOENT */ { + /* Do permission check for name_insert first */ + rc = mo_permission(info->mti_env, mdt_object_child(mtgtdir), + MAY_WRITE); + if (rc) + GOTO(out_unlock_tgtdir, rc); + rc = mdo_name_insert(info->mti_env, mdt_object_child(mtgtdir), rr->rr_tgt, rr->rr_fid2, S_ISDIR(ma->ma_attr.la_mode)); -- 1.8.3.1