Skip to content

cls/rgw : Add missing classes in < #include "cls/rgw/cls_rgw_types.h">#45394

Merged
alimaredia merged 1 commit intoceph:masterfrom
iqbalredkhan:amrojiqbal
Apr 14, 2022
Merged

cls/rgw : Add missing classes in < #include "cls/rgw/cls_rgw_types.h">#45394
alimaredia merged 1 commit intoceph:masterfrom
iqbalredkhan:amrojiqbal

Conversation

@iqbalredkhan
Copy link
Contributor

@iqbalredkhan iqbalredkhan commented Mar 15, 2022

New classes are declared in #include "cls/rgw/cls_rgw_types.h"
Missing memeber functions are declared and defined
Fixes: https://tracker.ceph.com/issues/54054
Signed-off-by: Iqbal Khan iqkhan@redhat.com

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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

@github-actions github-actions bot added the rgw label Mar 15, 2022
rgw_cls_bi_entry *m = new rgw_cls_bi_entry;
m->type = BIIndexType::Plain;
m->idx = "idx";
o.push_back(m);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ceph::buffer::list is not initalized in generate_test_instances function. I don't know how to put value in ceph::buffer::list.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. Did you figure this out by running ceph-dencoder in gdb?
I think what you would need is a serialized (encoded()) object of, apparently, the appropriate sort of bucket-index entry (plain, instance, or olh, apparently). So I think what you would do, is, since you appear to be creating a plain entry, instantiate an entry of that type right here, and then call encode() on it, handing it the data member as the buffer list--this will both initialize it and fill it with the serialized data

Copy link
Contributor

Choose a reason for hiding this comment

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

`3 rgw_bucket_dir_entry
2 rgw_bucket_dir_entry_meta
3 rgw_bucket_dir_header
2 rgw_bucket_entry_ver
2 rgw_bucket_olh_entry
2 rgw_bucket_olh_log_entry
2 rgw_bucket_pending_info
2 rgw_cls_bi_entry
2060087: terminate called after throwing an instance of 'ceph::buffer::v15_2_0::end_of_buffer'
2060087: what(): End of buffer
2060091: terminate called after throwing an instance of 'ceph::buffer::v15_2_0::end_of_buffer'
2060091: what(): End of buffer
2060082: terminate called after throwing an instance of 'ceph::buffer::v15_2_0::end_of_buffer'
2060082: what(): End of buffer
2060094: terminate called after throwing an instance of 'ceph::buffer::v15_2_0::end_of_buffer'
2060094: what(): End of buffer
2060091: /lv2tb/ceph-cp/qa/standalone/ceph-helpers.sh: line 2080: 2060093 Aborted (core dumped) "$@" > "$out"
2060082: /lv2tb/ceph-cp/qa/standalone/ceph-helpers.sh: line 2080: 2060084 Aborted (core dumped) "$@" > "$out"
2060094: /lv2tb/ceph-cp/qa/standalone/ceph-helpers.sh: line 2080: 2060095 Aborted (core dumped) "$@" > "$out"
2060087: /lv2tb/ceph-cp/qa/standalone/ceph-helpers.sh: line 2080: 2060089 Aborted (core dumped) "$@" > "$out"

so it looks like we could run the various point ceph-dencoder tests
like:

[mbenjamin@lemon build]$ ceph-dencoder type "rgw_cls_bi_entry" select_test "1" dump_json
terminate called after throwing an instance of 'ceph::buffer::v15_2_0::end_of_buffer'
what(): End of buffer
Aborted (core dumped)

(there are 3 more forms:
ceph-dencoder type "rgw_cls_bi_entry" select_test "1" encode decode dump_json
ceph-dencoder type "rgw_cls_bi_entry" select_test "1" copy dump_json
ceph-dencoder type "rgw_cls_bi_entry" select_test "1" copy_ctor dump_json

all of these currently abort due to "end_of_buffer" errors
)

A typical way to get some more info on what is happening in an abort
would be to run the program under the debugger:

mbenjamin@lemon build]$ gdb ceph-dencoder
GNU gdb (GDB) Fedora 9.1-7.fc32
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
http://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ceph-dencoder...
(gdb) run type "rgw_cls_bi_entry" select_test "1" dump_json
Starting program: /lv2tb/ceph-cp/build/bin/ceph-dencoder type "rgw_cls_bi_entry" select_test "1" dump_json
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.31-6.fc32.x86_64
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
terminate called after throwing an instance of 'ceph::buffer::v15_2_0::end_of_buffer'
what(): End of buffer

Program received signal SIGABRT, Aborted.
0x00007ffff68447d5 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install fuse-libs-2.9.9-9.fc32.x86_64 gperftools-libs-2.7-7.fc32.x86_64 libaio-0.3.111-7.fc32.x86_64 libcurl-7.69.1-8.fc32.x86_64 libgcc-10.3.1-1.fc32.x86_64 libstdc++-10.3.1-1.fc32.x86_64 libunwind-1.3.1-7.fc32.x86_64 libxcrypt-4.4.20-2.fc32.x86_64 openssl-libs-1.1.1k-1.fc32.x86_64 pcre2-10.36-4.fc32.x86_64 snappy-1.1.8-2.fc32.x86_64 sqlite-libs-3.34.0-1.fc32.x86_64 systemd-libs-245.9-1.fc32.x86_64
(gdb) up
#1 0x00007ffff682d895 in abort () from /lib64/libc.so.6
(gdb) up
#2 0x00007ffff6bd0941 in __gnu_cxx::__verbose_terminate_handler() [clone .cold] () from /lib64/libstdc++.so.6
(gdb) up
#3 0x00007ffff6bdc32c in __cxxabiv1::__terminate(void ()()) () from /lib64/libstdc++.so.6
(gdb) up
#4 0x00007ffff6bdc397 in std::terminate() () from /lib64/libstdc++.so.6
(gdb) up
#5 0x00007ffff6bdc649 in __cxa_throw () from /lib64/libstdc++.so.6
(gdb) up
#6 0x00007ffff77f8954 in ceph::buffer::v15_2_0::list::iterator_impl::copy (this=this@entry=0x7fffffffc7a0, len=len@entry=1,
dest=dest@entry=0x7fffffffc6c7 "") at /lv2tb/ceph-cp/src/include/buffer.h:478
478 bool operator==(const buffers_iterator& rhs) const {
(gdb) up
#7 0x00007ffff3fcf72a in ceph::decode_raw (p=..., t=@0x7fffffffc6c7: 0 '\000')
at /lv2tb/ceph-cp/src/include/encoding.h:86
86 WRITE_RAW_ENCODER(__u8)
(gdb) up
#8 ceph::decode (p=..., v=@0x7fffffffc6c7: 0 '\000') at /lv2tb/ceph-cp/src/include/encoding.h:86
86 WRITE_RAW_ENCODER(__u8)
(gdb) up
#9 rgw_bucket_dir_entry::decode (this=0x7fffffffc7c0, bl=...) at /lv2tb/ceph-cp/src/cls/rgw/cls_rgw_types.h:479
479 DECODE_START_LEGACY_COMPAT_LEN(8, 3, 3, bl);
(gdb) up
#10 0x00007ffff403af1d in decode (p=..., c=...) at /lv2tb/ceph-cp/src/cls/rgw/cls_rgw_types.h:532
532 WRITE_CLASS_ENCODER(rgw_bucket_dir_entry)
(gdb) up
#11 dump_bi_entry (bl=..., index_type=index_type@entry=BIIndexType::Plain, formatter=formatter@entry=0x7fffffffcdf0)
at /lv2tb/ceph-cp/src/cls/rgw/cls_rgw_types.cc:290
290 decode(entry, iter);
(gdb) up
#12 0x00007ffff403b1df in rgw_cls_bi_entry::dump (this=0x555555e38460, f=0x7fffffffcdf0) at /lv2tb/ceph-cp/src/include/buffer.h:594
594 ptr_node
clone = ptr_node::cloner()(node);
(gdb) up
#13 0x00007ffff3f8dd15 in DencoderBase<rgw_cls_bi_entry>::dump (this=, f=)
at /lv2tb/ceph-cp/src/tools/ceph-dencoder/denc_registry.h:78
78 void dump(ceph::Formatter *f) override {
(gdb) list
73 return {};
74 }
75
76 void encode(bufferlist& out, uint64_t features) override = 0;
77
78 void dump(ceph::Formatter *f) override {
79 m_object->dump(f);
80 }
81 void generate() override {
82 T::generate_test_instances(m_list);
(gdb) up
#14 0x00005555555616ab in main (argc=, argv=)
at /lv2tb/ceph-cp/src/tools/ceph-dencoder/ceph_dencoder.cc:189
189 den->dump(&jf);
(gdb) p jf
$1 = {ceph::Formatter = {_vptr.Formatter = 0x55555556f720 <vtable for ceph::JSONFormatter+16>}, m_pretty = true,
m_ss = {<std::__cxx11::basic_stringstream<char, std::char_traits, std::allocator >> = {}, },
m_pending_string = {<std::__cxx11::basic_stringstream<char, std::char_traits, std::allocator >> = {}, }, m_pending_name = "", m_stack = std::__cxx11::list = {[0] = {size = 2, is_array = false}}, m_is_pending_string = false,
m_line_break_enabled = false}
(gdb) list
184 cerr << "must first select type with 'type '" << std::endl;
185 return 1;
186 }
187 JSONFormatter jf(true);
188 jf.open_object_section("object");
189 den->dump(&jf);
190 jf.close_section();
191 jf.flush(cout);
192 cout << std::endl;
193
(gdb) p den
$2 = (Dencoder *) 0x555555e4a0c0
(gdb) p *den
$3 = {_vptr.Dencoder = 0x7ffff4b53108 <vtable for DencoderImplNoFeature<rgw_cls_bi_entry>+16>}
(gdb)
(gdb) b rgw_cls_bi_entry::generate_test_instances
Breakpoint 1 at 0x7ffff3f7aad0 (2 locations)
(gdb) run type "rgw_cls_bi_entry" select_test "1" dump_json
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /lv2tb/ceph-cp/build/bin/ceph-dencoder type "rgw_cls_bi_entry" select_test "1" dump_json
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, 0x00007ffff3f7aad0 in rgw_cls_bi_entry::generate_test_instances(std::__cxx11::list<rgw_cls_bi_entry*, std::allocator<rgw_cls_bi_entry*> >&)@plt () from /opt/ceph-3/lib64/ceph/denc/denc-mod-rgw.so
(gdb) list
194 } else if (*i == string("hexdump")) {
195 encbl.hexdump(cout);
196 } else if (*i == string("get_struct_v")) {
197 std::cout << den->get_struct_v(encbl, 0) << std::endl;
198 } else if (*i == string("get_struct_compat")) {
199 std::cout << den->get_struct_v(encbl, sizeof(uint8_t)) << std::endl;
200 } else if (*i == string("import")) {
201 ++i;
202 if (i == args.end()) {
203 cerr << "expecting filename" << std::endl;
(gdb) n
Single stepping until exit from function _ZN16rgw_cls_bi_entry23generate_test_instancesERNSt7__cxx114listIPS_SaIS2_EEE@plt,
which has no line number information.

Breakpoint 1, rgw_cls_bi_entry::generate_test_instances (o=empty std::__cxx11::list) at /lv2tb/ceph-cp/src/cls/rgw/cls_rgw_types.cc:408
408 {
(gdb) list
403 }
404
405 return account;
406 }
407 void rgw_cls_bi_entry::generate_test_instances(list<rgw_cls_bi_entry*>& o)
408 {
409 rgw_cls_bi_entry *m = new rgw_cls_bi_entry;
410 m->type = BIIndexType::Plain;
411 m->idx = "idx";
412 o.push_back(m);
(gdb) n
409 rgw_cls_bi_entry *m = new rgw_cls_bi_entry;
(gdb) n
410 m->type = BIIndexType::Plain;
(gdb) n
411 m->idx = "idx";
(gdb) n
412 o.push_back(m);
(gdb) n
413 o.push_back(new rgw_cls_bi_entry);
(gdb) n
main (argc=, argv=) at /lv2tb/ceph-cp/src/tools/ceph-dencoder/ceph_dencoder.cc:134
134 string cname = i;
(gdb) b rgw_cls_bi_entry::dump
Breakpoint 2 at 0x7ffff3f6dcd0 (2 locations)
(gdb) n
907 length() const _GLIBCXX_NOEXCEPT
(gdb) n
116 string err;
(gdb)
115 for (std::vector<const char
>::iterator i = args.begin(); i != args.end(); ++i) {
(gdb) n
116 string err;
(gdb) n
119 if (*i == string("help") || *i == string("-h") || *i == string("--help")) {
(gdb)
119 if (*i == string("help") || *i == string("-h") || *i == string("--help")) {
(gdb)
119 if (*i == string("help") || *i == string("-h") || *i == string("--help")) {
(gdb)
79 new_allocator() _GLIBCXX_USE_NOEXCEPT { }
(gdb) c
Continuing.

Breakpoint 2, 0x00007ffff3f6dcd0 in rgw_cls_bi_entry::dump(ceph::Formatter*) const@plt ()
from /opt/ceph-3/lib64/ceph/denc/denc-mod-rgw.so
(gdb) n
Single stepping until exit from function _ZNK16rgw_cls_bi_entry4dumpEPN4ceph9FormatterE@plt,
which has no line number information.

Breakpoint 2, rgw_cls_bi_entry::dump (this=0x555555e38460, f=0x7fffffffcdf0) at /lv2tb/ceph-cp/src/cls/rgw/cls_rgw_types.cc:350
350 {
(gdb) list
345 break;
346 }
347 }
348
349 void rgw_cls_bi_entry::dump(Formatter *f) const
350 {
351 string type_str;
352 switch (type) {
353 case BIIndexType::Plain:
354 type_str = "plain";
(gdb) list
355 break;
356 case BIIndexType::Instance:
357 type_str = "instance";
358 break;
359 case BIIndexType::OLH:
360 type_str = "olh";
361 break;
362 default:
363 type_str = "invalid";
364 }
(gdb) n
351 string type_str;
(gdb) n
352 switch (type) {
(gdb)
354 type_str = "plain";
(gdb)
365 encode_json("type", type_str, f);
(gdb)
366 encode_json("idx", idx, f);
(gdb) n
367 dump_bi_entry(data, type, f);
(gdb) n
terminate called after throwing an instance of 'ceph::buffer::v15_2_0::end_of_buffer'
what(): End of buffer

Program received signal SIGABRT, Aborted.
0x00007ffff68447d5 in raise () from /lib64/libc.so.6
(gdb)
`

