Bug#796699: switchconf: Has init script in runlevel S but no matching service file

[Jose M Calhariz]

By mistake I forgot to add the Close keyword to the changelog.  So
this bug is closed by the upload of version 0.0.15-1.

Kind regards
Jose M Calhariz

-- 
--
A utopia e o derradeiro reduto dos que não desesperaram da
liberdade.
-- Tristao de Ataide Alceu Amoroso Lima


signature.asc
Description: Digital signature

Bug#796699: switchconf: Has init script in runlevel S but no matching service file

[Jose M Calhariz]

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
>  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
> >>  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


signature.asc
Description: Digital signature

Bug#796699: switchconf: Has init script in runlevel S but no matching service file

[Felipe Sateler]

On 1 September 2015 at 15:51, Jose M Calhariz
 wrote:
> 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
>>  wrote:
>> > Hi Felipe,
>> > On Tue, Sep 01, 2015 at 10:25:28AM -0300, Felipe Sateler wrote:

>> >> 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:
>


Ah, sorry I missed it was a different version.

Excellent, then indeed the /var/log in RequiresMountsFor should not be
needed. The logging daemon should take care of making sure /var/log is
writable before writing there.

>>
>> >>
>> >> 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.

Thank you for taking care of switchconf.



-- 

Saludos,
Felipe Sateler

Bug#796699: switchconf: Has init script in runlevel S but no matching service file

[Felipe Sateler]

On 1 September 2015 at 15:13, Jose M Calhariz
 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
>>  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

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

Looks good to me!


-- 

Saludos,
Felipe Sateler

Bug#796699: switchconf: Has init script in runlevel S but no matching service file

[Jose M Calhariz]

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
>  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.


> 
> Otherwise looks OK!
> 

In attach is the complete switchconf.service, for the people that come
later in this discussion.

Kind regards
Jose M Calhariz


-- 
--
O que e assaltar um banco comparado com fundar um banco?
-- Bertold Brecht
[Unit]
Description=Switchconf configuration files during boot
DefaultDependencies=no
After=local-fs.target
Wants=network-pre.target
Before=network-pre.target
RequiresMountsFor=/var/log /var/lock /var/lib
ConditionKernelCommandLine=switchconf

[Service]
Type=oneshot
ExecStart=/etc/init.d/switchconf start

[Install]
WantedBy=sysinit.target


signature.asc
Description: Digital signature

Bug#796699: switchconf: Has init script in runlevel S but no matching service file

[Felipe Sateler]

Hi Jose,

On 1 September 2015 at 07:09, Jose M Calhariz
 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.

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.
2. I think you also need /var/log in RequiresMountsFor

Otherwise looks OK!

-- 

Saludos,
Felipe Sateler