rearrange / simplify RGWBucket::link logic - start bucket move support

The existing RGWBucket::link logic changed things incrementally in
order to relink a bucket.  When doing a "bucket move", this is no longer
a good idea - bucket objects must be written to new names which don't
exist, so it is better to create then wholly out of in-memory data.

Also, add != for rgw_bucket - inverse of existing of ==, provides
another option to arrange code to make it more readable.

Fixes: http://tracker.ceph.com/issues/35885
Signed-off-by: Marcus Watts <mwatts@redhat.com>

diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc
index de1175e71ad78bbaff9cb13e8cd517eddfc88a22..cd93cbdbb205c382c709e116fb2bf04dfb8908f6 100644 (file)
--- a/src/rgw/rgw_bucket.cc
+++ b/src/rgw/rgw_bucket.cc
@@ -865,6 +865,9 @@ int RGWBucket::link(RGWBucketAdminOpState& op_state,
     set_err_msg(err_msg, "specified bucket id does not match");
     return -EINVAL;
   }
+  rgw_bucket old_bucket = bucket;
+  bucket.tenant = tenant;
+
 
   const rgw_pool& root_pool = store->svc.zone->get_zone_params().domain_root;
   std::string bucket_entry;
@@ -873,42 +876,51 @@ int RGWBucket::link(RGWBucketAdminOpState& op_state,
   RGWObjVersionTracker objv_tracker;
 
   map<string, bufferlist>::iterator aiter = attrs.find(RGW_ATTR_ACL);
-  if (aiter != attrs.end()) {
-    bufferlist aclbl = aiter->second;
-    RGWAccessControlPolicy policy;
-    ACLOwner owner;
-    try {
-     auto iter = aclbl.cbegin();
-     decode(policy, iter);
-     owner = policy.get_owner();
-    } catch (buffer::error& err) {
-      set_err_msg(err_msg, "couldn't decode policy");
-      return -EIO;
-    }
+  if (aiter == attrs.end()) {
+    // XXX why isn't this an error?  mdw 20180825
+    ldout(store->ctx(), 0) << "WARNING: bucket link will do nothing because no acl on bucket=" << bucket_name << dendl;
+    return 0;
+  }
+  bufferlist aclbl = aiter->second;
+  RGWAccessControlPolicy policy;
+  ACLOwner owner;
+  try {
+   auto iter = aclbl.cbegin();
+   decode(policy, iter);
+   owner = policy.get_owner();
+  } catch (buffer::error& err) {
+    set_err_msg(err_msg, "couldn't decode policy");
+    return -EIO;
+  }
 
-    int r = rgw_unlink_bucket(store, owner.get_id(), bucket.tenant, bucket.name, false);
-    if (r < 0) {
-      set_err_msg(err_msg, "could not unlink policy from user " + owner.get_id().to_str());
-      return r;
-    }
+  int r = rgw_unlink_bucket(store, owner.get_id(),
+       old_bucket.tenant, old_bucket.name, false);
+  if (r < 0) {
+    set_err_msg(err_msg, "could not unlink policy from user " + owner.get_id().to_str());
+    return r;
+  }
 
-    // now update the user for the bucket...
-    if (display_name.empty()) {
-      ldout(store->ctx(), 0) << "WARNING: user " << user_info.user_id << " has no display name set" << dendl;
-    }
-    policy.create_default(user_info.user_id, display_name);
+  // now update the user for the bucket...
+  if (display_name.empty()) {
+    ldout(store->ctx(), 0) << "WARNING: user " << user_info.user_id << " has no display name set" << dendl;
+  }
+
+  RGWAccessControlPolicy policy_instance;
+  policy_instance.create_default(user_info.user_id, display_name);
+  owner = policy_instance.get_owner();
 
-    owner = policy.get_owner();
+  aclbl.clear();
+  policy_instance.encode(aclbl);
+
+  if (bucket == old_bucket) {
     r = store->set_bucket_owner(bucket_info.bucket, owner);
     if (r < 0) {
       set_err_msg(err_msg, "failed to set bucket owner: " + cpp_strerror(-r));
       return r;
     }
 
-    // ...and encode the acl
-    aclbl.clear();
-    policy.encode(aclbl);
-
+    rgw_raw_obj obj_bucket_instance;
+    store->get_bucket_instance_obj(bucket, obj_bucket_instance);
     auto sysobj = obj_ctx.get_obj(obj);
     r = sysobj.wop()
               .set_objv_tracker(&objv_tracker)
@@ -917,43 +929,39 @@ int RGWBucket::link(RGWBucketAdminOpState& op_state,
       set_err_msg(err_msg, "failed to set new acl");
       return r;
     }
-
-    RGWAccessControlPolicy policy_instance;
-    policy_instance.create_default(user_info.user_id, display_name);
-    aclbl.clear();
-    policy_instance.encode(aclbl);
-
-    rgw_raw_obj obj_bucket_instance;
-    store->get_bucket_instance_obj(bucket, obj_bucket_instance);
-    auto inst_sysobj = obj_ctx.get_obj(obj_bucket_instance);
-    r = inst_sysobj.wop()
-                   .set_objv_tracker(&objv_tracker)
-                   .write_attr(RGW_ATTR_ACL, aclbl, null_yield);
+  } else {
+    attrs[RGW_ATTR_ACL] = aclbl;
+    bucket_info.bucket.tenant = tenant;
+    bucket_info.owner = user_info.user_id;
+    r = store->put_bucket_instance_info(bucket_info, false, real_time(), &attrs);
     if (r < 0) {
-      set_err_msg(err_msg, "failed to set bucket policy");
+      set_err_msg(err_msg, "ERROR: failed writing bucket instance info: " + cpp_strerror(-r));
       return r;
     }
+  }
 
-    RGWBucketEntryPoint ep;
-    ep.bucket = bucket_info.bucket;
-    ep.owner = user_info.user_id;
-    ep.creation_time = bucket_info.creation_time;
-    ep.linked = true;
-    map<string, bufferlist> ep_attrs;
-    // XXX I am not convinced this is at all necessary; but previous
-    // versions would have found and copied VERSION_ATTR so I will
-    // do likewise for now...  mdw 20180825
-    auto version_iter = attrs.find(VERSION_ATTR);
-    if (version_iter != attrs.end())
-      ep_attrs[VERSION_ATTR] = version_iter->second;
-    rgw_ep_info ep_data{ep, ep_attrs};
-
-    r = rgw_link_bucket(store, user_info.user_id, bucket_info.bucket,
-                        ceph::real_time(), true, &ep_data);
-    if (r < 0) {
-      set_err_msg(err_msg, "failed to relink bucket");
-      return r;
-    }
+  RGWBucketEntryPoint ep;
+  ep.bucket = bucket_info.bucket;
+  ep.owner = user_info.user_id;
+  ep.creation_time = bucket_info.creation_time;
+  ep.linked = true;
+  map<string, bufferlist> ep_attrs;
+  // XXX I am not convinced this is at all necessary; but previous
+  //   versions would have found and copied VERSION_ATTR so I will
+  //   do likewise for now...  mdw 20180825
+  auto version_iter = attrs.find(VERSION_ATTR);
+  if (version_iter != attrs.end())
+    ep_attrs[VERSION_ATTR] = version_iter->second;
+  rgw_ep_info ep_data{ep, ep_attrs};
+
+  r = rgw_link_bucket(store, user_info.user_id, bucket_info.bucket,
+                     ceph::real_time(), true, &ep_data);
+  if (r < 0) {
+    set_err_msg(err_msg, "failed to relink bucket");
+    return r;
+  }
+  if (bucket != old_bucket) {
+       // howto remove old bucket ?
   }
 
   return 0;

diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h
index 80738a7b763074da2525f0c5a72b77842cb41445..20871d5388fba835f3a7f9a86c899a9582e527cd 100644 (file)
--- a/src/rgw/rgw_common.h
+++ b/src/rgw/rgw_common.h
@@ -1268,6 +1268,10 @@ struct rgw_bucket {
     return (tenant == b.tenant) && (name == b.name) && \
            (bucket_id == b.bucket_id);
   }
+  bool operator!=(const rgw_bucket& b) const {
+    return (tenant != b.tenant) || (name != b.name) ||
+           (bucket_id != b.bucket_id);
+  }
 };
 WRITE_CLASS_ENCODER(rgw_bucket)