Copy link
Contributor

Choose a reason for hiding this comment

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

this looks viable to me, and I know it compiles; does it run? :)

entry->exists = true;
entry->pending_removal = true;
o.push_back(entry);
o.push_back(new rgw_bucket_olh_entry);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

std::map<uint64_t, std::vector > pending_log is not initalized in this function. I am facing difficulties in initalizing this variable.

@iqbalredkhan
Copy link
Contributor Author

jenkins retest this please

@iqbalredkhan
Copy link
Contributor Author

jenkins test make check

1 similar comment
@kalebskeithley
Copy link
Contributor

jenkins test make check

@mattbenjamin
Copy link
Contributor

@alimaredia what do you think?

@iqbalredkhan
Copy link
Contributor Author

@mattbenjamin @alimaredia , I have initalized ceph::buffer::list in generate_test_instances function. Please review once

Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

just commenting for now, unless we've finished all types?
I think this change for rgw_cls_bi_entry::generate_test_instances(...) looks sane, can you tell us whether it runs?

@iqbalredkhan
Copy link
Contributor Author

jenkins test make check

@mattbenjamin mattbenjamin self-assigned this Apr 6, 2022
@mattbenjamin mattbenjamin self-requested a review April 6, 2022 11:05
Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

ah, that make sense, if we mis-matched the type of object being encoded vs decoded, it would run the wrong decoding logic. Please squash the last two commits into the 2nd one, please (use rebase -i with the squash option), or else give them meaningful commit messages.

