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

librbd: small cleanup for recently merged code #20578

Merged
merged 5 commits into from Mar 1, 2018

Conversation

Projects
None yet
2 participants
@trociny
Copy link
Contributor

commented Feb 24, 2018

Signed-off-by: Mykola Golub mgolub@suse.com

@trociny trociny changed the title pybind/rbd: pool id in rbd_group_image_info_t should be int64_t librbd: small cleanup for recently merged code Feb 26, 2018

@trociny trociny force-pushed the trociny:wip-pybind-group branch from f63ab6f to 8e8ae03 Feb 26, 2018

@dillaman
Copy link
Contributor

left a comment

lgtm


names[expected_size] = '\0';
names[expected_size - 1] = '\0';

This comment has been minimized.

Copy link
@dillaman

dillaman Feb 26, 2018

Contributor

If there are no groups, this will fail. I think the expected size just needs to start at 1 byte (for null-terminated array of zero elements).

2018-02-26T20:01:29.490 INFO:tasks.workunit.client.0.smithi132.stderr:test_rbd.test_list_groups_empty ... ==7424== Invalid write of size 1
2018-02-26T20:01:29.490 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== at 0x20B70F53: rbd_group_list (librbd.cc:4533)
2018-02-26T20:01:29.490 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x208C7D05: __pyx_pf_3rbd_3RBD_44group_list (rbd.c:10889)
2018-02-26T20:01:29.490 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x208C7D05: __pyx_pw_3rbd_3RBD_45group_list (rbd.c:10773)
2018-02-26T20:01:29.490 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4E819A2: PyObject_Call (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.490 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4F160F5: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.490 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4F1CEFC: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.490 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4EA6857: ??? (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.491 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4E819A2: PyObject_Call (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.491 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4F155BC: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.491 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4F1A57C: PyEval_EvalFrameEx (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.491 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4F1CEFC: PyEval_EvalCodeEx (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.491 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4EA694C: ??? (in /usr/lib64/libpython2.7.so.1.0)
2018-02-26T20:01:29.491 INFO:tasks.workunit.client.0.smithi132.stderr:==7424== by 0x4E819A2: PyObject_Call (in /usr/lib64/libpython2.7.so.1.0)

This comment has been minimized.

Copy link
@trociny

trociny Feb 27, 2018

Author Contributor

@dillaman I am not sure about returning expected size 1 for empty list.

It looks like rbd_group_list was made similar to rbd_list, and rbd_list returns 0 for the empty list. It looks like it is expected that the caller should check the returned value (see e.g. my "empty directory rbd_list test" commit I added to this PR).

I suppose we want rbd_list and rbd_group_list behave similar, and I suppose it is too late to change rbd_list behavior? So I just deleted names[expected_size] = '\0' which looks like unnecessary here (and rbd_list does not have it).

Additionaly, I added the fix for similar tracepoint leak in rbd_list, and also unified rbd_group_image_list and rbd_group_snap_list behavior.

trociny added some commits Feb 24, 2018

pybind/rbd: pool id in rbd_group_image_info_t should be int64_t
Signed-off-by: Mykola Golub <mgolub@suse.com>
test/librbd: add empty directory rbd_list test
Signed-off-by: Mykola Golub <mgolub@suse.com>
librbd: fix exit tracepoint leaks
Signed-off-by: Mykola Golub <mgolub@suse.com>

@trociny trociny force-pushed the trociny:wip-pybind-group branch from 8e8ae03 to 22aa59d Feb 27, 2018

@dillaman dillaman added the needs-qa label Feb 27, 2018

@trociny trociny force-pushed the trociny:wip-pybind-group branch from 22aa59d to f327784 Feb 27, 2018

@trociny

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2018

@dillaman repushed rbd_group C API tests: I had forgotten about cleanup, there were warnings from valgring.

librbd: normalization for group C API
Signed-off-by: Mykola Golub <mgolub@suse.com>

@trociny trociny force-pushed the trociny:wip-pybind-group branch from f327784 to 0c2d829 Feb 27, 2018

@dillaman dillaman merged commit f9130d4 into ceph:master Mar 1, 2018

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@trociny trociny deleted the trociny:wip-pybind-group branch Mar 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.