Re: init.d-script in 2.4.33pre
Bas van Schaik <[email protected]> Sun, 19 Mar 2006 14:21:16 +0100
| Newsgroups | gmane.linux.enbd.general |
|---|---|
| Message-ID | <[email protected]> |
Peter T. Breuer wrote:
>"Also sprach Bas van Schaik:"
>
>
>>I concur. However, yesterday I posted a small patch on the enbd-init
>>file, but I think the mailing list doesn't support attachments or
>>something...
>>
>>
>
>Probably - I don't know. It certainly has a size limit.
>
>I wouldn't want an attachment, anyway! I want to see the code inline in
>the mail, so I can comment on it. I can't comment if it is an
>attachment.
>
>
Good point.
>>8,10c8,10
>>< OLD_CONFIG="/etc/enbd.conf"
>>< SERVER_CONFIG="/etc/enbd-server.conf"
>>< CLIENT_CONFIG="/etc/enbd-client.conf"
>>---
>>
>>
>>>OLD_CONFIG="${ENBD_CONFIG-/etc/enbd.conf}"
>>>SERVER_CONFIG="${ENBD_SERVER_CONFIG-/etc/enbd-server.conf}"
>>>CLIENT_CONFIG="${ENBD_CLIENT_CONFIG-/etc/enbd-client.conf}"
>>>
>>>
>
>That's as I have it now. Oh, I see ... you have the patch reversed.
>I'll read it backwards.
>
>
Oops :P.
>What's wrong with the form I had? It allows overrides from the
>environment, which is what is wanted in order that the script can
>be be used to launch with different source configs. There's no
>security danger since either a user is allowed to use the port he is
>going to request or he isn't, etc. The daemons are not suid, so
>no privilege escalation can occur.
>
>
I'm sorry, my mistake, I erased too much code. Note that I've made this
patch way before your answers and corrections yesterday, so today it was
not really up-to-date anymore! Some of the corrections I made were
already taken care of.
>>21a22
>>
>>
>>405,406c406,407
>>< [ -r "$SERVER_CONFIG" ] && cat "$SERVER_CONFIG";
>>< [ -r "$CLIENT_CONFIG" ] && cat "$CLIENT_CONFIG";
>>---
>>
>>
>>> [ -r "$SERVER_CONFIG" ] && [ -x "$THESERVER" ] && cat "$SERVER_CONFIG";
>>> [ -r "$CLIENT_CONFIG" ] && [ -x "$THECLIENT" ] && cat "$CLIENT_CONFIG";
>>>
>>>
>
>You want me to NOT check that there is a server to start before going
>through its config file? I'm not sure I agree! We can't pass the arguments
>on to it, so we might as well not look at them. Oh .. I see. You want us
>to fall into the trap later down that makes noise about it all. OK.
>
>
Yes, it seemed more logical to me to make noise if there's a problem and
be quiet if not.
>Anyway, there was a subtlety here that later versions of this script
>addressed: in case the server and client config files are the same,
>we only want to cat one of them into the parser.
>
>
Hmmm... You're right, it's possibe there's only one configfile
("enbd.conf"), and this will be used as both server and client config.
>>433,440c434,435
>>< # client configuration found! Check if client binary is
>>< # available, issue a warning when missing.
>>< if [ ! -x "$THECLIENT" ]; then
>>< echo "Warning: enbd-client is not available, client
>>config ignored"
>>< continue
>>< fi
>><
>>< echo "$rest" | while read device server port options; do
>>---
>>
>>
>>> [ -x "$THECLIENT" ] || continue
>>> echo "$rest" | while read device server port options; do
>>>
>>>
>
>OK - I don't generally like noise from init scripts, but OK.
>Note that this will make noise for EACH client config line found if
>there is no client. Ditto server. A counter would silence it? Added.
>
>
That counter is a good idea, but I don't understand why you don't like
the noise in init-script. I think it's good to warn the user that he's
doing something impossible.
>>480,481c469,472
>>< # exit if both server and client binaries are unavailable
>>< [ -x "$THECLIENT" ] && [ -x "$THESERVER" ] && exit 1
>>---
>>
>>
>>># exit if server is here and there is no server config, etc.
>>>[ ! -r "$CLIENT_CONFIG" ] && [ -x "$THECLIENT" ] && exit 1
>>>[ ! -r "$SERVER_CONFIG" ] && [ -x "$THESERVER" ] && exit 1
>>>
>>>
>
>
>Indeed, those should have been anded. Ho hum .. yes, I suppose you are
>right (modulo adding a "!"). You don't care if they are there, but
>their config files aren't?
>
>
Sure, I can imagine one has installed both client and server packages,
but only wants to use (and configure) the server package. In the old
situation, the init-script would exit before entering the mainloop,
because the client configuration is missing while the daemon is available.
>>Summary of the changes I made:
>>1) I don't think it's correct to exit if there's a server binary
>>installed without a configuration file. It should be the other way
>>around: exit (or warn) when a server config exists, but there's no
>>server binary.
>>
>>
>
>Well, if there is no config file, the server won't be launched. If there
>is no config file for either, then neither server nor client will be
>launched. So what should the init script do? Answer, nothing! It may
>as well exit. So I think that my two statements (when anded together)
>were right. If there is no server config and no client config, then
>exit. If the server or client is absent then they won't be launched
>(and their config file won't be read - or rather, will cause the
>warning you added to be displayed).
>
>I think in other words that
>
> [ ! -x "$THECLIENT" ] && [ ! -x "$THESERVER" ] && exit 1
>
>is correct, and also
>
> [ ! -r "$CLIENT_CONFIG" ] && [ ! -r "$SERVER_CONFIG" ] && exit 1
>
>is correct.
>
>
True!
>>2) Exit if both client and server binaries are unavailable
>>
>>
>
>Fine.
>
>
That's exactly what you've implemented in the statement above, or am I
missing something?
>>3) Issue a warning when server configuration found, but no server binary
>>(see above)
>>
>>
>
>I don't like that - we can have a server config there ready for the day
>when we install a server. I don't think it should cause noise. Maybe
>just one line of noise at max.
>
>
Of course, one line is sufficient.
>>4) Changed the variables (but you already fixed that)
>>
>>As I tried to post yesterday: feel free to use the "patch" above, but
>>feel free also to trash it in /dev/null.
>>
>>
>All taken account of, I hope.
>
>
Yes, thanks!
-- Bas