@iqbalredkhan
Copy link
Contributor Author

jenkins test make check

@iqbalredkhan
Copy link
Contributor Author

ah, that make sense, if we mis-matched the type of object being encoded vs decoded, it would run the wrong decoding logic. Please squash the last two commits into the 2nd one, please (use rebase -i with the squash option), or else give them meaningful commit messages.

Yes, I had squashed all commits into one commit. @mattbenjamin, and it's working fine

@mattbenjamin
Copy link
Contributor

@alimaredia can you have a look?

@alimaredia alimaredia self-requested a review April 6, 2022 22:31
@alimaredia
Copy link
Contributor

@iqbalredkhan this looks great! could you change the header to the commit to add the subsystem of Ceph to it.
In your case Missing functions and TYPES of cls_rgw_types.h are added becomes rgw: Missing functions and TYPES of cls_rgw_types.h are added

@iqbalredkhan
Copy link
Contributor Author

yes, @alimaredia , I changed header of commit

@iqbalredkhan this looks great! could you change the header to the commit to add the subsystem of Ceph to it. In your case Missing functions and TYPES of cls_rgw_types.h are added becomes rgw: Missing functions and TYPES of cls_rgw_types.h are added

yes, @alimaredia , I changed header of commit

@iqbalredkhan
Copy link
Contributor Author

I ran my code changes in teuthology qa/tests, and it passed 46 out of 49.
waiting to be reviewed @alimaredia @mattbenjamin

https://pulpito.ceph.com/iqbalredkhan-2022-04-12_17:23:11-rgw-wip-iqbal-add-missing-cls-rgw-classes-distro-default-smithi/

@alimaredia alimaredia merged commit 5c1c754 into ceph:master Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants