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

librados: cleanups #24896

Merged
merged 10 commits into from
Nov 8, 2018
Merged

librados: cleanups #24896

merged 10 commits into from
Nov 8, 2018

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Nov 2, 2018

changes from the wishlist in https://pad.ceph.com/p/librados3

@tchaikov tchaikov requested a review from jcsp November 2, 2018 14:07
@tchaikov tchaikov force-pushed the wip-librados-cleanup branch 6 times, most recently from 5640d6a to 2a7e7ce Compare November 2, 2018 17:11
debian/control Outdated
@@ -589,7 +583,7 @@ Description: RADOS striping interface (development files)
Package: librbd1
Architecture: linux-any
Section: libs
Depends: librados3 (= ${binary:Version}),
Copy link
Member

Choose a reason for hiding this comment

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

@dillaman any issues with CephContext or other structures here?

Copy link

@dillaman dillaman Nov 2, 2018

Choose a reason for hiding this comment

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

Yeah -- they need to be version linked (or the cct() method needs to be yanked and we need to figure out a new way to do logging)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh, i see. we take advantage of the fact that librados::config_t is actually CephContext* in (anonymous_namespace)::get_cct().

Copy link
Member

@jdurgin jdurgin left a comment

Choose a reason for hiding this comment

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

lgtm aside from the question about librbd exact version dependence

@jdurgin
Copy link
Member

jdurgin commented Nov 2, 2018

looks like the make check failure is real:
../../../lib/librbd_internal.a(internal.cc.o): In function `librbd::tmap_set(librados::IoCtx&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)': /home/jenkins-build/build/workspace/ceph-pull-requests/src/librbd/internal.cc:285: undefined reference to `librados::IoCtx::tmap_update(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, ceph::buffer::list&)' ../../../lib/librbd_internal.a(internal.cc.o): In function `librbd::tmap_rm(librados::IoCtx&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)': /home/jenkins-build/build/workspace/ceph-pull-requests/src/librbd/internal.cc:294: undefined reference to `librados::IoCtx::tmap_update(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, ceph::buffer::list&)' ../../../lib/librbd_internal.a(RenameRequest.cc.o): In function `librbd::operation::RenameRequest<librbd::ImageCtx>::send_update_directory()': /home/jenkins-build/build/workspace/ceph-pull-requests/src/librbd/operation/RenameRequest.cc:175: undefined reference to `librados::ObjectWriteOperation::tmap_update(ceph::buffer::list const&)'

@tchaikov tchaikov force-pushed the wip-librados-cleanup branch 2 times, most recently from 42b8617 to 235b014 Compare November 3, 2018 02:37
@tchaikov tchaikov force-pushed the wip-librados-cleanup branch 2 times, most recently from 302bfbf to 2f3f022 Compare November 3, 2018 14:39
@tchaikov
Copy link
Contributor Author

tchaikov commented Nov 4, 2018

@tchaikov
Copy link
Contributor Author

tchaikov commented Nov 7, 2018

retest this please.

- python-* packages are using the C APIs which are not changed across
librados2 and librados3.

Signed-off-by: Kefu Chai <kchai@redhat.com>
this change reverts cac1d6f

Signed-off-by: Kefu Chai <kchai@redhat.com>
this change partially reverts 6eca7d0

Signed-off-by: Kefu Chai <kchai@redhat.com>
this change partially reverts 6eca7d0

Signed-off-by: Kefu Chai <kchai@redhat.com>
this change reverts b8ff781

Signed-off-by: Kefu Chai <kchai@redhat.com>
we have switched from tmap to omap long ago.

but keep the server side implementation around, in case ancient
client is still using these tmap APIs.

also, tmap_update() is kept, because librbd is using it for v1 image
backward compatibility.

Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

tchaikov commented Nov 7, 2018

changelog:

  • add a commit to fix the dependency of libradospp-dev: 70a26ed

it should address the failure of http://pulpito.ceph.com/kchai-2018-11-07_07:51:37-rados-wip-kefu2-testing-2018-11-04-1018-distro-basic-smithi/3233734/ , where we have

2018-11-07T08:20:28.949 INFO:tasks.workunit.client.0.smithi158.stdout:The following information may help to resolve the situation:
2018-11-07T08:20:28.949 INFO:tasks.workunit.client.0.smithi158.stdout:
2018-11-07T08:20:28.949 INFO:tasks.workunit.client.0.smithi158.stdout:The following packages have unmet dependencies:
2018-11-07T08:20:28.995 INFO:tasks.workunit.client.0.smithi158.stdout: libradospp-dev : Depends: libradospp (= 14.0.0-4787-g9116858-1bionic) but it is not installable
2018-11-07T08:20:29.011 INFO:tasks.workunit.client.0.smithi158.stderr:E: Unable to correct problems, you have held broken packages.
2018-11-07T08:20:29.013 DEBUG:teuthology.orchestra.run:got remote process result: 100
  • add a commit to install libradospp-dev for testing librados*-{dev,devel}

it should address the failure of http://pulpito.ceph.com/kchai-2018-11-07_07:51:37-rados-wip-kefu2-testing-2018-11-04-1018-distro-basic-smithi/3233772/, where we have

2018-11-07T08:59:41.067 INFO:tasks.workunit.client.0.smithi150.stdout:g++ -std=c++11 -Wno-unused-parameter -Wall -Wextra -Werror -g   -o hello_world_cpp hello_world.cc -lrados -lradosstriper
2018-11-07T08:59:41.401 INFO:tasks.workunit.client.0.smithi150.stderr:hello_world.cc:14:10: fatal error: rados/librados.hpp: No such file or directory
2018-11-07T08:59:41.401 INFO:tasks.workunit.client.0.smithi150.stderr: #include <rados/librados.hpp>
2018-11-07T08:59:41.401 INFO:tasks.workunit.client.0.smithi150.stderr:          ^~~~~~~~~~~~~~~~~~~~
2018-11-07T08:59:41.401 INFO:tasks.workunit.client.0.smithi150.stderr:compilation terminated.
2018-11-07T08:59:41.406 INFO:tasks.workunit.client.0.smithi150.stdout:Makefile:29: recipe for target 'hello_world_cpp' failed
2018-11-07T08:59:41.406 INFO:tasks.workunit.client.0.smithi150.stderr:make: *** [hello_world_cpp] Error 

libradospp-{dev,devel} is necessary for compiling sources in
examples/librados/hello_world.cc

Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
because librados.hpp `#include`s librados.h

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov tchaikov merged commit 0966f43 into ceph:master Nov 8, 2018
@tchaikov tchaikov deleted the wip-librados-cleanup branch November 8, 2018 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants