Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw: fix disabling Swift's object versioning through empty X-Versions-Location #13303

Conversation

Jing-Scott
Copy link
Contributor

@Jing-Scott Jing-Scott commented Feb 8, 2017

we should be able to disable object verioning by removing its X-Versions-Location metadata header by sending an empty key value. this description can be found at No.8 in http://docs.openstack.org/user-guide/cli-swift-set-object-versions.html.

Fixes: http://tracker.ceph.com/issues/18852
Signed-off-by: Jing Wenjun jingwenjun@cmss.chinamobile.com

@Jing-Scott
Copy link
Contributor Author

@rzarzynski : could you help to take a look?
CC: @cbodley
Thanks a lot :)

@rzarzynski rzarzynski self-assigned this Feb 8, 2017
@Jing-Scott Jing-Scott force-pushed the fix-swift-cannot-disable-object-versioning branch from 89ad8de to 7e1834b Compare February 8, 2017 10:37
we should be able to disable object verioning by removing its X-Versions-Location
metadata header by sending an empty key value. this description can be found at
No.8 in http://docs.openstack.org/user-guide/cli-swift-set-object-versions.html.

Fixes: http://tracker.ceph.com/issues/18852
Signed-off-by: Jing Wenjun <jingwenjun@cmss.chinamobile.com>
Copy link
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally OK. One minor only.

/* If the Swift's versioning is globally disabled but someone wants to
* enable it for a given container, new version of Swift will generate
* the precondition failed error. */
if (! s->cct->_conf->rgw_swift_versioning_enabled) {
return -ERR_PRECONDITION_FAILED;
}

swift_ver_location = std::move(vloc);
swift_ver_location = std::move(s->info.env->get("HTTP_X_VERSIONS_LOCATION", ""));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I would say that std::move isn't necessary anymore. The method already returns a temporary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, you're right! I've updated it. Thanks!

@Jing-Scott Jing-Scott force-pushed the fix-swift-cannot-disable-object-versioning branch from 7e1834b to 17c5a0e Compare February 13, 2017 14:39
Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm (+yehuda)

@mattbenjamin
Copy link
Contributor

@Jing-Scott the thing Yehuda noted was that this change consults the headers map twice, and it could be done once; we'll update after merging

@mattbenjamin mattbenjamin merged commit 24f43e9 into ceph:master Feb 13, 2017
@rzarzynski rzarzynski changed the title rgw: fix swift cannot disable object versioning rgw: fix disabling Swift's object versioning through empty X-Versions-Location Mar 6, 2017
@rzarzynski
Copy link
Contributor

Changed the pull request's name as it was confusing. We did support disabling the Swift's object versioning by sending X-Remove-Versions-Location from the very beginning. The fix implements the second way for doing that -- by supplying an empty X-Versions-Location header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants