From 0bf5a0474702079adfe5c6005c0fec6a21d06c86 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Thu, 23 Mar 2023 14:01:51 -0400 Subject: [PATCH] rgw: dont duplicate sal handles in RGWCopyObj the CopyObject request is issued against the destination bucket/object, so refer directly to s->bucket and s->object instead of creating separate dst_bucket and dst_object handles Signed-off-by: Casey Bodley (cherry picked from commit a779f46a9c59c2aa8ac7bd46d5b58945ed68bdab) --- src/rgw/rgw_file.h | 11 +++++---- src/rgw/rgw_op.cc | 51 ++++++++++++++------------------------- src/rgw/rgw_op.h | 4 --- src/rgw/rgw_rest_s3.cc | 12 +++------ src/rgw/rgw_rest_swift.cc | 6 ----- 5 files changed, 27 insertions(+), 57 deletions(-) diff --git a/src/rgw/rgw_file.h b/src/rgw/rgw_file.h index ea7c2b24c8fcc..65ec3dd15d339 100644 --- a/src/rgw/rgw_file.h +++ b/src/rgw/rgw_file.h @@ -2610,16 +2610,17 @@ public: state->info.method = "PUT"; // XXX check state->op = OP_PUT; - src_bucket_name = src_parent->bucket_name(); - state->src_bucket_name = src_bucket_name; - dest_bucket_name = dst_parent->bucket_name(); - state->bucket_name = dest_bucket_name; - dest_obj_name = dst_parent->format_child_name(dst_name, false); + state->src_bucket_name = src_parent->bucket_name(); + state->bucket_name = dst_parent->bucket_name(); + + std::string dest_obj_name = dst_parent->format_child_name(dst_name, false); int rc = valid_s3_object_name(dest_obj_name); if (rc != 0) return rc; + state->object = RGWHandler::driver->get_object(rgw_obj_key(dest_obj_name)); + /* XXX and fixup key attr (could optimize w/string ref and * dest_obj_name) */ buffer::list ux_key; diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 21b7737f66f00..c8be835533fdc 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -5206,8 +5206,8 @@ int RGWCopyObj::verify_permission(optional_yield y) } op_ret = driver->get_bucket(this, s->user.get(), - rgw_bucket(src_tenant_name, - src_bucket_name, + rgw_bucket(s->src_tenant_name, + s->src_bucket_name, s->bucket_instance_id), &src_bucket, y); if (op_ret < 0) { @@ -5317,45 +5317,30 @@ int RGWCopyObj::verify_permission(optional_yield y) RGWAccessControlPolicy dest_bucket_policy(s->cct); - if (src_bucket_name.compare(dest_bucket_name) == 0) { /* will only happen if s->local_source - or intra region sync */ - dest_bucket = src_bucket->clone(); - } else { - op_ret = driver->get_bucket(this, s->user.get(), dest_tenant_name, dest_bucket_name, &dest_bucket, y); - if (op_ret < 0) { - if (op_ret == -ENOENT) { - ldpp_dout(this, 0) << "ERROR: Destination Bucket not found for user: " << s->user->get_id().to_str() << dendl; - op_ret = -ERR_NO_SUCH_BUCKET; - } - return op_ret; - } - } - - dest_object = dest_bucket->get_object(rgw_obj_key(dest_obj_name)); - dest_object->set_atomic(); + s->object->set_atomic(); /* check dest bucket permissions */ - op_ret = read_bucket_policy(this, driver, s, dest_bucket->get_info(), - dest_bucket->get_attrs(), - &dest_bucket_policy, dest_bucket->get_key(), y); + op_ret = read_bucket_policy(this, driver, s, s->bucket->get_info(), + s->bucket->get_attrs(), + &dest_bucket_policy, s->bucket->get_key(), y); if (op_ret < 0) { return op_ret; } - auto dest_iam_policy = get_iam_policy_from_attr(s->cct, dest_bucket->get_attrs(), dest_bucket->get_tenant()); + auto dest_iam_policy = get_iam_policy_from_attr(s->cct, s->bucket->get_attrs(), s->bucket->get_tenant()); /* admin request overrides permission checks */ if (! s->auth.identity->is_admin_of(dest_policy.get_owner().get_id())){ if (dest_iam_policy != boost::none || ! s->iam_user_policies.empty() || !s->session_policies.empty()) { //Add destination bucket tags for authorization auto [has_s3_existing_tag, has_s3_resource_tag] = rgw_check_policy_condition(this, dest_iam_policy, s->iam_user_policies, s->session_policies); if (has_s3_resource_tag) - rgw_iam_add_buckettags(this, s, dest_bucket.get()); + rgw_iam_add_buckettags(this, s, s->bucket.get()); rgw_add_to_iam_environment(s->env, "s3:x-amz-copy-source", copy_source); if (md_directive) rgw_add_to_iam_environment(s->env, "s3:x-amz-metadata-directive", *md_directive); - ARN obj_arn(dest_object->get_obj()); + ARN obj_arn(s->object->get_obj()); auto identity_policy_res = eval_identity_or_session_policies(this, s->iam_user_policies, s->env, rgw::IAM::s3PutObject, @@ -5493,13 +5478,13 @@ void RGWCopyObj::execute(optional_yield y) } if ( ! version_id.empty()) { - dest_object->set_instance(version_id); - } else if (dest_bucket->versioning_enabled()) { - dest_object->gen_rand_obj_instance_name(); + s->object->set_instance(version_id); + } else if (s->bucket->versioning_enabled()) { + s->object->gen_rand_obj_instance_name(); } s->src_object->set_atomic(); - dest_object->set_atomic(); + s->object->set_atomic(); encode_delete_at_attr(delete_at, attrs); @@ -5544,7 +5529,7 @@ void RGWCopyObj::execute(optional_yield y) return; } // enforce quota against the destination bucket owner - op_ret = dest_bucket->check_quota(this, quota, astate->accounted_size, y); + op_ret = s->bucket->check_quota(this, quota, astate->accounted_size, y); if (op_ret < 0) { return; } @@ -5555,7 +5540,7 @@ void RGWCopyObj::execute(optional_yield y) /* Handle object versioning of Swift API. In case of copying to remote this * should fail gently (op_ret == 0) as the dst_obj will not exist here. */ - op_ret = dest_object->swift_versioning_copy(this, s->yield); + op_ret = s->object->swift_versioning_copy(this, s->yield); if (op_ret < 0) { return; } @@ -5563,8 +5548,8 @@ void RGWCopyObj::execute(optional_yield y) op_ret = s->src_object->copy_object(s->user.get(), &s->info, source_zone, - dest_object.get(), - dest_bucket.get(), + s->object.get(), + s->bucket.get(), src_bucket.get(), s->dest_placement, &src_mtime, @@ -5588,7 +5573,7 @@ void RGWCopyObj::execute(optional_yield y) s->yield); // send request to notification manager - int ret = res->publish_commit(this, obj_size, mtime, etag, dest_object->get_instance()); + int ret = res->publish_commit(this, obj_size, mtime, etag, s->object->get_instance()); if (ret < 0) { ldpp_dout(this, 1) << "ERROR: publishing notification failed, with error: " << ret << dendl; // too late to rollback operation, hence op_ret is not set here diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index d29dc01415383..b29c87c51cae0 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -1510,11 +1510,7 @@ protected: ceph::real_time *mod_ptr; ceph::real_time *unmod_ptr; rgw::sal::Attrs attrs; - std::string src_tenant_name, src_bucket_name, src_obj_name; std::unique_ptr src_bucket; - std::string dest_tenant_name, dest_bucket_name, dest_obj_name; - std::unique_ptr dest_bucket; - std::unique_ptr dest_object; ceph::real_time src_mtime; ceph::real_time mtime; rgw::sal::AttrsMod attrs_mod; diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index ccb8a397aec25..359c94dadd87d 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -3418,12 +3418,6 @@ int RGWCopyObj_ObjStore_S3::get_params(optional_yield y) if_match = s->info.env->get("HTTP_X_AMZ_COPY_SOURCE_IF_MATCH"); if_nomatch = s->info.env->get("HTTP_X_AMZ_COPY_SOURCE_IF_NONE_MATCH"); - src_tenant_name = s->src_tenant_name; - src_bucket_name = s->src_bucket_name; - dest_tenant_name = s->bucket->get_tenant(); - dest_bucket_name = s->bucket->get_name(); - dest_obj_name = s->object->get_name(); - if (s->system_request) { source_zone = s->info.args.get(RGW_SYS_PARAM_PREFIX "source-zone"); s->info.args.get_bool(RGW_SYS_PARAM_PREFIX "copy-if-newer", ©_if_newer, false); @@ -3447,9 +3441,9 @@ int RGWCopyObj_ObjStore_S3::get_params(optional_yield y) } if (source_zone.empty() && - (dest_tenant_name.compare(src_tenant_name) == 0) && - (dest_bucket_name.compare(src_bucket_name) == 0) && - (dest_obj_name.compare(s->src_object->get_name()) == 0) && + (s->bucket->get_tenant() == s->src_tenant_name) && + (s->bucket->get_name() == s->src_bucket_name) && + (s->object->get_name() == s->src_object->get_name()) && s->src_object->get_instance().empty() && (attrs_mod != rgw::sal::ATTRSMOD_REPLACE)) { need_to_check_storage_class = true; diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index a3ab698ac191f..a9565817005dd 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -1382,12 +1382,6 @@ int RGWCopyObj_ObjStore_SWIFT::get_params(optional_yield y) if_match = s->info.env->get("HTTP_COPY_IF_MATCH"); if_nomatch = s->info.env->get("HTTP_COPY_IF_NONE_MATCH"); - src_tenant_name = s->src_tenant_name; - src_bucket_name = s->src_bucket_name; - dest_tenant_name = s->bucket_tenant; - dest_bucket_name = s->bucket_name; - dest_obj_name = s->object->get_name(); - const char * const fresh_meta = s->info.env->get("HTTP_X_FRESH_METADATA"); if (fresh_meta && strcasecmp(fresh_meta, "TRUE") == 0) { attrs_mod = rgw::sal::ATTRSMOD_REPLACE; -- 2.39.5