init-ceph: use getopt to make option processing more flexible #6089
Conversation
|
@smithfarm I see you figured out the false negative ;-) Please repush to trigger the bot. |
smithfarm
force-pushed
the
wip-init-ceph-getopt
branch
2 times, most recently
from
3d8d7e7
to
997b3ef
Compare
smithfarm
changed the title
| @@ -1,4 +1,4 @@ | |||
| #!/bin/sh | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
unlike getopts, getopt is an external command. so maybe we don't need it at all? and imho, as a startup script, it would be ideal to be POSIX compliant, and not rely on a certain shell implementation. @smithfarm what do you think?
There was a problem hiding this comment.
@tchaikov The rationale here was that we are not supporting any non-Linux platforms, so why not be explicit about that?
There was a problem hiding this comment.
i mean, user installs a POSIX compliant shell by default, but it could be dash or any other shell implementations. we'd better not force user to install bash even it's ubiquitous. and i don't think, bash is equivalent to GNU/linux.
smithfarm
changed the title
smithfarm
force-pushed
the
wip-init-ceph-getopt
branch
from
316e9dc
to
997b3ef
Compare
smithfarm
changed the title
smithfarm
force-pushed
the
wip-init-ceph-getopt
branch
2 times, most recently
from
3378ce6
to
6b705a7
Compare
|
@tchaikov Good point. Leaving |
|
@smithfarm thanks. but you dropped the allhost change on purpose? |
|
@tchaikov: Yes, I dropped that commit on purpose in response to @liewegas feedback on ceph-devel: "I certainly won't miss it, but I suspect a fair number of people use it (e.g., service ceph -a start). The general strategy is to avoid adding any such hacks to the newer init system scripts, so as everyone moves to systemd this will go away." |
|
@tchaikov: Regarding use of |
|
@smithfarm thanks for referencing the rbdmap init script. but i still believe that a POSIX compliant shell script would help our user:
|
|
lgtm. |
|
@liewegas When you wrote:
did you mean that minor sysvinit issues like this one - http://tracker.ceph.com/issues/3015 - should not be fixed? |
|
@smithfarm if we have a fix and are confident it won't break things then we may as well merge it, but i would spend time in higher priority areas... |
|
@smithfarm besides the trailing space, lgtm |
http://tracker.ceph.com/issues/3015 Fixes: ceph#3015 Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Nathan Cutler <ncutler@suse.com>
smithfarm
force-pushed
the
wip-init-ceph-getopt
branch
from
6b705a7
to
a6d354c
Compare
|
@tchaikov Thanks! Trailing space removed, rebased and repushed. |
init-ceph: use getopt to make option processing more flexible Reviewed-by: Kefu Chai <kchai@redhat.com>
http://tracker.ceph.com/issues/3015 Fixes: #3015