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

tools/rbd_nbd: add --version show support #16254

Merged
merged 1 commit into from Jul 12, 2017

Conversation

Projects
None yet
2 participants
@TsaiJin
Contributor

TsaiJin commented Jul 11, 2017

There exists --version option in the rbd-nbd help info.
However, it doesn't work when run the command `rbd-nbd --version`

Signed-off-by: Jin Cai caijin.caij@alibaba-inc.com

@joscollin joscollin self-requested a review Jul 11, 2017

@TsaiJin

This comment has been minimized.

Contributor

TsaiJin commented Jul 11, 2017

retest this please.

@@ -932,6 +933,9 @@ static int parse_args(vector<const char*>& args, std::ostream *err_msg, Config *
for (i = args.begin(); i != args.end(); ) {
if (ceph_argparse_flag(args, i, "-h", "--help", (char*)NULL)) {
return -ENODATA;
} else if (ceph_argparse_flag(args, i, "-v", "--version", (char*)NULL)) {
std::cout << pretty_version_to_str() << std::endl;
_exit(0);

This comment has been minimized.

@joscollin

joscollin Jul 11, 2017

Member

I don't understand why an error is returned for --help. As per the convention followed here, the option --version also should return an error. But Why can't they return a positive int values to distinguish --help and --version ? Then process inside rbd_nbd() and then return 0 from there ?

This comment has been minimized.

@TsaiJin

TsaiJin Jul 11, 2017

Contributor

@joscollin I change this pull request according to your advice: return a positive int value to distinguish --help and --version and then process inside rbd_nbd()

tools/rbd_nbd: add version show support
Signed-off-by: Jin Cai <caijin.caij@alibaba-inc.com>
@TsaiJin

This comment has been minimized.

Contributor

TsaiJin commented Jul 12, 2017

retest this please.

@joscollin

joscollin approved these changes Jul 12, 2017 edited

Looks Good. This is what exactly I meant. As a result, see the main returns 0 now for both --help and --version, instead of EXIT_FAILURE.

@joscollin joscollin added the needs-qa label Jul 12, 2017

@joscollin joscollin changed the title from tools/rbd_nbd: add version show support to tools/rbd_nbd: add --version show support Jul 12, 2017

@joscollin

This comment has been minimized.

Member

joscollin commented Jul 12, 2017

@TsaiJin main returns 1 for --help though, but that is not related with this PR.

@joscollin

This comment has been minimized.

Member

joscollin commented Jul 12, 2017

Test Result:

[jcollin@earth build]$ ./bin/rbd-nbd -v
ceph version 12.1.0-761-gb7dec695ec (b7dec695ec84718e18f515be5435398fe9085729) luminous (rc)
[jcollin@earth build]$ ./bin/rbd-nbd --version
ceph version 12.1.0-761-gb7dec695ec (b7dec695ec84718e18f515be5435398fe9085729) luminous (rc)
@TsaiJin

This comment has been minimized.

Contributor

TsaiJin commented Jul 12, 2017

@joscollin The return value of --help is because the generic_server_usage() function always exits with status 1

@joscollin joscollin merged commit 82e85a6 into ceph:master Jul 12, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

@TsaiJin TsaiJin deleted the TsaiJin:wip-rbd-nbd-version branch Jul 12, 2017

@joscollin

This comment has been minimized.

Member

joscollin commented Jul 12, 2017

@TsaiJin Yes, I have verified that. It is not related with this PR, so merging this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment