Skip to content

librbd: include asio strand headers in librbd.cc#54839

Merged
idryomov merged 1 commit intoceph:mainfrom
cbodley:wip-librbd-strands
Dec 8, 2023
Merged

librbd: include asio strand headers in librbd.cc#54839
idryomov merged 1 commit intoceph:mainfrom
cbodley:wip-librbd-strands

Conversation

@cbodley
Copy link
Contributor

@cbodley cbodley commented Dec 8, 2023

Fixes: https://tracker.ceph.com/issues/63682

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

@cbodley
librbd: include asio strand headers in librbd.cc
4304ca9 
Fixes: https://tracker.ceph.com/issues/63682

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley requested a review from a team as a code owner December 8, 2023 16:26
@github-actions github-actions bot added the rbd label Dec 8, 2023
@cbodley
Copy link
Contributor Author

cbodley commented Dec 8, 2023

before:

~/ceph/build $ nm -C lib/librbd.so | grep -w top_  
00000000007ff4b0 b guard variable for boost::asio::detail::call_stack<boost::asio::detail::strand_service::strand_impl, unsigned char>::top_
00000000007ff4b8 b guard variable for boost::asio::detail::call_stack<boost::asio::detail::thread_context, boost::asio::detail::thread_info_base>::top_
0000000000800760 b guard variable for boost::asio::detail::call_stack<boost::asio::detail::strand_executor_service::strand_impl, unsigned char>::top_         
00000000007ff4c4 b boost::asio::detail::call_stack<boost::asio::detail::strand_service::strand_impl, unsigned char>::top_
00000000007ff4c8 b boost::asio::detail::call_stack<boost::asio::detail::thread_context, boost::asio::detail::thread_info_base>::top_
0000000000800768 b boost::asio::detail::call_stack<boost::asio::detail::strand_executor_service::strand_impl, unsigned char>::top_

after:

~/ceph/build $ nm -C lib/librbd.so | grep -w top_
0000000000801470 u guard variable for boost::asio::detail::call_stack<boost::asio::detail::strand_service::strand_impl, unsigned char>::top_
0000000000801480 u guard variable for boost::asio::detail::call_stack<boost::asio::detail::thread_context, boost::asio::detail::thread_info_base>::top_
0000000000801478 u guard variable for boost::asio::detail::call_stack<boost::asio::detail::strand_executor_service::strand_impl, unsigned char>::top_
000000000080148c u boost::asio::detail::call_stack<boost::asio::detail::strand_service::strand_impl, unsigned char>::top_
0000000000801494 u boost::asio::detail::call_stack<boost::asio::detail::thread_context, boost::asio::detail::thread_info_base>::top_
0000000000801490 u boost::asio::detail::call_stack<boost::asio::detail::strand_executor_service::strand_impl, unsigned char>::top_

@cbodley
Copy link
Contributor Author

cbodley commented Dec 8, 2023

i also tested the interaction with #52495. once that merges, the boost/asio/io_context_strand.hpp include should no longer be necessary

@cbodley cbodley mentioned this pull request Dec 8, 2023
Closed
@cbodley
Copy link
Contributor Author

cbodley commented Dec 8, 2023

prior to commit f479bbc, librbd.cc had indirectly included asio headers via common/error_code.h:

                 from /home/cbodley/ceph/build/boost/include/boost/asio.hpp:34,
                 from /home/cbodley/ceph/src/common/error_code.h:22,                                                                                          
                 from /home/cbodley/ceph/src/include/denc.h:53,                                                                                               
                 from /home/cbodley/ceph/src/include/encoding.h:42,                                                                                           
                 from /home/cbodley/ceph/src/include/uuid.h:9,                                                                                                
                 from /home/cbodley/ceph/src/include/types.h:21,                                                                                              
                 from /home/cbodley/ceph/src/msg/msg_types.h:27,                                                                                              
                 from /home/cbodley/ceph/src/common/ceph_context.h:36,                                                                                        
                 from /home/cbodley/ceph/src/common/dout.h:29,                                                                                                
                 from /home/cbodley/ceph/src/librbd/librbd.cc

it looks like the other rbd sources that include asio headers are linked into librbd.so indirectly through librbd_internal.a which i guess changed how their symbols were exposed

idryomov
idryomov approved these changes Dec 8, 2023
View reviewed changes
Copy link
Contributor

@idryomov idryomov left a comment

Choose a reason for hiding this comment

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

Thanks for following up on this, Casey! In addition to nm output, this passes my local RBD migration test.

@idryomov
Copy link
Contributor

idryomov commented Dec 8, 2023

jenkins test make check

@idryomov
Copy link
Contributor

idryomov commented Dec 8, 2023

jenkins test api

@idryomov idryomov merged commit d5122b9 into ceph:main Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@idryomov idryomov idryomov approved these changes

Assignees

No one assigned

Labels

bug-fix rbd

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cbodley @idryomov