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.

Sign up for GitHub

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

Jump to bottom

msg: encode entity_addr_t with features #9184

Merged
merged 24 commits into from
Jun 8, 2016
Merged

msg: encode entity_addr_t with features #9184

merged 24 commits into from
Jun 8, 2016

Conversation

liewegas
Copy link
Member

This needs some careful review. Mostly we're just adding arguments, but lots
of opportunity for typos.

dzafman
dzafman reviewed May 19, 2016
View reviewed changes
src/msg/msg_types.h
@@ -365,7 +365,7 @@ struct entity_addr_t {
// broader study


void encode(bufferlist& bl) const {
void encode(bufferlist& bl, uint64_t features) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

How come encode of an entity_addr_t doesn't use the features value?

Copy link
Member Author

Choose a reason for hiding this comment

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

@liewegas liewegas force-pushed the wip-addr-work branch 2 times, most recently from 1617f6c to 1153e96 Compare May 21, 2016 18:20
@liewegas
Copy link
Member Author

retest this please

@liewegas
Copy link
Member Author

test this please

dillaman
dillaman reviewed May 31, 2016
View reviewed changes
src/objclass/class_api.cc
@@ -677,6 +677,12 @@ int cls_current_subop_num(cls_method_context_t hctx)
return ctx->current_osd_subop_num;
}

uint64_t cls_get_features(cls_method_context_t hctx)
Copy link

Choose a reason for hiding this comment

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

Need to add a dummy implementation of this to test/librados_test_sub/librados/LibradosTestStub.cc

Sage Weil and others added 18 commits May 31, 2016 15:32
@liewegas
encoding: featureful list<>
099a654 
Signed-off-by: Sage Weil <sage@inktank.com>
@liewegas
encoding: featureful vector<shared_ptr<>>
27a9e0a 
Signed-off-by: Sage Weil <sage@inktank.com>
@liewegas
encoding: featureful deque<>
82d56e8 
Signed-off-by: Sage Weil <sage@inktank.com>
@liewegas
encoding: featureful unordered_map<>
bc300f3 
Signed-off-by: Sage Weil <sage@inktank.com>
@liewegas
msg/msg_types: optionally accept features when encoding entity_{addr,…
5290133 
…inst}_t

We will eventually require features, but it will take many patches to get
all callers to pass them in.  While we're doing that, behave both with
and without features.

v2:
  Add entity_inst_t::dump() & entity_inst_t::generate_test_instances()

Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
@liewegas
objclass: add cls_get_features
eef9cfa 
Expose cls methods to the current OSDMap features set.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
common/LogEntry: include features in addr encode
3612b92 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
messages/MTimeCheck: encode addrs with features
af17134 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
cls_lock: encode addrs with features
e40dd92 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
cls_rbd: encode addrs with features
16a4d90 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
mon/MonMap: encode addrs with features
29fabdb 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
osd/OSDMap: encode addrs with features
d79714e 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
messages: encode addrs with features
e5318e0 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
osd: encode PullOp, PushOp with features
cdb3f22 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
osd: add OpContext::get_features()
d371d8b 
For convenience.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
osd: encode ObjectRecoveryInfo with features
9538e17 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
osd: encode object_info_t with features
80f83cd 
In a few places, we encode oi with full features.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
osd: encode watch_info_t with features
40f93dd 
Signed-off-by: Sage Weil <sage@redhat.com>
liewegas and others added 5 commits May 31, 2016 15:32
@liewegas
mds/MDSMap: encode addrs with features
6636321 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
mds: encode client_maps with features
9ccd844 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
mds: encode session_info_t with features
133e984 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
mds: ESession: encode addr with features
0abc51f 
Signed-off-by: Sage Weil <sage@redhat.com>
@zhjwpku @liewegas
msg: encode msg/simple and msg/async with features 0
8eb094a 
msg/simple and msg/async should explicitly encode with features 0
since the protocol is defined in terms of the legacy encoding.

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
@yuriw
Copy link
Contributor

yuriw commented May 31, 2016 •
edited
Loading

rbd tests failed

Value of: open_image(m_image_name, &ictx)
Actual: -38
Expected: 0
librbd/AioImageRequestWQ.cc: In function 'void librbd::AioImageRequestWQ::shut_down(Context_)' thread 7f8dd3b4c6c0 time 2016-05-31 18:17:37.122263
librbd/AioImageRequestWQ.cc: 212: FAILED assert(!m_shutdown)
ceph version 10.2.0-1576-g4ee42bc (4ee42bc0660868ccdbbbe62d97c6a1d7cea70e24)
1: (ceph::ceph_assert_fail(char const, char const, int, char const_)+0x8b) [0x7f8dd361ce0b]
2: (librbd::AioImageRequestWQ::shut_down(Context_)+0x2f6) [0x7f8dd3496166]
3: (librbd::image::CloseRequestlibrbd::ImageCtx::send_shut_down_aio_queue()+0xc3) [0x7f8dd34f8743]
4: (librbd::image::CloseRequestlibrbd::ImageCtx::send_unregister_image_watcher()+0x1cd) [0x7f8dd34f8b6d]
5: (librbd::ImageStatelibrbd::ImageCtx::send_close_unlock()+0xae) [0x7f8dd34b1dce]
6: (librbd::ImageStatelibrbd::ImageCtx::close(Context_)+0xa4) [0x7f8dd34b3534]
7: (librbd::ImageStatelibrbd::ImageCtx::close()+0xd2) [0x7f8dd34b37c2]
8: (TestFixture::TearDown()+0x30) [0x7f8dd3386ee0]
9: (TestMockFixture::TearDown()+0x9) [0x7f8dd31cf069]
10: (void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test_, void (testing::Test::)(), char const)+0x65) [0x7f8dd35f00c0]
11: (void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test_, void (testing::Test::)(), char const)+0x4b) [0x7f8dd35eb274]
12: (testing::Test::Run()+0x11e) [0x7f8dd35d2c48]
13: (testing::TestInfo::Run()+0x108) [0x7f8dd35d33f8]
14: (testing::TestCase::Run()+0xf4) [0x7f8dd35d3abc]
15: (testing::internal::UnitTestImpl::RunAllTests()+0x298) [0x7f8dd35da3f0]
16: (bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl_, bool (testing::internal::UnitTestImpl::)(), char const)+0x65) [0x7f8dd35f14a6]
17: (bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl_, bool (testing::internal::UnitTestImpl::)(), char const)+0x4b) [0x7f8dd35ec0bc]
18: (testing::UnitTest::Run()+0xb4) [0x7f8dd35d9016]
19: (main()+0xf8) [0x7f8dd31c3418]
20: (__libc_start_main()+0xf5) [0x7f8dc7478ec5]
21: (()+0x42bb0a) [0x7f8dd31ceb0a]
NOTE: a copy of the executable, or objdump -rdS <executable> is needed to interpret this.
test/run-rbd-unit-tests.sh: line 8: 15116 Aborted unittest_librbd

so untagged for now, per @dillaman

cls_lock isn't being loaded
yuriw: commit 120b053 added a new method "cls_get_features" and that new method wasn't added to librados_test_stub

@liewegas
msg/msg_types: entity_addr_t/entity_inst_t with features
612ccb5 
Since all the call sites need features to encode entity_addr_t,
now it's time to commit this require-features-patch.

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
@liewegas
Copy link
Member Author

fixed cls hook, passes make check for me now, yay!

@yuriw yuriw merged commit f69f1bd into ceph:master Jun 8, 2016
@yuriw
Copy link
Contributor

yuriw commented Jun 8, 2016

test run http://pulpito.ceph.com/yuriw-2016-06-03_10:48:51-rados-wip-yuri-testing2-distro-basic-smithi/

@liewegas liewegas deleted the wip-addr-work branch August 12, 2016 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
common core needs-qa needs-review wip-yuri2-testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@liewegas @yuriw @dzafman @dillaman @zhjwpku