Bug 489378

From the RPM version number, it looks like you are running Fedora 10. I'm changing the version number on this bug to match, since we track based on the version of the whole release rather than the specific component.  Feel free to correct me if I'm wrong.

I'm aware of this problem. Actually it is a bit more complicated as we're starting boinc using the daemon() function from /etc/init.d/functions which redirects all output to /dev/null. And I do not want to store the logs in boinc's data dir (std{out,err}dae.txt) because of SELinux related problems (the logrotate daemon needs log_t context on file to rotate, we'd need to change/supply policy changes etc., moreover I think all logs really should be in /var/log).

I'll try to propose a change to the daemon() function to be able to set some $STDOUT and $STDERR...otherwise I will think of another solution.

...looking more careful into it now I see that I was plain wrong in this, the redirection to /dev/null in daemon() doesn't apply to the started executable itself. This is indeed caused by the "--daemon" option, I'll remove it.

Good news, it was not necessary to change the daemon function.  Take a look at this patch and see what you think.  I removed the --daemon option and used \ on all redirected output so that it is interpreted the way we want it to be.  Prior to this both files were receiving output from the call to the daemon function.  The double quotes around $BOINCEXE where not doing anything either so I removed them as well.

I encountered a file permission problem the first time I executed the new init script.  The boinc user does not have write priviledges on /var/log (makes sense).  The previous log files were created initially by root.  When logrotate runs it creates the new logs correctly with uid:gid, boinc:boinc.  However, if the file is empty it doesn't bother to rotate it.  This was the case with boincerr.log which was still uid:gid, root:root.  After changing this the script worked fine.

You'll need to work something into the package to change log file permissions on existing installations.  A new installation would need to have them set up correctly to begin with.  Maybe it would be better to create a /var/log/boinc directory with a boinc uid and gid.  Something to think about.

Created attachment 334754 [details]
Patch for boinc-client Init Script

Hi William, 

(In reply to comment #4)
> Good news, it was not necessary to change the daemon function.  Take a look at
> this patch and see what you think.  I removed the --daemon option and used \ on
> all redirected output so that it is interpreted the way we want it to be. 
> Prior to this both files were receiving output from the call to the daemon
> function.  The double quotes around $BOINCEXE where not doing anything either
> so I removed them as well.

Yep, the backticks introduce single quotes anyway.

> I encountered a file permission problem the first time I executed the new init
> script.  The boinc user does not have write priviledges on /var/log (makes
> sense).  The previous log files were created initially by root.  When logrotate
> runs it creates the new logs correctly with uid:gid, boinc:boinc.  However, if
> the file is empty it doesn't bother to rotate it.  This was the case with
> boincerr.log which was still uid:gid, root:root.  After changing this the
> script worked fine.
> 
> You'll need to work something into the package to change log file permissions
> on existing installations.  A new installation would need to have them set up
> correctly to begin with.  Maybe it would be better to create a /var/log/boinc
> directory with a boinc uid and gid.  Something to think about.  

yesterday evening I finally decided to almost completely rewrite the pretty old upstream init script (...and revealed many other small bugs), among other things I've added:

# Check that log files exist, otherwise create them with proper ownership
        if [ ! -x $LOGFILE ]; then
                touch $LOGFILE && chown $BOINCUSER:$BOINCUSER $LOGFILE
        fi
        if [ ! -x $ERRORLOG ]; then
                touch $ERRORLOG && chown $BOINCUSER:$BOINCUSER $ERRORLOG
        fi

And regarding older releases, there have been (and unfortunately there will have to be for at least F10/F11) a fix in the spec file for a long time:

#correct wrong owner and group on files under /var/lib/boinc and log files
#caused by bug fixed in 5.10.45-8
chown --silent -R boinc:boinc %{_localstatedir}/log/boinc* \
%{_localstatedir}/lib/boinc/* 2>/dev/null || :

The init script relies now almost completely on the /etc/init.d/functions "library", supports localised messages, doesn't output garbage into logfile, the 'reload' action works again...and moreover it is half so long as it was, which I appreciate at most.

So far I've unpushed the previous release from updates-testing and want to test the new init script hard, as there were indeed so many changes that it is hard to believe I didn't introduce new bugs. I'll post a link to the new version here today, you can test it if you want too. I hope to submit it to updates-testing again by the end of the week or so.

(In reply to comment #6)

>         if [ ! -x $LOGFILE ]; then

Err, shouldn't just copy'n'paste: not -x, but -e of course.

OK, here it is...I've also finally find out why there was a stale init script process left when starting, the daemon line should look as follows:

daemon --check $BOINCEXE --user $BOINCUSER +19 "$BOINCEXE $BOINCOPTS --dir $BOINCDIR >>$LOGFILE 2>>$ERRORLOG &" >& /dev/null

This returns always 0, hence we must check whether boinc is up and running then.

The RPMS are available from:
http://mjakubicek.fedorapeople.org/boinc/

I've downloaded the new RPM and will take a look later today.

Milos, if you want to release the updates for Rawhide, I can also check there.

boinc-client-6.4.7-5.r17542svn.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing-newkey update boinc-client'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-2560

boinc-client-6.4.7-5.r17542svn.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update boinc-client'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-2428

Life has a way of getting busy so it took me longer than expected to test and review the fix.  All features in the new init script are working fine and this does solve the problem with the log files.  There is one problem in the package spec file that should be taken care of.  The /etc/sysconfig/boinc-client configuration file will be overwritten on update which may not be desirable for some installations.  Recommend using the %config(noreplace) option to prevent this from happening.

The rest of my comments are directed toward simplifying the init script even further.  These won't change the functionality, but might make things easier to maintain in the future.  Don't feel you have to make any changes unless you want to.

1) Comments at the top of the script are excessive.  For example, a pidfile is mentioned, but never used and the description is written twice.  
2) On line 70 the pid file portion of the comment can be removed.
3) On line 100, "there're" is not a word, replace with "there are".
4) check_status() function is not needed, it does the same thing as status() in /etc/init.d/functions.
5) restart() function is not needed, most init scripts just use stop, start in the case statement.
6) I felt the message returned from a reload could be better.  Perhaps something along the lines of "Rereading Client Configuration  [OK]"

(In reply to comment #13)
> The /etc/sysconfig/boinc-client
> configuration file will be overwritten on update which may not be desirable for
> some installations.  Recommend using the %config(noreplace) option to prevent
> this from happening.

Definitely yes, thanks!

> 1) Comments at the top of the script are excessive.  For example, a pidfile is
> mentioned, but never used and the description is written twice.

I didn't touch the description much, I'll remove the mention of pid file, but otherwise the description duplication is correct, must be equal and must be there, see our guidelines for initscripts:

https://fedoraproject.org/wiki/Packaging/SysVInitScript#.23_description:_line
https://fedoraproject.org/wiki/Packaging/SysVInitScript#.23_Description:_line_2

> 2) On line 70 the pid file portion of the comment can be removed.

Yes.

> 3) On line 100, "there're" is not a word, replace with "there are".

Yes, of course:)

> 4) check_status() function is not needed, it does the same thing as status() in /etc/init.d/functions.

Oh, yes, I intended to put ">& /dev/null" into check_status() and use it then instead of having to type each time "status $BOINCEXE >& /dev/null"...somehow I forgot to finish the redirection. The redirection is necessary as the status() from /etc/init.d/functions does echo_success/echo_failed.

> 5) restart() function is not needed, most init scripts just use stop, start in
> the case statement.

Yes, this is a relict of some previous version where it had much more lines:) Thanks for noticing.

> 6) I felt the message returned from a reload could be better.  Perhaps
> something along the lines of "Rereading Client Configuration  [OK]"  

Hm, yes, I've considered this and left it as it is because this message is printed directly from the boinc_client, so one would need to make some unstable sedding (the message can change anytime). But I've just found out, that the message is always the same ("retval 0"), even if the cc_config.xml file is corrupted -- some errors are only printed into the errorlog. So I'll throw away the whole output as it doesn't make any sense to keep it when it is so meaningless.

William, thanks for extensive testing, I'll leave now the package in updates-testing for a while (at least a week or so) and then release a new one including all the small changes as discussed above and push it directly to stable updates then.

(In reply to comment #14)

> > 5) restart() function is not needed, most init scripts just use stop, start in
> > the case statement.
> 
> Yes, this is a relict of some previous version where it had much more lines:)
> Thanks for noticing.

Oh, just remembered that this has another reason: restart() is called from other functions, I find it more effective and safer than just doing "$0 restart".

boinc-client-6.4.7-8.r17542svn.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

boinc-client-6.4.7-7.r17542svn.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Confirmed that this is fixed in Rawhide as well; thanks!