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: should delete in_stream_req if conn->get_obj(...) return not zero value #9950

Merged
merged 1 commit into from May 4, 2017

Conversation

Projects
None yet
5 participants
@weiqiaomiao
Contributor

weiqiaomiao commented Jun 27, 2016

Signed-off-by: weiqiaomiao wei.qiaomiao@zte.com.cn

@weiqiaomiao

This comment has been minimized.

Contributor

weiqiaomiao commented Jun 28, 2016

retest this please

@mattbenjamin mattbenjamin self-assigned this Jul 1, 2016

@@ -6590,7 +6590,7 @@ int RGWRados::fetch_remote_obj(RGWObjectCtx& obj_ctx,
{
/* source is in a different zonegroup, copy from there */
RGWRESTStreamReadRequest *in_stream_req;
RGWRESTStreamReadRequest *in_stream_req = NULL;

This comment has been minimized.

@mattbenjamin

mattbenjamin Jul 1, 2016

Contributor

should be nullptr these days, but ok :)

@@ -6676,6 +6676,9 @@ int RGWRados::fetch_remote_obj(RGWObjectCtx& obj_ctx,
dest_mtime_weight.zone_short_id, dest_mtime_weight.pg_ver,
true, &cb, &in_stream_req);
if (ret < 0) {

This comment has been minimized.

@mattbenjamin

mattbenjamin Jul 1, 2016

Contributor

does look correct

@mattbenjamin

This comment has been minimized.

Contributor

mattbenjamin commented Jul 1, 2016

@cbodley do you have any issue with this?

@cbodley

This comment has been minimized.

Contributor

cbodley commented Jul 1, 2016

the patch looks correct, but we might want to attack the issue in RGWRESTConn::get_obj() instead. if it returns an error, it should clean up the RGWRESTStreamReadRequest itself instead of making the caller do it

@cbodley

This comment has been minimized.

Contributor

cbodley commented Jul 1, 2016

i.e. at the bottom of RGWRESTConn::get_obj():

-  return (*req)->get_obj(key, extra_headers, obj);
+  int r = (*req)->get_obj(key, extra_headers, obj);
+  if (r < 0) {
+    delete *req;
+    *req = nullptr;
+  }
+  return r;
@mattbenjamin

This comment has been minimized.

Contributor

mattbenjamin commented Jul 1, 2016

@cbodley agree; if get_obj failed, there was no reason for it to return something for us to delete
@weiqiaomiao can you revise this?

rgw: should delete in_stream_req if conn->get_obj(...) return not zer…
…o value

Signed-off-by: weiqiaomiao <wei.qiaomiao@zte.com.cn>
@weiqiaomiao

This comment has been minimized.

Contributor

weiqiaomiao commented Jul 4, 2016

@mattbenjamin yes, i revise it.

@weiqiaomiao

This comment has been minimized.

Contributor

weiqiaomiao commented Aug 1, 2016

retest this please

1 similar comment
@weiqiaomiao

This comment has been minimized.

Contributor

weiqiaomiao commented Aug 6, 2016

retest this please

@weiqiaomiao

This comment has been minimized.

Contributor

weiqiaomiao commented Aug 24, 2016

@liewegas liewegas added the needs-qa label Mar 28, 2017

@oritwas

oritwas approved these changes Apr 3, 2017

@oritwas oritwas assigned oritwas and unassigned mattbenjamin Apr 3, 2017

@cbodley cbodley merged commit ff724c9 into ceph:master May 4, 2017

2 checks passed

Signed-off-by all commits in this PR are signed
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment