Skip to content

Commit

Permalink
rgw: add exclusive flag to set_as_default()
Browse files Browse the repository at this point in the history 
this dodges the race in RGWRealm::create() and RGWZoneParams::create()
that decides whether to set the new object as a default. by calling
set_as_default() with exclusive=true, it will fail with EEXIST if a
default is already set

it also fixes an issue with 'realm pull' on a secondary zone, where a
'default' zone may be created but never actually set_as_default()

Signed-off-by: Casey Bodley <cbodley@redhat.com>
  • Loading branch information
@cbodley
cbodley committed Mar 31, 2016
1 parent b3ece2c commit a25f266
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 51 deletions.
24 changes: 12 additions & 12 deletions src/rgw/rgw_admin.cc
View file
Expand Up @@ -2550,7 +2550,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = realm.set_as_default();
ret = realm.set_as_default(false);
if (ret < 0) {
cerr << "failed to set realm " << realm_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand Down Expand Up @@ -2706,7 +2706,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = realm.set_as_default();
ret = realm.set_as_default(false);
if (ret < 0) {
cerr << "failed to set realm " << realm_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand All @@ -2724,7 +2724,7 @@ int main(int argc, char **argv)
cerr << "failed to init realm: " << cpp_strerror(-ret) << std::endl;
return -ret;
}
ret = realm.set_as_default();
ret = realm.set_as_default(false);
if (ret < 0) {
cerr << "failed to set realm as default: " << cpp_strerror(-ret) << std::endl;
return -ret;
Expand Down Expand Up @@ -2791,7 +2791,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = realm.set_as_default();
ret = realm.set_as_default(false);
if (ret < 0) {
cerr << "failed to set realm " << realm_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand Down Expand Up @@ -2856,7 +2856,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = zonegroup.set_as_default();
ret = zonegroup.set_as_default(false);
if (ret < 0) {
cerr << "failed to set zonegroup " << zonegroup_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand All @@ -2881,7 +2881,7 @@ int main(int argc, char **argv)
return -ret;
}

ret = zonegroup.set_as_default();
ret = zonegroup.set_as_default(false);
if (ret < 0) {
cerr << "failed to set zonegroup as default: " << cpp_strerror(-ret) << std::endl;
return -ret;
Expand Down Expand Up @@ -2998,7 +2998,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = zonegroup.set_as_default();
ret = zonegroup.set_as_default(false);
if (ret < 0) {
cerr << "failed to set zonegroup " << zonegroup_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand Down Expand Up @@ -3040,7 +3040,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = zonegroup.set_as_default();
ret = zonegroup.set_as_default(false);
if (ret < 0) {
cerr << "failed to set zonegroup " << zonegroup_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand Down Expand Up @@ -3201,7 +3201,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = zone.set_as_default();
ret = zone.set_as_default(false);
if (ret < 0) {
cerr << "failed to set zone " << zone_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand Down Expand Up @@ -3229,7 +3229,7 @@ int main(int argc, char **argv)
cerr << "unable to initialize zone: " << cpp_strerror(-ret) << std::endl;
return -ret;
}
ret = zone.set_as_default();
ret = zone.set_as_default(false);
if (ret < 0) {
cerr << "failed to set zone as default: " << cpp_strerror(-ret) << std::endl;
return -ret;
Expand Down Expand Up @@ -3362,7 +3362,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = zone.set_as_default();
ret = zone.set_as_default(false);
if (ret < 0) {
cerr << "failed to set zone " << zone_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand Down Expand Up @@ -3458,7 +3458,7 @@ int main(int argc, char **argv)
}

if (set_default) {
ret = zone.set_as_default();
ret = zone.set_as_default(false);
if (ret < 0) {
cerr << "failed to set zone " << zone_name << " as default: " << cpp_strerror(-ret) << std::endl;
}
Expand Down
57 changes: 21 additions & 36 deletions src/rgw/rgw_rados.cc
View file
Expand Up @@ -350,7 +350,7 @@ int RGWZoneGroup::read_default_id(string& default_id, bool old_format)
return RGWSystemMetaObj::read_default_id(default_id, old_format);
}

int RGWZoneGroup::set_as_default()
int RGWZoneGroup::set_as_default(bool exclusive)
{
if (realm_id.empty()) {
/* try using default realm */
Expand All @@ -363,7 +363,7 @@ int RGWZoneGroup::set_as_default()
realm_id = realm.get_id();
}

return RGWSystemMetaObj::set_as_default();
return RGWSystemMetaObj::set_as_default(exclusive);
}

int RGWSystemMetaObj::init(CephContext *_cct, RGWRados *_store, bool setup_obj, bool old_format)
Expand Down Expand Up @@ -441,7 +441,7 @@ int RGWSystemMetaObj::use_default(bool old_format)
return read_default_id(id, old_format);
}

int RGWSystemMetaObj::set_as_default()
int RGWSystemMetaObj::set_as_default(bool exclusive)
{
string pool_name = get_pool_name(cct);
string oid = get_default_oid();
Expand All @@ -454,7 +454,8 @@ int RGWSystemMetaObj::set_as_default()

::encode(default_info, bl);

int ret = rgw_put_system_obj(store, pool, oid, bl.c_str(), bl.length(), false, NULL, real_time(), NULL);
int ret = rgw_put_system_obj(store, pool, oid, bl.c_str(), bl.length(),
exclusive, NULL, real_time(), NULL);
if (ret < 0)
return ret;

Expand Down Expand Up @@ -688,16 +689,7 @@ const string& RGWRealm::get_predefined_name(CephContext *cct) {

int RGWRealm::create(bool exclusive)
{
list<string> realms;
int ret = store->list_realms(realms);
if (ret < 0 && ret != -ENOENT) {
ldout(cct, 0) << "ERROR: listing realms, ret=" << ret << dendl;
return ret;
}

bool first_realm = realms.empty();

ret = RGWSystemMetaObj::create(exclusive);
int ret = RGWSystemMetaObj::create(exclusive);
if (ret < 0) {
ldout(cct, 0) << "ERROR creating new realm object " << name << ": " << cpp_strerror(-ret) << dendl;
return ret;
Expand Down Expand Up @@ -733,12 +725,11 @@ int RGWRealm::create(bool exclusive)
ldout(cct, 0) << "ERROR: failed set current period " << current_period << dendl;
return ret;
}

if (first_realm) { /* this is racy, but it's fine */
ret = set_as_default();
if (ret < 0) {
ldout(cct, 0) << "WARNING: failed to set realm as default realm, ret=" << ret << dendl;
}
// try to set as default, but pass exclusive=true so we don't override an
// existing default
ret = set_as_default(true);
if (ret < 0 && ret != -EEXIST) {
ldout(cct, 0) << "WARNING: failed to set realm as default realm, ret=" << ret << dendl;
}

return 0;
Expand Down Expand Up @@ -1485,15 +1476,9 @@ int RGWZoneParams::fix_pool_names()

int RGWZoneParams::create(bool exclusive)
{
list<string> zones;
int r = store->list_zones(zones);
if (r < 0) {
ldout(cct, 10) << "WARNING: store->list_zones() returned r=" << r << dendl;
}

/* check for old pools config */
rgw_obj obj(domain_root, avail_pools);
r = store->raw_obj_stat(obj, NULL, NULL, NULL, NULL, NULL, NULL);
int r = store->raw_obj_stat(obj, NULL, NULL, NULL, NULL, NULL, NULL);
if (r < 0) {
ldout(store->ctx(), 10) << "couldn't find old data placement pools config, setting up new ones for the zone" << dendl;
/* a new system, let's set new placement info */
Expand All @@ -1516,11 +1501,11 @@ int RGWZoneParams::create(bool exclusive)
return r;
}

if (zones.empty()) { /* first zone? maybe, it's a racy check */
r = set_as_default();
if (r < 0) {
ldout(cct, 10) << "WARNING: failed to set zone as default, r=" << r << dendl;
}
// try to set as default, but pass exclusive=true so we don't override an
// existing default
r = set_as_default(true);
if (r < 0 && r != -EEXIST) {
ldout(cct, 10) << "WARNING: failed to set zone as default, r=" << r << dendl;
}

return 0;
Expand Down Expand Up @@ -1589,7 +1574,7 @@ int RGWZoneParams::read_default_id(string& default_id, bool old_format)
}


int RGWZoneParams::set_as_default()
int RGWZoneParams::set_as_default(bool exclusive)
{
if (realm_id.empty()) {
/* try using default realm */
Expand All @@ -1602,7 +1587,7 @@ int RGWZoneParams::set_as_default()
realm_id = realm.get_id();
}

return RGWSystemMetaObj::set_as_default();
return RGWSystemMetaObj::set_as_default(exclusive);
}

void RGWPeriodMap::encode(bufferlist& bl) const {
Expand Down Expand Up @@ -3358,7 +3343,7 @@ int RGWRados::replace_region_with_zonegroup()
ldout(cct, 0) << "Error creating new realm: " << cpp_strerror(-ret) << dendl;
return ret;
}
ret = new_realm.set_as_default();
ret = new_realm.set_as_default(false);
if (ret < 0) {
ldout(cct, 0) << "Error setting realm as default: " << cpp_strerror(-ret) << dendl;
return ret;
Expand Down Expand Up @@ -3401,7 +3386,7 @@ int RGWRados::replace_region_with_zonegroup()
return ret;
}
if (zonegroup.get_name() == default_region) {
ret = zonegroup.set_as_default();
ret = zonegroup.set_as_default(false);
if (ret < 0) {
ldout(cct, 0) << "failed to set_as_default " << *iter << ": ret "<< ret << " " << cpp_strerror(-ret)
<< dendl;
Expand Down
6 changes: 3 additions & 3 deletions src/rgw/rgw_rados.h
View file
Expand Up @@ -783,7 +783,7 @@ class RGWSystemMetaObj {
}
int init(CephContext *_cct, RGWRados *_store, bool setup_obj = true, bool old_format = false);
virtual int read_default_id(string& default_id, bool old_format = false);
virtual int set_as_default();
virtual int set_as_default(bool exclusive);
int delete_default();
virtual int create(bool exclusive = true);
int delete_obj(bool old_format = false);
Expand Down Expand Up @@ -884,7 +884,7 @@ struct RGWZoneParams : RGWSystemMetaObj {
bool old_format = false);
using RGWSystemMetaObj::init;
int read_default_id(string& default_id, bool old_format = false);
int set_as_default();
int set_as_default(bool exclusive) override;
int create_default(bool old_format = false);
int create(bool exclusive = true);
int fix_pool_names();
Expand Down Expand Up @@ -1151,7 +1151,7 @@ struct RGWZoneGroup : public RGWSystemMetaObj {
}

int read_default_id(string& default_id, bool old_format = false);
int set_as_default();
int set_as_default(bool exclusive) override;
int create_default(bool old_format = false);
int equals(const string& other_zonegroup) const;
int add_zone(const RGWZoneParams& zone_params, bool *is_master, bool *read_only, const list<string>& endpoints);
Expand Down

0 comments on commit a25f266

Please sign in to comment.