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
tool: misc cleanup of ceph-kvstore-tool #18815
Conversation
jenkins test this please |
src/tools/ceph_kvstore_tool.cc
Outdated
} | ||
std::ofstream fs(argv[4]); | ||
uint32_t crc = st.traverse(string(), true, &fs); | ||
fs.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"fix" is too vague for me. could you expand a little bit in the commit message? iiuc, you are passing a never used ostream to traverse()
, why?
i see, argv[4]
is the output arg.
also, fs
will be closed when it's destroyed. i don't think this call is really necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you, kefu, updated!
…e file as expected "store-crc <path>" command did not save result to <path>, it didn't use <path> argument and alwayls passed a NULL to "st.traverse" function. now we will create file <path>, pass a *ofstream to "st.traverse" function. Fiexes: http://tracker.ceph.com/issues/22092 Signed-off-by: Chang Liu <liuchang0812@gmail.com>
Signed-off-by: Chang Liu <liuchang0812@gmail.com>
0432b3e
to
1f1a6f1
Compare
usage(argv[0]); | ||
return 1; | ||
} | ||
std::ofstream fs(argv[4]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be argv[3]
. as cmd is argv[2]
.
could you use nvm, seems we are using args[3]
instead?argv
everywhere else ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be argv[4].
➜ build git:(wip-kv-store-crc) ✗ ./bin/ceph-kvstore-tool bluestore-kv dev/osd2 store-crc t2.txt 2>/dev/null
store at 't2.txt' crc 4037852868
store-crc <path>
command. this command did not do anything before.@tchaikov @liewegas would you mind taking a look? thanks