Hi Felipe,

On Tue, Sep 01, 2015 at 03:23:14PM -0300, Felipe Sateler wrote:
> On 1 September 2015 at 15:13, Jose M Calhariz
> <jose.calha...@tecnico.ulisboa.pt> wrote:
> > Hi Felipe,
> > On Tue, Sep 01, 2015 at 10:25:28AM -0300, Felipe Sateler wrote:
> >> Hi Jose,
> >>
> >> On 1 September 2015 at 07:09, Jose M Calhariz
> >> <jose.calha...@tecnico.ulisboa.pt> wrote:
> >> > Hi,
> >> >
> >> > I have created a systemd service unit for switchconf.  While waiting
> >> > for my sponsor to upload the new version, it's possible to download a
> >> > preview from my blog: http://blog.calhariz.com.
> >
> > I have updated switchconf.service, follow the patch:
> >
> > Index: debian/switchconf.service
> > ===================================================================
> > --- debian/switchconf.service   (revision 2952)
> > +++ debian/switchconf.service   (working copy)
> > @@ -4,7 +4,8 @@
> >  After=local-fs.target
> >  Wants=network-pre.target
> >  Before=network-pre.target
> > -RequiresMountsFor=/var/lock /var/lib
> > +RequiresMountsFor=/var/log /var/lock /var/lib
> > +ConditionKernelCommandLine=switchconf
> >
> >  [Service]
> >  Type=oneshot
> >
> >
> >
> >
> >
> >>
> >> It helps reviewers if you attach a diff or at least the file ;). Some 
> >> comments:
> >>
> >> 1. Please replicate the skip conditions in the service file using
> >> ConditionKernelCommandLine. Check systemd.unit(5) for details. This
> >> way systemd skips your script itself.
> >
> > Done
> >
> >> 2. I think you also need /var/log in RequiresMountsFor
> >
> > I am not certain.  switchconf don't write directly to the /var/log.
> > It uses the command logger to write via syslog.  It is not be better
> > to depend on the syslog service than to depend on /var/log.  I am just
> > asking.
> 
> It seems to me that it does write:
> 
> http://sources.debian.net/src/switchconf/0.0.9-2/switchconf/#L41

Since version 0.0.10, not yet on Debian, it uses logger.  Here is a
fragment from version 0.0.14 that I intend to upload to Debian:

run_scripts(){
    local scriptsdir=$1
    shift
    if test -x "${run_parts}" ; then
        echo "Executing scripts: ";
        ${run_parts} --arg "${conf}" -v "$scriptsdir" 2>&1
        echo "Executing scripts done.";
    else
        echo "Executing scripts: ";
        for r in `find ${scriptsdir} -type f -perm +1 | sort`; do
            echo "$r"
            ${r} "${conf}" 2>&1
        done
        echo "Executing scripts done.";
    fi | logger -s -t switchconf -i -p daemon.notice
}


> 
> >>
> >> Otherwise looks OK!
> >>
> >
> > In attach is the complete switchconf.service, for the people that come
> > later in this discussion.
> 
> Looks good to me!
> 
> 

Thank you for the review.

Kind regards
Jose M Calhariz



-- 
--
        O que e assaltar um banco comparado com fundar um banco?
                -- Bertold Brecht

Attachment: signature.asc
Description: Digital signature

Reply via email to