Re: [systemd-devel] systemd doesn't see ttyPS0 devices from udev

[Michael Olbrich]

Hi,

On Wed, Sep 23, 2020 at 04:58:57PM +0800, ZhouPeng wrote:
> I find that boot with the 'rescue' option, then exit 'rescue' mode, then
> I can login to the ttyPS0, as below

You're using a custom kernel, right? Please check the README in the systemd
source tree. Specifically the kernel config options.

From what I remember, what you describe happens when CONFIG_FHANDLE is
disabled in the kernel.

Regards,
Michael


> bash-4.4# exit  // *here exit rescue mode*
> exit
> Reloading system manager configuration
> Starting default target
> [ 1708.91] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> [ 1712.08] macb e000b000.ethernet eth0: link up (100/Full)
> [ 1712.08] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> ...
> Build date: 2019-08-22 21:56
> 
> 
> Kernel 4.18.0-14480-gc4490f2dabc1 on an riscv64 (ttyPS0)
> ...
> stage4 login: root
> Password: 
> [root@stage4 ~]#
> 
> 
> 
> 
> But how boot directly normally.
> Thanks all,
> At 2020-09-23 16:37:48, "ZhouPeng"  wrote:
> >Thank you very much.  ' udevadm info -a /dev/ttyPS0' output:
> >
> >
> >bash-4.4# udevadm info -a /dev/ttyPS0
> >
> >Udevadm info starts with the device specified by the devpath and then
> >walks up the chain of parent devices. It prints for every device
> >found, all possible attributes in the udev rules key format.
> >A rule to match, can be composed by the attributes of the device
> >and the attributes from one single parent device.
> >
> >  looking at device '/devices/platform/soc/e000.serial/tty/ttyPS0':
> >KERNEL=="ttyPS0"
> >SUBSYSTEM=="tty"
> >DRIVER==""
> >ATTR{close_delay}=="50"
> >ATTR{closing_wait}=="3000"
> >ATTR{custom_divisor}=="0"
> >ATTR{flags}=="0x1000"
> >ATTR{io_type}=="3"
> >ATTR{iomem_base}=="0xE000"
> >ATTR{iomem_reg_shift}=="0"
> >ATTR{irq}=="2"
> >ATTR{line}=="0"
> >ATTR{port}=="0x0"
> >ATTR{type}=="98"
> >ATTR{uartclk}=="1"
> >ATTR{xmit_fifo_size}=="64"
> >
> >  looking at parent device '/devices/platform/soc/e000.serial':
> >KERNELS=="e000.serial"
> >SUBSYSTEMS=="platform"
> >DRIVERS=="xuartps"
> >ATTRS{driver_override}=="(null)"
> >
> >  looking at parent device '/devices/platform/soc':
> >KERNELS=="soc"
> >SUBSYSTEMS=="platform"
> >DRIVERS==""
> >ATTRS{driver_override}=="(null)"
> >
> >  looking at parent device '/devices/platform':
> >KERNELS=="platform"
> >SUBSYSTEMS==""
> >DRIVERS==""
> >bash-4.4# ls /dev/ttyPS0 -lh
> >crw-rw 1 root dialout 254, 0 Jan 28 16:05 /dev/ttyPS0
> >
> >
> >Thanks,
> >
> >On Wed, Sep 23, 2020, 15:34，"Mantas Mikulėnas"  wrote:
> >
> >
> >On Wed, Sep 23, 2020, 09:21 ZhouPeng  wrote:
> >
> >
> >
> >Thank you  very much for you great suggestions.
> >
> >
> >
> >I chroot the rootfs and tried to the 3 methods in '/usr/lib/udev/rules.d'  
> >respectively:
> >
> >
> >
> >try 1) add  a line of  ACTION!="remove", KERNEL=="ttyPS0", TAG+="systemd" 
> >below the line of  "ACTION=="remove", GOTO="systemd_end"" in file 
> >99-systemd.rules
> >
> >try 2) add  a line of  ACTION!="remove", KERNEL=="ttyPS0",  NAME="ttyPS0", 
> >TAG+="systemd"  below the line of  "ACTION=="remove", GOTO="systemd_end"" in 
> >file 99-systemd.rules
> >
> >try 3) replace the line  "SUBSYSTEM=="tty", 
> >KERNEL=="tty[a-zA-Z]*|hvc*|xvc*|hvsi*|ttysclp*|sclp_line*|3270/tty[0-9]*", 
> >TAG+="systemd" with "SUBSYSTEM=="tty", 
> >KERNEL=="ttyPS[0-9]|tty[a-zA-Z]*|hvc*|xvc*|hvsi*|ttysclp*|sclp_line*|3270/tty[0-9]*",
> > TAG+="systemd"",  in file 99-systemd.rules.
> >
> >
> >
> >At least the 1st one *should* have worked, though NAME= seems wrong in the 
> >2nd one...
> >
> >
> >
> >
> >
> >
> >
> >At the same time, I replace the line of 
> >"KERNEL=="tty[A-Z]*[0-9]|ttymxc[0-9]*|pppox[0-9]*|ircomm[0-9]*|noz[0-9]*|rfcomm[0-9]*",
> > GROUP="dialout"" with 
> >"KERNEL=="ttyPS[0-9]|tty[A-Z]*[0-9]|ttymxc[0-9]*|pppox[0-9]*|ircomm[0-9]*|noz[0-9]*|rfcomm[0-9]*",
> > GROUP="dialout"".
> >
> >
> >
> >This line is irrelevant, since TAG+="systemd" is the important part – groups 
> >and modes shouldn't be the problem.
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >But they didn't take any effect.
> >
> >
> >
> >Then at the same time, I do
> >
> >cp /usr/lib/systemd/system/serial-getty@.service 
> >/etc/systemd/system/serial-getty@ttyPS0.service
> >
> >Edit /etc/systemd/system/serial-getty\@ttyPS0.service:  replace  
> >"ExecStart=-/sbin/agetty --keep-baud 115200,38400,9600 %I $TERM" with 
> >"ExecStart=-/sbin/agetty --keep-baud 115200 %I $TERM"
> >
> >ln -s /etc/systemd/system/serial-getty@ttyPS0.service 
> >/etc/systemd/system/getty.target.wants/
> >
> >
> >
> >But there was still no effect. There is still boot failure logs like:
> >
> >[ *] (3 of 3) a start job is running for dev-ttyPS0.device (41s / 1min 
> >30s)// **here**
> >
> >...
> >
> > [ TIME ] Timed out waiting for device dev-ttyPS0.device. // **here**
> >
> >...
> >
> >
> >
> >Yeah, the baudrate won't change anything if syst

Re: [systemd-devel] Bugfix release(s)

[Michael Olbrich]

On Tue, Jan 15, 2019 at 01:12:13PM +0100, Lennart Poettering wrote:
> On Di, 15.01.19 09:51, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > On Mon, Jan 14, 2019 at 04:36:49PM +0100, Lennart Poettering wrote:
> > > I'd love to see some more CI hookup with Arch and Debian for example
> > > (right now there is zero) or even just a git preview package set or so
> > > that interested people can test. Without either it's very likely that
> > > things break on those distros, because there's no way we'll catch
> > > things beforehand.
> >
> > I'm not Arch or Debian and I don't have the resources to set up a CI but
> > I'd like to do more testing before the release. The problem is, that I
> > don't have the time to test master all the time, so a heads up would be
> > nice before the release. Just a short mail to the list with the planed
> > release date a few weeks before the release date.
> > I think something like this happened in the past, but I didn't see anything
> > for v240 and v241.
> 
> We use the milestones to track progress. i.e. currently:
> 
> https://github.com/systemd/systemd/milestone/18
> 
> As the list gets shorter the closer we get to a release.
> 
> There's also a telegram channel some of us hang out where talk about
> this, if you want I can add you there.

Well, I don't have telegram, so that's currently not possible. But I think
the milestones and especially the rc tags are good enough for me.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Bugfix release(s)

[Michael Olbrich]

On Mon, Jan 14, 2019 at 04:36:49PM +0100, Lennart Poettering wrote:
> I'd love to see some more CI hookup with Arch and Debian for example
> (right now there is zero) or even just a git preview package set or so
> that interested people can test. Without either it's very likely that
> things break on those distros, because there's no way we'll catch
> things beforehand.

I'm not Arch or Debian and I don't have the resources to set up a CI but
I'd like to do more testing before the release. The problem is, that I
don't have the time to test master all the time, so a heads up would be
nice before the release. Just a short mail to the list with the planed
release date a few weeks before the release date.
I think something like this happened in the past, but I didn't see anything
for v240 and v241.

> (That said v241 is going to be a bugfix release mostly, and is
> currently being prepared.)

So, any timeline for this?

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] ShutdownWatchdogSec does not work as described

[Michael Olbrich]

On Mon, Mar 19, 2018 at 11:41:33AM +0530, prashantkumar dhotre wrote:
> I am observing that  ShutdownWatchdogSec   setting in  system.conf
> 
> In man page, for ShutdownWatchdogSec, I see :
> " It works as a safety net to ensure that the reboot takes place even if a
> clean reboot attempt times out. "
> I am not clear what is meant by 'clean reboot' and 'times out ' here
> 
> For testing, I have set it to 5 sec
> 
> ++
> root@re1:~# grep ShutdownWatchdogSec /etc/systemd/system.conf
> ShutdownWatchdogSec=5sec
> root@re1:~#
> ++
> 
> 
> and I have my.service , which on stop will sleep for few minutes
> 
> ++
> # cat /etc/systemd/system/my.service
> [Unit]
> Description="my"
> 
> [Service]
> Type=oneshot
> RemainAfterExit=yes
> ExecStart=/bin/true
> ExecStopPost=/bin/sleep 3000
> Restart=no
> 
> [Install]
> WantedBy=multi-user.target
> ++
> 
> But I dont see hardware watchdog being kicked in.
> Is this a bug in ShutdownWatchdogSec ? if not am I missing any config ?
> 
> I did some research and found below links which talk about effect of
> ShutdownWatchdogSec .
> 
> https://utcc.utoronto.ca/~cks/space/blog/linux/SystemdNFSMountShutdown
> https://utcc.utoronto.ca/~cks/space/blog/linux/SystemdShutdownWatchdog
> 
> as described in above pages, one option seem to be to lower value of
> JobTimeoutSec
>  in
> reboot.target but that solution is still about software watchdog and not
> about hardware watchdog.
> 
> 
> My requirement is to put upper bound time limit of reboot, and once upper
> limit is reached, i want to kick in hardware watchdog
> 
> Could you please let me know how do I achieve this ?

Rebooting with systemd can be split into 3 stages:
1. Activate reboot.target
   That's just a normal transition in systemd. The mainloop is still
   running and RuntimeWatchdogSec= is used and the watchdog is pinged as
   needed.
2. Kill all remaining processes and unmount all filesystem. The mainloop is
   no longer running at this point. This is covered by ShutdownWatchdogSec=
   in case any of these operations does not finish.
3. Execute the reboot() system call.

I'm not sure if you can limit the duration of the first step reliably. You
can skip it with 'systemctl reboot -f'.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] stacked automounts

[Michael Olbrich]

On Fri, Nov 24, 2017 at 09:03:23PM +0100, Olaf Hering wrote:
> Is there a way to have stacked automounts?

Not really.

> In this example only /d is mounted when /d/l/1/ is accessed:
> 
> LABEL=d/d xfs 
> noatime,x-systemd.automount,x-systemd.idle-timeout=22 1 2
> /d/i/1.iso /d/l/1 iso9660 
> ro,loop,x-systemd.automount,x-systemd.idle-timeout=11 0 0
> 
> In the logs I see:
> Set up automount d-l-1.automount.
> Unset automount d-l-1.automount.

Systemd mounts a autofs for each automount
filesystem. In this case this means autofs is mounted to /d/l/1 so the real
filesystem for /d is mounted at boot-time. So using x-systemd.automount for
/d is pointless. It's mounted immediately and cannot expire.

Which version are you using? I tested this with some tmpfs and it works as
expected with systemd v235.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] automount expiry breaks further automounts (was Re: why does bootctl default to /boot and not to /boot/efi?)

[Michael Olbrich]

On Thu, Jun 02, 2016 at 08:03:31AM +0300, Mantas Mikulėnas wrote:
> On Wed, Jun 1, 2016 at 4:51 PM, Lennart Poettering 
> wrote:
> 
> > On Wed, 01.06.16 09:15, Mantas Mikulėnas (graw...@gmail.com) wrote:
> >
> > > I'd buy into it if vfat weren't so brittle – several times I had to use
> > > syslinux in /boot because the ESP lost *both* kernels I had in it...
> > "sync;
> > > sync; unmount; mount; check" was part of my kernel update ritual for a
> > > while. Maybe it's the Linux driver, maybe it's my UEFI that's bad at
> > FAT, I
> > > dunno.
> >
> > Note that in today's systemd the ESP is automounted on request, and
> > unmounting when idle. This means that the FAT partition is generally
> > unmounted, except when you actually invoke bootctl. This should make
> > access pretty safe. (of course, this only works if the efi mount
> > generator actually does its job, it's not in effect if you explicit
> > mount something else into /boot).
> 
> 
> Testing this. Looks like once systemd has idle-unmounted something, it will
> actually completely ignore further automount requests (they don't even show
> up in the debug log) and I have to run "systemctl daemon-reexec" to unwedge
> it... I suppose that's a bug in event handling somewhere.
> 
> Post-reexec, it logs this:
> 
> Jun 02 08:01:22 radius systemd[1]: Reexecuting.
> Jun 02 08:01:22 radius systemd[1]: Failed to expire automount, ignoring:
> Host is down
> Jun 02 08:01:22 radius systemd[1]: tmp-autotest.automount: Got automount
> request for /tmp/autotest, triggered by 24024 (touch)

Should be fixed with https://github.com/systemd/systemd/pull/3434

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [HEADSUP] systemd-222 around the corner

[Michael Olbrich]

On Tue, Jul 07, 2015 at 10:57:32AM +0200, Kay Sievers wrote:
> On Tue, Jul 7, 2015 at 10:10 AM, Michael Olbrich
>  wrote:
> > On Mon, Jul 06, 2015 at 07:54:41PM +0200, David Herrmann wrote:
> >> We intend to release v222 tomorrow. If anyone has open issues that
> >> need to be in that release, please speak up. Right now, the release
> >> consists almost exclusively of bug-fixes, and we want to get those
> >> into distributions.
[...]
> > If that's the
> > case, then please merge https://github.com/systemd/systemd/pull/505 or make
> > libgcrypt optional in some other way.
> 
> Hmm, this seems a bit out-of-scope for the systemd build system.

I guess I'll add this to my growing list of not-for-upstream patches... :-/

> Why
> don't you just add the usually used stub lines for things you want to
> optimize out:
>   echo 'AC_DEFUN([AM_PATH_LIBGCRYPT],[:])' > m4/gcrypt.m4

I think a patch, even if I have to keep it for a long time, is cleaner than
that. And I prefer a solution that keeps the configure script consistent.

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [HEADSUP] systemd-222 around the corner

[Michael Olbrich]

Hi,

On Mon, Jul 06, 2015 at 07:54:41PM +0200, David Herrmann wrote:
> We intend to release v222 tomorrow. If anyone has open issues that
> need to be in that release, please speak up. Right now, the release
> consists almost exclusively of bug-fixes, and we want to get those
> into distributions.

This will be a release without 'distcheck' tarballs, right? If that's the
case, then please merge https://github.com/systemd/systemd/pull/505 or make
libgcrypt optional in some other way.

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] no tar balls at release time

[Michael Olbrich]

On Mon, Jun 22, 2015 at 10:21:18PM -0400, Mike Gilbert wrote:
> On Mon, Jun 22, 2015 at 7:30 PM, Lennart Poettering
>  wrote:
> > On Tue, 23.06.15 01:21, Kay Sievers (k...@vrfy.org) wrote:
> >
> >> We currently considering to stop creating release tar balls.
> >>
> >> For build systems which still require them, they can be created
> >> locally from the upstream git repository with:
> >>   git archive --format=tar --prefix=systemd-$(VERSION)/ $(VERSION) | \
> >> xz > systemd-$(VERSION).tar.xz
> >>
> >> These tar balls will not include the "500k of shell scripts" added by
> >> autotools. These files need to be added to the extracted tarball by
> >> running ./autogen.sh.
> >>
> >> These tar balls will also not include any generated content like
> >> fonts, man, html pages. This is intentional.
> >
> > Which of course means the build tools for all of these need to be
> > around on the build machines, as *everything* will be rebuilt from
> > scratch now. Specifically you need
> > autoconf/automake/python/perl/m4/xsltproc/... on every build machine.
> >
> > Hence the question to ask is: is anyone downstream relying on the
> > pre-built stuff, and has a very good reason why he couldn't just
> > install the build tools on his build machine and build things with
> > that?
> 
> On Gentoo, most users build from source, so this means additional
> dependencies on most users' systems. We would appreciate having the
> autotools-generated tarballs, but we can certainly live without them.
> 
> FYI, having to run autoreconf introduces the following dependencies for us:
> 
> app-text/docbook-xml-dtd:4.2
> app-text/docbook-xml-dtd:4.5
> app-text/docbook-xsl-stylesheets
> dev-libs/libxslt:0
> >=dev-libs/libgcrypt-1.4.5:0"

I don't mind the autotools requirements. But building libgcrypt just for
autogen is really annoying when cross-compiling for systems where libgcrypt
isn't installed at all.

So, if you do this, please add all necessary m4 files to the tarballs or
make it optional, the way gtk-doc was handled.

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Minimum required gcc version?

[Michael Olbrich]

Hi,

On Thu, Jun 18, 2015 at 03:20:04PM +0200, Lennart Poettering wrote:
> On Thu, 18.06.15 14:29, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > Do we have a minimum required gcc version? The README just lists gcc
> > without any version. However the current git fails to build with gcc-4.7:
> > In this version, gcc produces -Wshadow warnings for variables with the same
> > name as a defined function (e.g. 'now' in several functions in
> > src/core/device.c). This results in build errors because we compile with
> > -Werror=shadow now.
> 
> Hm, yuck. I figure we could add that only for gcc versions newer than
> that, with a configure check...
> 
> or can't you add a -Wno-shadow to CFLAGS on the configure cmdline,
> overriding what we set there?

I can easily work around this issue with cc_cv_CFLAGS__Werror_shadow=no in
the environment for configure. I just wanted to report this in case you
care about gcc-4.7.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Minimum required gcc version?

[Michael Olbrich]

Hi,

Do we have a minimum required gcc version? The README just lists gcc
without any version. However the current git fails to build with gcc-4.7:
In this version, gcc produces -Wshadow warnings for variables with the same
name as a defined function (e.g. 'now' in several functions in
src/core/device.c). This results in build errors because we compile with
-Werror=shadow now.

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [HEADSUP] Intend to release 221 by the end of the week

[Michael Olbrich]

On Mon, Jun 15, 2015 at 05:07:05PM +0200, Lennart Poettering wrote:
> People asked for a heads-up on this: I intend to prepare v221 by the
> end of this week. 

Thanks!

> It's a good time to start testing what's currently in git!
> 
> If you take this as hint to start your auto-builder however, then
> that's wrong: you should run your auto-builder CI-style all the time
> anyway, not just now shortly before the release!

Well, my more obscure tests need some manual interaction, so the headsup is
appreciated.

> We are now labelling release critical issues in github with the
> "release-critical" label. To help us with the release patches for
> those issues would be particularly well received!
> 
> https://github.com/systemd/systemd/labels/release-critical

I have a pull request[1] that I think should be release-critical, but it
seems I can't add labels to it. Either I'm doing something wrong, or I
don't have the necessary permissions to add labels.

Michael

[1] https://github.com/systemd/systemd/pull/281

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] random-util: guard including sys/auxv.h with the corresponding ifdef check

[Michael Olbrich]

On Tue, Jun 02, 2015 at 02:09:36PM +0200, Umut Tezduyar Lindskog wrote:
> http://lists.freedesktop.org/archives/systemd-devel/2015-May/032473.html

Oh, I missed that patch. I need to read the mails more carefully :-)

Michael

> On Tue, Jun 2, 2015 at 11:07 AM, Michael Olbrich
>  wrote:
> > ---
> >  src/shared/random-util.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/src/shared/random-util.c b/src/shared/random-util.c
> > index 88f5182508e7..b230044f5099 100644
> > --- a/src/shared/random-util.c
> > +++ b/src/shared/random-util.c
> > @@ -23,7 +23,9 @@
> >  #include 
> >  #include 
> >  #include 
> > +#ifdef HAVE_SYS_AUXV_H
> >  #include 
> > +#endif
> >  #include 
> >
> >  #include "random-util.h"
> > --
> > 2.1.4
> >
> > ___
> > systemd-devel mailing list
> > systemd-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] missing: add more btrfs defines

[Michael Olbrich]

---
 src/shared/missing.h | 28 
 1 file changed, 28 insertions(+)

diff --git a/src/shared/missing.h b/src/shared/missing.h
index 919400949138..be7f6186fcfb 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -269,6 +269,11 @@ struct btrfs_qgroup_inherit {
 __u64 qgroups[0];
 };
 
+struct btrfs_ioctl_qgroup_limit_args {
+__u64 qgroupid;
+struct btrfs_qgroup_limit lim;
+};
+
 struct btrfs_ioctl_vol_args_v2 {
 __s64 fd;
 __u64 transid;
@@ -360,6 +365,14 @@ struct btrfs_ioctl_clone_range_args {
 __u64 src_offset, src_length;
 __u64 dest_offset;
 };
+
+#define BTRFS_QUOTA_CTL_ENABLE  1
+#define BTRFS_QUOTA_CTL_DISABLE 2
+#define BTRFS_QUOTA_CTL_RESCAN__NOTUSED 3
+struct btrfs_ioctl_quota_ctl_args {
+__u64 cmd;
+__u64 status;
+};
 #endif
 
 #ifndef BTRFS_IOC_DEFRAG
@@ -367,6 +380,11 @@ struct btrfs_ioctl_clone_range_args {
  struct btrfs_ioctl_vol_args)
 #endif
 
+#ifndef BTRFS_IOC_RESIZE
+#define BTRFS_IOC_RESIZE _IOW(BTRFS_IOCTL_MAGIC, 3, \
+ struct btrfs_ioctl_vol_args)
+#endif
+
 #ifndef BTRFS_IOC_CLONE
 #define BTRFS_IOC_CLONE _IOW(BTRFS_IOCTL_MAGIC, 9, int)
 #endif
@@ -424,6 +442,16 @@ struct btrfs_ioctl_clone_range_args {
  struct btrfs_ioctl_vol_args)
 #endif
 
+#ifndef BTRFS_IOC_QUOTA_CTL
+#define BTRFS_IOC_QUOTA_CTL _IOWR(BTRFS_IOCTL_MAGIC, 40, \
+   struct btrfs_ioctl_quota_ctl_args)
+#endif
+
+#ifndef BTRFS_IOC_QGROUP_LIMIT
+#define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \
+   struct btrfs_ioctl_qgroup_limit_args)
+#endif
+
 #ifndef BTRFS_FIRST_FREE_OBJECTID
 #define BTRFS_FIRST_FREE_OBJECTID 256
 #endif
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] random-util: guard including sys/auxv.h with the corresponding ifdef check

[Michael Olbrich]

---
 src/shared/random-util.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/shared/random-util.c b/src/shared/random-util.c
index 88f5182508e7..b230044f5099 100644
--- a/src/shared/random-util.c
+++ b/src/shared/random-util.c
@@ -23,7 +23,9 @@
 #include 
 #include 
 #include 
+#ifdef HAVE_SYS_AUXV_H
 #include 
+#endif
 #include 
 
 #include "random-util.h"
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] missing: add more IFLA_VXLAN_* defines

[Michael Olbrich]

Otherwise building faild with kernel headers < v3.16
---
 configure.ac |  2 +-
 src/shared/missing.h | 11 +--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 48cedb5ab61a..0818dd80cf0c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -334,7 +334,7 @@ AC_CHECK_DECLS([IFLA_INET6_ADDR_GEN_MODE,
 IFLA_PHYS_PORT_ID,
 IFLA_BOND_AD_INFO,
 IFLA_VLAN_PROTOCOL,
-IFLA_VXLAN_LOCAL6,
+IFLA_VXLAN_REMCSUM_NOPARTIAL,
 IFLA_IPTUN_6RD_RELAY_PREFIXLEN,
 IFLA_BRIDGE_VLAN_INFO,
 IFLA_BRPORT_UNICAST_FLOOD,
diff --git a/src/shared/missing.h b/src/shared/missing.h
index 8ca6f8edb62c..919400949138 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -713,7 +713,7 @@ static inline int setns(int fd, int nstype) {
 #define IFLA_VLAN_MAX   (__IFLA_VLAN_MAX - 1)
 #endif
 
-#if !HAVE_DECL_IFLA_VXLAN_LOCAL6
+#if !HAVE_DECL_IFLA_VXLAN_REMCSUM_NOPARTIAL
 #define IFLA_VXLAN_UNSPEC 0
 #define IFLA_VXLAN_ID 1
 #define IFLA_VXLAN_GROUP 2
@@ -732,7 +732,14 @@ static inline int setns(int fd, int nstype) {
 #define IFLA_VXLAN_PORT 15
 #define IFLA_VXLAN_GROUP6 16
 #define IFLA_VXLAN_LOCAL6 17
-#define __IFLA_VXLAN_MAX 18
+#define IFLA_VXLAN_UDP_CSUM 18
+#define IFLA_VXLAN_UDP_ZERO_CSUM6_TX 19
+#define IFLA_VXLAN_UDP_ZERO_CSUM6_RX 20
+#define IFLA_VXLAN_REMCSUM_TX 21
+#define IFLA_VXLAN_REMCSUM_RX 22
+#define IFLA_VXLAN_GBP 23
+#define IFLA_VXLAN_REMCSUM_NOPARTIAL 24
+#define __IFLA_VXLAN_MAX 25
 
 #define IFLA_VXLAN_MAX  (__IFLA_VXLAN_MAX - 1)
 #endif
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] tmpfiles: try to handle read-only file systems gracefully

[Michael Olbrich]

On Wed, May 06, 2015 at 04:11:57AM +, Zbigniew Jędrzejewski-Szmek wrote:
> On Thu, Apr 30, 2015 at 08:50:38PM +0200, Michael Olbrich wrote:
> > On read-only filesystems trying to create the target will not fail with
> > EEXIST but with EROFS. Handle EROFS by checking if the target already
> > exists, and if empty when truncating.
> > This avoids reporting errors if tmpfiles doesn't actually needs to do
> > anything.
> > ---
> > 
> > This is a rework of a patch I wrote some time ago[1]. This time reacting to
> > EROFS instead of preempting it.
> Applied, with some small changes, see below.
> 
> > 
> > Michael
> > 
> > [1] 
> > http://lists.freedesktop.org/archives/systemd-devel/2014-August/022158.html
> > 
> >  src/tmpfiles/tmpfiles.c | 12 +---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
> > index 218d55051410..4473bf019911 100644
> > --- a/src/tmpfiles/tmpfiles.c
> > +++ b/src/tmpfiles/tmpfiles.c
> > @@ -983,9 +983,11 @@ static int write_one_file(Item *i, const char *path) {
> >  log_debug_errno(errno, "Not writing \"%s\": %m", 
> > path);
> >  return 0;
> >  }
> > -
> > -log_error_errno(errno, "Failed to create file %s: %m", 
> > path);
> > -return -errno;
> > +r = -errno;
> > +if (i->argument || r != -EROFS || stat(path, &st) < 0 || 
> > (i->type == TRUNCATE_FILE && st.st_size > 0)) {
> > +log_error_errno(r, "Failed to create file %s: %m", 
> > path);
> > +return r;
> > +}
> I reverted the condition, because it seems safer to whitelist instead of
> blacklisting. Also, if the condition didn't succeed, we'd do stat twice.
> Please check that it still works as intended.

Looks good here.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] tmpfiles: try to handle read-only file systems gracefully

[Michael Olbrich]

On read-only filesystems trying to create the target will not fail with
EEXIST but with EROFS. Handle EROFS by checking if the target already
exists, and if empty when truncating.
This avoids reporting errors if tmpfiles doesn't actually needs to do
anything.
---

This is a rework of a patch I wrote some time ago[1]. This time reacting to
EROFS instead of preempting it.

Michael

[1] http://lists.freedesktop.org/archives/systemd-devel/2014-August/022158.html

 src/tmpfiles/tmpfiles.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 218d55051410..4473bf019911 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -983,9 +983,11 @@ static int write_one_file(Item *i, const char *path) {
 log_debug_errno(errno, "Not writing \"%s\": %m", path);
 return 0;
 }
-
-log_error_errno(errno, "Failed to create file %s: %m", path);
-return -errno;
+r = -errno;
+if (i->argument || r != -EROFS || stat(path, &st) < 0 || 
(i->type == TRUNCATE_FILE && st.st_size > 0)) {
+log_error_errno(r, "Failed to create file %s: %m", 
path);
+return r;
+}
 }
 
 if (i->argument) {
@@ -1154,6 +1156,10 @@ static int create_item(Item *i) {
 
 log_debug("Copying tree \"%s\" to \"%s\".", resolved, i->path);
 r = copy_tree(resolved, i->path, false);
+
+if (r == -EROFS && stat(i->path, &st) == 0)
+r = -EEXIST;
+
 if (r < 0) {
 struct stat a, b;
 
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] tmpfiles: remember errno before it might be overwritten

[Michael Olbrich]

---

I'm not sure if this is really necessary right now, but that might change
in the future. Saving errno before calling another function is always a
good idea.

Michael

 src/tmpfiles/tmpfiles.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index d574254e0fb8..218d55051410 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1279,13 +1279,15 @@ static int create_item(Item *i) {
 
 mac_selinux_create_file_prepare(i->path, S_IFLNK);
 r = symlink(resolved, i->path);
+if (r < 0)
+r = -errno;
 mac_selinux_create_file_clear();
 
 if (r < 0) {
 _cleanup_free_ char *x = NULL;
 
-if (errno != EEXIST)
-return log_error_errno(errno, "symlink(%s, %s) 
failed: %m", resolved, i->path);
+if (r != -EEXIST)
+return log_error_errno(r, "symlink(%s, %s) 
failed: %m", resolved, i->path);
 
 r = readlink_malloc(i->path, &x);
 if (r < 0 || !streq(resolved, x)) {
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] shared/utmp-wtmp: fix copy/paste error

[Michael Olbrich]

---
 src/shared/utmp-wtmp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/utmp-wtmp.h b/src/shared/utmp-wtmp.h
index 6ac2c7b1c768..5d26ba6fb1d0 100644
--- a/src/shared/utmp-wtmp.h
+++ b/src/shared/utmp-wtmp.h
@@ -65,7 +65,7 @@ static inline int utmp_wall(
 const char *username,
 const char *origin_tty,
 bool (*match_tty)(const char *tty, void *userdata),
-void *userdata);
+void *userdata) {
 return 0;
 }
 
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v2] automount: add expire support

[Michael Olbrich]

On Tue, Apr 21, 2015 at 08:45:30PM +0200, Lennart Poettering wrote:
> On Tue, 14.04.15 22:01, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> 
> I added some more logging (so that automount_dispatch_expire() can
> never fail without this being logged) and applied it!
> 
> I only gave this superficial testing though, please check if this all
> still works fine for you now!

Looks good. My test-cases work just fine.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2] automount: add expire support

[Michael Olbrich]

---
Changes in v2:
 - addressed comments
 - check every MAX(a->timeout_idle_usec/10, USEC_PER_SEC) instead of every
   5 seconds

 man/systemd.automount.xml |   8 ++
 man/systemd.mount.xml |   9 ++
 src/core/automount.c  | 221 --
 src/core/automount.h  |   6 +-
 src/core/dbus-automount.c |   1 +
 src/core/load-fragment-gperf.gperf.m4 |   1 +
 src/core/mount.c  |  20 +--
 src/fstab-generator/fstab-generator.c |  24 
 8 files changed, 262 insertions(+), 28 deletions(-)

diff --git a/man/systemd.automount.xml b/man/systemd.automount.xml
index b5b5885cdff2..9561590c5c89 100644
--- a/man/systemd.automount.xml
+++ b/man/systemd.automount.xml
@@ -135,6 +135,14 @@
 creating these directories. Takes an access mode in octal
 notation. Defaults to 0755.
   
+  
+TimeoutIdleSec=
+Configures an idleness timeout. Once the mount has been
+idle for the specified time, systemd will attempt to unmount. Takes a
+unit-less value in seconds, or a time span value such as "5min 20s".
+Pass 0 to disable the timeout logic. The timeout is disabled by
+default.
+  
 
   
 
diff --git a/man/systemd.mount.xml b/man/systemd.mount.xml
index fcb9a4416104..e102d27ab762 100644
--- a/man/systemd.mount.xml
+++ b/man/systemd.mount.xml
@@ -148,6 +148,15 @@
   
 
   
+x-systemd.idle-timeout=
+
+Configures the idleness timeout of the
+automount unit. See TimeoutIdleSec= in
+
systemd.automount5
+for details.
+  
+
+  
 x-systemd.device-timeout=
 
 Configure how long systemd should wait for a
diff --git a/src/core/automount.c b/src/core/automount.c
index ce484ff1cd16..1cf108524a80 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -42,6 +42,7 @@
 #include "bus-error.h"
 #include "formats-util.h"
 #include "process-util.h"
+#include "async.h"
 
 static const UnitActiveState state_translation_table[_AUTOMOUNT_STATE_MAX] = {
 [AUTOMOUNT_DEAD] = UNIT_INACTIVE,
@@ -50,6 +51,22 @@ static const UnitActiveState 
state_translation_table[_AUTOMOUNT_STATE_MAX] = {
 [AUTOMOUNT_FAILED] = UNIT_FAILED
 };
 
+struct expire_data {
+int dev_autofs_fd;
+int ioctl_fd;
+};
+
+static inline void expire_data_free(struct expire_data *data) {
+if (!data)
+return;
+
+safe_close(data->dev_autofs_fd);
+safe_close(data->ioctl_fd);
+free(data);
+}
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(struct expire_data*, expire_data_free);
+
 static int open_dev_autofs(Manager *m);
 static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, 
void *userdata);
 
@@ -81,13 +98,16 @@ static void repeat_unmount(const char *path) {
 }
 }
 
+static int automount_send_ready(Automount *a, Set *tokens, int status);
+
 static void unmount_autofs(Automount *a) {
 assert(a);
 
 if (a->pipe_fd < 0)
 return;
 
-automount_send_ready(a, -EHOSTDOWN);
+automount_send_ready(a, a->tokens, -EHOSTDOWN);
+automount_send_ready(a, a->expire_tokens, -EHOSTDOWN);
 
 a->pipe_event_source = sd_event_source_unref(a->pipe_event_source);
 a->pipe_fd = safe_close(a->pipe_fd);
@@ -112,6 +132,10 @@ static void automount_done(Unit *u) {
 
 set_free(a->tokens);
 a->tokens = NULL;
+set_free(a->expire_tokens);
+a->expire_tokens = NULL;
+
+a->expire_event_source = sd_event_source_unref(a->expire_event_source);
 }
 
 static int automount_add_mount_links(Automount *a) {
@@ -265,6 +289,7 @@ static int automount_coldplug(Unit *u, Hashmap 
*deferred_work) {
 }
 
 static void automount_dump(Unit *u, FILE *f, const char *prefix) {
+char time_string[FORMAT_TIMESPAN_MAX];
 Automount *a = AUTOMOUNT(u);
 
 assert(a);
@@ -273,11 +298,13 @@ static void automount_dump(Unit *u, FILE *f, const char 
*prefix) {
 "%sAutomount State: %s\n"
 "%sResult: %s\n"
 "%sWhere: %s\n"
-"%sDirectoryMode: %04o\n",
+"%sDirectoryMode: %04o\n"
+"%sTimeoutIdleUSec: %s\n",
 prefix, automount_state_to_string(a->state),
 prefix, automount_result_to_string(a->result),
 prefix, a->where,
-prefix, a->directory_mode);
+prefix, a->directory_mode,
+prefix, format_timespan(time_string, FORMAT_TIMESPAN_MAX, 
a->timeout_idle_usec, USEC_PER_SEC));
 }
 
 static void automount_enter_dead(Automount *a, AutomountResult f) {
@@ -367,7 +394,7 @@ static int autofs_protocol(int dev_autofs_fd, int ioctl_fd) 
{
 return 0;
 }
 
-static int autofs_set_timeout(int dev_autofs_fd, int ioctl_fd, time_t sec) {
+static int autofs_set_timeout(int dev_autofs_fd, int ioctl_fd, usec_t us

Re: [systemd-devel] [PATCH] automount: add expire support

[Michael Olbrich]

On Thu, Apr 02, 2015 at 12:54:00PM +0200, Lennart Poettering wrote:
> On Sun, 22.03.15 13:36, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> 
> Love this work!

Thanks.

[...]
> > +
> > +if (a->expire_event_source) {
> > +r = sd_event_source_set_time(a->expire_event_source, 
> > now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC);
> > +if (r < 0)
> > +return r;
> > +
> > +return sd_event_source_set_enabled(a->expire_event_source, 
> > SD_EVENT_ONESHOT);
> > +}
> > +
> > +return sd_event_add_time(
> > +UNIT(a)->manager->event,
> > +&a->expire_event_source,
> > +CLOCK_MONOTONIC,
> > +now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC, 0,
> > +automount_dispatch_expire, a);
> > +}
> 
> Maybe the 5 * USEC_PER_SEC should become a #define somewhere at the
> top of the file? Given that this is used more than once it might be
> advisable to use the same definition.

I've been thinking about this again and I changed this. Checking every 5
Seconds is rather often for longer timeouts. I've changed it to
MAX(a->timeout_idle_usec/10, USEC_PER_SEC). I think that's a good
compromise.

> > +
> > +r = set_ensure_allocated(&a->expire_tokens, NULL);
> > +if (r < 0) {
> > +log_unit_error(UNIT(a)->id, "Failed to
> > allocate token set.");
> 
> Sounds like a job for log_oom().

It's basically a copy of the code right above it, so it should match. I
think that should be a separate patch.

> > +goto fail;
> > +}
> > +
> > +r = set_put(a->expire_tokens, 
> > UINT_TO_PTR(packet.v5_packet.wait_queue_token));
> > +if (r < 0) {
> > +log_unit_error_errno(UNIT(a)->id, r, "Failed to 
> > remember token: %m");
> > +goto fail;
> > +}
> > +r = manager_add_job(UNIT(a)->manager, JOB_STOP, 
> > UNIT_TRIGGER(UNIT(a)),
> > +JOB_REPLACE, true, &error,
> > NULL);
> 
> We normally don't like break this eagerly.

Actually we do. In this file at least. The manager_add_job for JOB_START
has the same line break :-). But I don't care either way, so I changed it.

> > +return 0;
> > +}
> > +
> > +fprintf(f,
> > +"TimeoutIdleSec=" USEC_FMT "\n",
> > +u / USEC_PER_SEC);
> > +
> > +return 0;
> 
> Why not format that using format_timespan() here, so that the accuracy
> is not lost? (We'll lose it when passing it to the kernel ultimately,
> but we shouldn't lose it any earlier. In particular since you round up
> when passing it to the kernel, but are rounding down here... Hence,
> let's keep this simple, and always pass our native accuracy along
> until the last point where we really have to convert it.

Sounds reasonable, so I changed it. I basically copied this from
the x-systemd.device-timeout implementation, so you might want to change
that as well.

I've changed everything else as requested. New patch follows.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] automount: add expire support

[Michael Olbrich]

---
 man/systemd.automount.xml |   8 ++
 man/systemd.mount.xml |   9 ++
 src/core/automount.c  | 209 --
 src/core/automount.h  |   6 +-
 src/core/dbus-automount.c |   1 +
 src/core/load-fragment-gperf.gperf.m4 |   1 +
 src/core/mount.c  |  20 +---
 src/fstab-generator/fstab-generator.c |  27 +
 8 files changed, 253 insertions(+), 28 deletions(-)

diff --git a/man/systemd.automount.xml b/man/systemd.automount.xml
index b5b5885cdff2..9561590c5c89 100644
--- a/man/systemd.automount.xml
+++ b/man/systemd.automount.xml
@@ -135,6 +135,14 @@
 creating these directories. Takes an access mode in octal
 notation. Defaults to 0755.
   
+  
+TimeoutIdleSec=
+Configures an idleness timeout. Once the mount has been
+idle for the specified time, systemd will attempt to unmount. Takes a
+unit-less value in seconds, or a time span value such as "5min 20s".
+Pass 0 to disable the timeout logic. The timeout is disabled by
+default.
+  
 
   
 
diff --git a/man/systemd.mount.xml b/man/systemd.mount.xml
index fcb9a4416104..e102d27ab762 100644
--- a/man/systemd.mount.xml
+++ b/man/systemd.mount.xml
@@ -148,6 +148,15 @@
   
 
   
+x-systemd.idle-timeout=
+
+Configures the idleness timeout of the
+automount unit. See TimeoutIdleSec= in
+
systemd.automount5
+for details.
+  
+
+  
 x-systemd.device-timeout=
 
 Configure how long systemd should wait for a
diff --git a/src/core/automount.c b/src/core/automount.c
index cec90cbb319d..d2c148943e0f 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -40,6 +40,7 @@
 #include "dbus-automount.h"
 #include "bus-util.h"
 #include "bus-error.h"
+#include "async.h"
 
 static const UnitActiveState state_translation_table[_AUTOMOUNT_STATE_MAX] = {
 [AUTOMOUNT_DEAD] = UNIT_INACTIVE,
@@ -79,13 +80,16 @@ static void repeat_unmount(const char *path) {
 }
 }
 
+static int automount_send_ready(Automount *a, Set *tokens, int status);
+
 static void unmount_autofs(Automount *a) {
 assert(a);
 
 if (a->pipe_fd < 0)
 return;
 
-automount_send_ready(a, -EHOSTDOWN);
+automount_send_ready(a, a->tokens, -EHOSTDOWN);
+automount_send_ready(a, a->expire_tokens, -EHOSTDOWN);
 
 a->pipe_event_source = sd_event_source_unref(a->pipe_event_source);
 a->pipe_fd = safe_close(a->pipe_fd);
@@ -110,6 +114,8 @@ static void automount_done(Unit *u) {
 
 set_free(a->tokens);
 a->tokens = NULL;
+set_free(a->expire_tokens);
+a->expire_tokens = NULL;
 }
 
 static int automount_add_mount_links(Automount *a) {
@@ -263,6 +269,7 @@ static int automount_coldplug(Unit *u, Hashmap 
*deferred_work) {
 }
 
 static void automount_dump(Unit *u, FILE *f, const char *prefix) {
+char time_string[FORMAT_TIMESPAN_MAX];
 Automount *a = AUTOMOUNT(u);
 
 assert(a);
@@ -271,11 +278,13 @@ static void automount_dump(Unit *u, FILE *f, const char 
*prefix) {
 "%sAutomount State: %s\n"
 "%sResult: %s\n"
 "%sWhere: %s\n"
-"%sDirectoryMode: %04o\n",
+"%sDirectoryMode: %04o\n"
+"%sTimeoutIdleUSec: %s\n",
 prefix, automount_state_to_string(a->state),
 prefix, automount_result_to_string(a->result),
 prefix, a->where,
-prefix, a->directory_mode);
+prefix, a->directory_mode,
+prefix, format_timespan(time_string, FORMAT_TIMESPAN_MAX, 
a->timeout_idle_usec, USEC_PER_SEC));
 }
 
 static void automount_enter_dead(Automount *a, AutomountResult f) {
@@ -365,7 +374,7 @@ static int autofs_protocol(int dev_autofs_fd, int ioctl_fd) 
{
 return 0;
 }
 
-static int autofs_set_timeout(int dev_autofs_fd, int ioctl_fd, time_t sec) {
+static int autofs_set_timeout(int dev_autofs_fd, int ioctl_fd, usec_t usec) {
 struct autofs_dev_ioctl param;
 
 assert(dev_autofs_fd >= 0);
@@ -373,7 +382,9 @@ static int autofs_set_timeout(int dev_autofs_fd, int 
ioctl_fd, time_t sec) {
 
 init_autofs_dev_ioctl(¶m);
 param.ioctlfd = ioctl_fd;
-param.timeout.timeout = sec;
+
+/* Convert to seconds, rounding up. */
+param.timeout.timeout = usec / USEC_PER_SEC + (usec % USEC_PER_SEC > 
0);
 
 if (ioctl(dev_autofs_fd, AUTOFS_DEV_IOCTL_TIMEOUT, ¶m) < 0)
 return -errno;
@@ -402,7 +413,7 @@ static int autofs_send_ready(int dev_autofs_fd, int 
ioctl_fd, uint32_t token, in
 return 0;
 }
 
-int automount_send_ready(Automount *a, int status) {
+static int automount_send_ready(Automount *a, Set *tokens, int status) {
 _cleanup_close_ int ioctl_fd = -1;
 unsigned token;
 in

Re: [systemd-devel] experiments with 'minimal build'

[Michael Olbrich]

On Thu, Mar 19, 2015 at 11:37:41PM -0700, Alison Chaiken wrote:
> Jeff Waugh:
> >The last + is +SYSVINIT, but there doesn't appear to be a configure option
> >to disable sysvinit compatibility anymore.
> 
> I was wondering about that myself.

Try --with-sysvinit-path="" --with-sysvrcnd-path="". That should disable it.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] missing.h: add more btrfs types and defines

[Michael Olbrich]

On Thu, Mar 19, 2015 at 02:05:00PM +0100, David Herrmann wrote:
> On Wed, Mar 18, 2015 at 2:04 PM, Michael Olbrich
>  wrote:
> > it seems we're getting a lot of btrfs stuff here. Maybe we should ship a
> > copy of btrfs.h instead?
> 
> All these definitions where introduced with linux-3.8 (well, the
> header was moved to linux/btrfs.h at that time). Our current
> requirement is linux-3.7. I'd just say, lets require linux-headers-3.8
> at build-time?

Actually, that would be 3.9 not 3.8. And I'd really like to avoid such a
dependency. When cross-compiling for embedded systems, a host
'systemd-hwdb' is needed to generate the hwdb. And build servers often run
older Distributions (e.g. Debian stable).

Well, what I really want, is just to build systemd-hwdb and just ignore the
whole btrfs and network stuff, that would require newer kernel headers.
However, the way the systemd build-system is set up, that's currently not
possible.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] missing.h: add more btrfs types and defines

[Michael Olbrich]

---

Hi,

it seems we're getting a lot of btrfs stuff here. Maybe we should ship a
copy of btrfs.h instead?

Michael

 src/shared/missing.h | 151 +++
 1 file changed, 151 insertions(+)

diff --git a/src/shared/missing.h b/src/shared/missing.h
index 802b4957e0a1..ca670cea43f7 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -230,12 +230,59 @@ static inline int getrandom(void *buffer, size_t count, 
unsigned flags) {
 #define BTRFS_UUID_SIZE 16
 #endif
 
+#ifndef BTRFS_SUBVOL_RDONLY
+#define BTRFS_SUBVOL_RDONLY (1ULL << 1)
+#endif
+
+#ifndef BTRFS_SUBVOL_NAME_MAX
+#define BTRFS_SUBVOL_NAME_MAX 4039
+#endif
+
+#ifndef BTRFS_INO_LOOKUP_PATH_MAX
+#define BTRFS_INO_LOOKUP_PATH_MAX 4080
+#endif
+
+#ifndef BTRFS_SEARCH_ARGS_BUFSIZE
+#define BTRFS_SEARCH_ARGS_BUFSIZE (4096 - sizeof(struct 
btrfs_ioctl_search_key))
+#endif
+
 #ifndef HAVE_LINUX_BTRFS_H
 struct btrfs_ioctl_vol_args {
 int64_t fd;
 char name[BTRFS_PATH_NAME_MAX + 1];
 };
 
+struct btrfs_qgroup_limit {
+__u64 flags;
+__u64 max_rfer;
+__u64 max_excl;
+__u64 rsv_rfer;
+__u64 rsv_excl;
+};
+
+struct btrfs_qgroup_inherit {
+__u64 flags;
+__u64 num_qgroups;
+__u64 num_ref_copies;
+__u64 num_excl_copies;
+struct btrfs_qgroup_limit lim;
+__u64 qgroups[0];
+};
+
+struct btrfs_ioctl_vol_args_v2 {
+__s64 fd;
+__u64 transid;
+__u64 flags;
+union {
+struct {
+__u64 size;
+struct btrfs_qgroup_inherit *qgroup_inherit;
+};
+__u64 unused[4];
+};
+char name[BTRFS_SUBVOL_NAME_MAX + 1];
+};
+
 struct btrfs_ioctl_dev_info_args {
 uint64_t devid; /* in/out */
 uint8_t uuid[BTRFS_UUID_SIZE];  /* in/out */
@@ -251,6 +298,68 @@ struct btrfs_ioctl_fs_info_args {
 uint8_t fsid[BTRFS_FSID_SIZE];  /* out */
 uint64_t reserved[124]; /* pad to 1k */
 };
+
+struct btrfs_ioctl_ino_lookup_args {
+__u64 treeid;
+__u64 objectid;
+char name[BTRFS_INO_LOOKUP_PATH_MAX];
+};
+
+struct btrfs_ioctl_search_key {
+/* which root are we searching.  0 is the tree of tree roots */
+__u64 tree_id;
+
+/* keys returned will be >= min and <= max */
+__u64 min_objectid;
+__u64 max_objectid;
+
+/* keys returned will be >= min and <= max */
+__u64 min_offset;
+__u64 max_offset;
+
+/* max and min transids to search for */
+__u64 min_transid;
+__u64 max_transid;
+
+/* keys returned will be >= min and <= max */
+__u32 min_type;
+__u32 max_type;
+
+/*
+ * how many items did userland ask for, and how many are we
+ * returning
+ */
+__u32 nr_items;
+
+/* align to 64 bits */
+__u32 unused;
+
+/* some extra for later */
+__u64 unused1;
+__u64 unused2;
+__u64 unused3;
+__u64 unused4;
+};
+
+struct btrfs_ioctl_search_header {
+__u64 transid;
+__u64 objectid;
+__u64 offset;
+__u32 type;
+__u32 len;
+};
+
+
+struct btrfs_ioctl_search_args {
+struct btrfs_ioctl_search_key key;
+char buf[BTRFS_SEARCH_ARGS_BUFSIZE];
+};
+
+struct btrfs_ioctl_clone_range_args {
+__s64 src_fd;
+__u64 src_offset, src_length;
+__u64 dest_offset;
+};
 #endif
 
 #ifndef BTRFS_IOC_DEFRAG
@@ -258,6 +367,48 @@ struct btrfs_ioctl_fs_info_args {
  struct btrfs_ioctl_vol_args)
 #endif
 
+#ifndef BTRFS_IOC_CLONE
+#define BTRFS_IOC_CLONE _IOW(BTRFS_IOCTL_MAGIC, 9, int)
+#endif
+
+#ifndef BTRFS_IOC_CLONE_RANGE
+#define BTRFS_IOC_CLONE_RANGE _IOW(BTRFS_IOCTL_MAGIC, 13, \
+ struct btrfs_ioctl_clone_range_args)
+#endif
+
+#ifndef BTRFS_IOC_SUBVOL_CREATE
+#define BTRFS_IOC_SUBVOL_CREATE _IOW(BTRFS_IOCTL_MAGIC, 14, \
+ struct btrfs_ioctl_vol_args)
+#endif
+
+#ifndef BTRFS_IOC_SNAP_DESTROY
+#define BTRFS_IOC_SNAP_DESTROY _IOW(BTRFS_IOCTL_MAGIC, 15, \
+ struct btrfs_ioctl_vol_args)
+#endif
+
+#ifndef BTRFS_IOC_TREE_SEARCH
+#define BTRFS_IOC_TREE_SEARCH _IOWR(BTRFS_IOCTL_MAGIC, 17, \
+ struct btrfs_ioctl_search_args)
+#endif
+
+#ifndef BTRFS_IOC_INO_LOOKUP
+#define BTRFS_IOC_INO_LOOKUP _IOWR(BTRFS_IOCTL_MAGIC, 18, \
+ struct btrfs_ioctl_ino_lookup_args)
+#endif
+
+#ifndef BTRFS_IOC_SNAP_CREATE_V2
+#define BTRFS_IOC_SNAP_CREATE_V2 _IOW(BTRFS_IOCTL_MAGIC, 23, \
+ struct btrfs_ioctl_vol_args_v2)
+#endif
+
+#ifndef BTRFS_IOC_SUBVOL_GETFLAGS
+#define BTRFS_IOC_SUBVOL_GETFLAGS _IOR(BTRFS_IOCTL_MAGIC, 25, __u64)
+#endif
+
+#ifndef BTRFS_IOC_SUBVOL_S

[systemd-devel] [PATCH] missing.h: add NDA_*

[Michael Olbrich]

This is necessary to build with older kernel headers. NDA_VLAN was
introduced in v3.9 and NDA_PORT, NDA_VNI and NDA_IFINDEX in v3.10
---
 configure.ac |  4 +++-
 src/shared/missing.h | 16 
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 14518bc52b1d..29111f5fee84 100644
--- a/configure.ac
+++ b/configure.ac
@@ -333,7 +333,8 @@ AC_CHECK_DECLS([IFLA_INET6_ADDR_GEN_MODE,
 IFLA_VXLAN_LOCAL6,
 IFLA_IPTUN_6RD_RELAY_PREFIXLEN,
 IFLA_BRIDGE_VLAN_INFO,
-IFLA_BRPORT_UNICAST_FLOOD],
+IFLA_BRPORT_UNICAST_FLOOD,
+NDA_IFINDEX],
 [], [], [[
 #include 
 #include 
@@ -344,6 +345,7 @@ AC_CHECK_DECLS([IFLA_INET6_ADDR_GEN_MODE,
 #include 
 #include 
 #include 
+#include 
 ]])
 
 # This makes sure pkg.m4 is available.
diff --git a/src/shared/missing.h b/src/shared/missing.h
index e72631e130d9..802b4957e0a1 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef HAVE_AUDIT
 #include 
@@ -623,6 +624,21 @@ static inline int setns(int fd, int nstype) {
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
 #endif
 
+#if !HAVE_DECL_NDA_IFINDEX
+#define NDA_UNSPEC 0
+#define NDA_DST 1
+#define NDA_LLADDR 2
+#define NDA_CACHEINFO 3
+#define NDA_PROBES 4
+#define NDA_VLAN 5
+#define NDA_PORT 6
+#define NDA_VNI 7
+#define NDA_IFINDEX 8
+#define __NDA_MAX 9
+
+#define NDA_MAX (__NDA_MAX - 1)
+#endif
+
 #ifndef IPV6_UNICAST_IF
 #define IPV6_UNICAST_IF 76
 #endif
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] unaligned write in dhcp_identifier_set_iaid

[Michael Olbrich]

Hi,

there is an unaligned write in dhcp_identifier_set_iaid() and I'm not quite
sure what the correct fix is:

int dhcp_identifier_set_iaid(int ifindex, uint8_t *mac, size_t mac_len, 
uint32_t *_id) {
[...]
*_id = (id & 0x) ^ (id >> 32);
[...]
}

And this is called with:
r = dhcp_identifier_set_iaid(client->index, client->mac_addr, 
client->mac_addr_len, &client->client_id.ns.iaid);

And iaid is not unaligned because of the packing in struct sd_dhcp_client.
I'm not sure why '_packed_' is used there. It this just to save some space,
or is there a reason for this?

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] systemctl: don't update the reboot parameter if none is given

[Michael Olbrich]

Otherwise systemd-reboot.service will remove the parameter that was set
before. This was broken in commit "b986229efe2cc96157aa14c37bab7843311bbef1
systemctl: bugfix for systemctl reboot command with argument"
---

This is more a hack than anything else, but I'm not sure who to fix this
otherwise.

Michael

 src/systemctl/systemctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 2d70ff1de69e..7b81a9c98592 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -2993,7 +2993,7 @@ static int start_special(sd_bus *bus, char **args) {
 return -EPERM;
 }
 
-if (a == ACTION_REBOOT) {
+if (a == ACTION_REBOOT && args[1]) {
 r = update_reboot_param_file(args[1]);
 if (r < 0)
 return r;
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] config_parse_set_status: put signals in the correct set

[Michael Olbrich]

This was broken when the code was rearranged in "1e2fd62d70ff
core/load-fragment.c: correct argument sign and split up long lines"
---
 src/core/load-fragment.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 516731aba26c..571bb64b7824 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -3150,6 +3150,7 @@ int config_parse_set_status(
 FOREACH_WORD(word, l, rvalue, state) {
 _cleanup_free_ char *temp;
 int val;
+Set **set;
 
 temp = strndup(word, l);
 if (!temp)
@@ -3162,21 +3163,23 @@ int config_parse_set_status(
 if (val <= 0) {
 log_syntax(unit, LOG_ERR, filename, line, -val,
"Failed to parse value, ignoring: 
%s", word);
-return 0;
+continue;
 }
+set = &status_set->signal;
 } else {
 if (val < 0 || val > 255) {
 log_syntax(unit, LOG_ERR, filename, line, 
ERANGE,
"Value %d is outside range 0-255, 
ignoring", val);
 continue;
 }
+set = &status_set->status;
 }
 
-r = set_ensure_allocated(&status_set->status, NULL);
+r = set_ensure_allocated(set, NULL);
 if (r < 0)
 return log_oom();
 
-r = set_put(status_set->status, INT_TO_PTR(val));
+r = set_put(*set, INT_TO_PTR(val));
 if (r < 0) {
 log_syntax(unit, LOG_ERR, filename, line, -r,
"Unable to store: %s", word);
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] missing: define correct syscall numbers for memfd_create() and getrandom() on aarch64

[Michael Olbrich]

---

Hi,

I've tested getrandom(): With 384 the kernel dumps a warning, because the
syscall does not exist. With 278 the syscall tracer tells me that it's
called when I run e.g. journalctl, so that looks good.
I've not tested memfd_create() but it's defined right below getrandom() in
asm-generic/unistd.h, so I think that's correct too.

Regards,
Michael

 src/shared/missing.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/shared/missing.h b/src/shared/missing.h
index 5b95b0006f5e..f8721e6b99ee 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -136,6 +136,8 @@ static inline int pivot_root(const char *new_root, const 
char *put_old) {
 #define __NR_memfd_create 319
 #  elif defined __arm__
 #define __NR_memfd_create 385
+#  elif defined __aarch64__
+#define __NR_memfd_create 279
 #  elif defined _MIPS_SIM
 #if _MIPS_SIM == _MIPS_SIM_ABI32
 #  define __NR_memfd_create 4354
@@ -165,8 +167,10 @@ static inline int memfd_create(const char *name, unsigned 
int flags) {
 #define __NR_getrandom 318
 #  elif defined(__i386__)
 #define __NR_getrandom 355
-#  elif defined(__arm__) || defined(__aarch64__)
+#  elif defined(__arm__)
 #define __NR_getrandom 384
+# elif defined(__aarch64__)
+#define __NR_getrandom 278
 #  elif defined(__ia64__)
 #define __NR_getrandom 1339
 #  elif defined(__m68k__)
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Build errors with lto and compat-libs

[Michael Olbrich]

On Tue, Aug 26, 2014 at 09:42:38PM +0200, Lennart Poettering wrote:
> On Tue, 26.08.14 15:15, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > On Mon, Aug 18, 2014 at 03:48:09PM +0200, Lennart Poettering wrote:
> > > On Sun, 17.08.14 09:54, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > > > With --enable-compat-libs building fails like this:
> > > > 
> > > >   CCLD libsystemd-journal.la
> > > > [...]
> > > > /tmp/ccISOiYU.ltrans1.ltrans.o: In function `sd_journal_process':
> > > > ccISOiYU.ltrans1.o:(.text+0x0): multiple definition of 
> > > > `sd_journal_process'
> > > > libsystemd_journal_internal_la-sd-journal.o (symbol from 
> > > > plugin):(.text+0x0): first defined here
> > > > [...]
> > > > for all symbols listed in src/compat-libs/libsystemd-journal.sym
> > > > 
> > > > I have no idea what happens here, but making 'obsolete_lib()' a noop or
> > > > removing lto from configure.ac 'fixes' the problem.
> > > > 
> > > > This is with gcc-4.8.2 and binutils-2.24 building for ARM.
> > > > 
> > > > Any ideas what happens here?
> > > 
> > > No really. But I figure LTO is not very reliable on ARM and stuff. It's
> > > probably best to turn it off there.
> > 
> > Well it looks like it fails on x86 as well here, with the same compiler
> > version. I can run configure with cc_cv_CFLAGS__flto=no here. I'm not sure,
> > how an upstream fix should look like.
> 
> Hmm, I am not experiencing this here, not sure how I can help you...

That's ok. I've disabled it for now. I'll see if I can find what triggers
the problem here.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] missing: add BPF_XOR

[Michael Olbrich]

On Thu, Aug 21, 2014 at 01:51:50PM +0200, Michal Sekletar wrote:
> On Thu, Aug 21, 2014 at 12:38:08PM +0200, Michael Olbrich wrote:
> > BPF_XOR was introduced in kernel 3.7
> > ---
> > 
> > This fixes compiling systemd for me. I'm not sure about the implications of
> > this. I'm not sure what happens if the code using it is executed on a linux
> > kernel < 3.7
> 
> I don't think that merging this patch makes sense since we don't really 
> support
> combination of old kernel and new systemd. Both components are so centric 
> these
> days that they should be updated in lockstep. Hence allowing people to compile
> against very old kernels shouldn't be possible really.

In that case, the README should be modified accordingly. That said, I've
come across this issue in a cross-compile environment: To generate the
hwdb.bin I need to build systemd for the build system. This failed on
debian stable. I think that's a reasonable use-case.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Build errors with lto and compat-libs

[Michael Olbrich]

On Mon, Aug 18, 2014 at 03:48:09PM +0200, Lennart Poettering wrote:
> On Sun, 17.08.14 09:54, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > With --enable-compat-libs building fails like this:
> > 
> >   CCLD libsystemd-journal.la
> > [...]
> > /tmp/ccISOiYU.ltrans1.ltrans.o: In function `sd_journal_process':
> > ccISOiYU.ltrans1.o:(.text+0x0): multiple definition of `sd_journal_process'
> > libsystemd_journal_internal_la-sd-journal.o (symbol from 
> > plugin):(.text+0x0): first defined here
> > [...]
> > for all symbols listed in src/compat-libs/libsystemd-journal.sym
> > 
> > I have no idea what happens here, but making 'obsolete_lib()' a noop or
> > removing lto from configure.ac 'fixes' the problem.
> > 
> > This is with gcc-4.8.2 and binutils-2.24 building for ARM.
> > 
> > Any ideas what happens here?
> 
> No really. But I figure LTO is not very reliable on ARM and stuff. It's
> probably best to turn it off there.

Well it looks like it fails on x86 as well here, with the same compiler
version. I can run configure with cc_cv_CFLAGS__flto=no here. I'm not sure,
how an upstream fix should look like.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] missing: add BPF_XOR

[Michael Olbrich]

BPF_XOR was introduced in kernel 3.7
---

This fixes compiling systemd for me. I'm not sure about the implications of
this. I'm not sure what happens if the code using it is executed on a linux
kernel < 3.7

Michael

 src/shared/missing.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/shared/missing.h b/src/shared/missing.h
index 3ff1a21..1321db1 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -589,3 +589,7 @@ static inline int setns(int fd, int nstype) {
 #ifndef NET_NAME_RENAMED
 #  define NET_NAME_RENAMED 4
 #endif
+
+#ifndef BPF_XOR
+#  define BPF_XOR 0xa0
+#endif
-- 
2.1.0.rc1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Build errors with lto and compat-libs

[Michael Olbrich]

Hi,

With --enable-compat-libs building fails like this:

  CCLD libsystemd-journal.la
[...]
/tmp/ccISOiYU.ltrans1.ltrans.o: In function `sd_journal_process':
ccISOiYU.ltrans1.o:(.text+0x0): multiple definition of `sd_journal_process'
libsystemd_journal_internal_la-sd-journal.o (symbol from plugin):(.text+0x0): 
first defined here
[...]
for all symbols listed in src/compat-libs/libsystemd-journal.sym

I have no idea what happens here, but making 'obsolete_lib()' a noop or
removing lto from configure.ac 'fixes' the problem.

This is with gcc-4.8.2 and binutils-2.24 building for ARM.

Any ideas what happens here?

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] tmpfiles: explicitly check for existing files

[Michael Olbrich]

On read-only filesystems trying to create the target will not fail with
EEXIST but with EROFS.
---

Some more cases that fail on read-only filesystems.

 src/tmpfiles/tmpfiles.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 3bab7ac..3572367 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -671,7 +671,10 @@ static int create_item(Item *i) {
 break;
 
 case COPY_FILES:
-r = copy_tree(i->argument, i->path, false);
+if (stat(i->path, &st) < 0)
+r = copy_tree(i->argument, i->path, false);
+else
+r = -EEXIST;
 if (r < 0) {
 struct stat a, b;
 
@@ -789,13 +792,17 @@ static int create_item(Item *i) {
 case CREATE_SYMLINK:
 
 label_context_set(i->path, S_IFLNK);
-r = symlink(i->argument, i->path);
+if (stat(i->path, &st) < 0) {
+if (symlink(i->argument, i->path) < 0)
+r = -errno;
+} else
+r = -EEXIST;
 label_context_clear();
 
 if (r < 0) {
 _cleanup_free_ char *x = NULL;
 
-if (errno != EEXIST) {
+if (r != -EEXIST) {
 log_error("symlink(%s, %s) failed: %m", 
i->argument, i->path);
 return -errno;
 }
-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCHv2 1/2] tmpfiles: only execute chmod()/chown() when needed

[Michael Olbrich]

This avoids errors like this, when the paths are already there with the
correct permissions and owner:

chmod(/var/spool) failed: Read-only file system
---

Changes since v1:
- remember if stat() was successful and use it everywhere.

The original code checked for 'stat() >= 0'. Any reason for that? My
man-page says 'On success, zero is returned'.

 src/tmpfiles/tmpfiles.c | 38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 79fd0b7..3bab7ac 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -453,35 +453,39 @@ finish:
 }
 
 static int item_set_perms(Item *i, const char *path) {
+struct stat st;
+bool st_valid;
+
 assert(i);
 assert(path);
 
+st_valid = stat(path, &st) == 0;
+
 /* not using i->path directly because it may be a glob */
 if (i->mode_set) {
 mode_t m = i->mode;
 
-if (i->mask_perms) {
-struct stat st;
-
-if (stat(path, &st) >= 0) {
-if (!(st.st_mode & 0111))
-m &= ~0111;
-if (!(st.st_mode & 0222))
-m &= ~0222;
-if (!(st.st_mode & 0444))
-m &= ~0444;
-if (!S_ISDIR(st.st_mode))
-m &= ~07000; /* remove 
sticky/sgid/suid bit, unless directory */
-}
+if (i->mask_perms && st_valid) {
+if (!(st.st_mode & 0111))
+m &= ~0111;
+if (!(st.st_mode & 0222))
+m &= ~0222;
+if (!(st.st_mode & 0444))
+m &= ~0444;
+if (!S_ISDIR(st.st_mode))
+m &= ~07000; /* remove sticky/sgid/suid bit, 
unless directory */
 }
 
-if (chmod(path, m) < 0) {
-log_error("chmod(%s) failed: %m", path);
-return -errno;
+if (!st_valid || m != (st.st_mode & 0)) {
+if (chmod(path, m) < 0) {
+log_error("chmod(%s) failed: %m", path);
+return -errno;
+}
 }
 }
 
-if (i->uid_set || i->gid_set)
+if ((!st_valid || (i->uid != st.st_uid || i->gid != st.st_gid)) &&
+(i->uid_set || i->gid_set))
 if (chown(path,
   i->uid_set ? i->uid : (uid_t) -1,
   i->gid_set ? i->gid : (gid_t) -1) < 0) {
-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

[Michael Olbrich]

On Thu, Aug 14, 2014 at 05:38:05PM +0200, Lennart Poettering wrote:
> On Fri, 25.07.14 09:48, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> 
> > What I'm _not_ seeing, and what usually comes when anything else changes in
> > the network configuration is:
> > systemd-timesyncd[348]: Network configuration changed, trying to establish 
> > connection.
> > 
> > I would expect, that systemd-timesyncd should be notified in this case as
> > well, right?
> 
> This should be fixed with current git. Could you please recheck?

Indeed:

Aug 01 00:20:15 BaseKit systemd-networkd[434]: eth0: removed 
address: 192.168.51.144/24 (valid for 0)
Aug 01 00:20:15 BaseKit systemd-timesyncd[346]: Network configuration changed, 
trying to establish connection.
Aug 01 00:20:15 BaseKit systemd-networkd[434]: eth0: added address: 
192.168.51.145/24 (valid for 9min 30s)
Aug 01 00:20:15 BaseKit systemd-timesyncd[346]: Network configuration changed, 
trying to establish connection.

I'm not sure, why the new address is found again though.

Note: this is with "net.ipv4.conf.all.promote_secondaries = 1".  Setting
just "net.ipv4.conf.default.promote_secondaries = 1", as it's currently
done in /usr/lib/sysctl.d/50-default.conf is not always sufficient. I think
the default only works for new interfaces that show up afterwards.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: don't fail if IPv6 is not available

[Michael Olbrich]

On Wed, Aug 13, 2014 at 03:04:20PM +0200, Lennart Poettering wrote:
> I applied a different patch now that makes sure we either get the full
> IPv6 support or none at all, and doesn't generate a warning.
> 
> Please have a look, if this fixes things for you.

This work now. However I had to revert
5ba73e9b646af4d8109a5a633aa235665858144d (resolved: clarify that LLMNR
scopes must have a link assigned). dns_scope_llmnr_membership() is called
twice with s->protocol = DNS_PROTOCOL_DNS. dns_scope_new() calls it with
the specified link and protocol here:

$ git grep dns_scope_new | grep AF_UNSPEC
src/resolve/resolved-link.c:r = 
dns_scope_new(l->manager, &l->unicast_scope, l, DNS_PROTOCOL_DNS, AF_UNSPEC);
src/resolve/resolved-manager.c:r = dns_scope_new(m, &m->unicast_scope, 
NULL, DNS_PROTOCOL_DNS, AF_UNSPEC);

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] resolved: don't fail if IPv6 is not available

[Michael Olbrich]

---
 src/resolve/resolved-manager.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index a2de2ed..7063af5 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -429,7 +429,9 @@ static int manager_llmnr_start(Manager *m) {
 r = manager_llmnr_ipv6_udp_fd(m);
 if (r == -EADDRINUSE)
 goto eaddrinuse;
-if (r < 0)
+if (r == -EAFNOSUPPORT)
+log_warning("Failed to setup LLMNR IPv6 UDP socket: %s", 
strerror(-r));
+else if (r < 0)
 return r;
 
 r = manager_llmnr_ipv4_tcp_fd(m);
@@ -441,7 +443,9 @@ static int manager_llmnr_start(Manager *m) {
 r = manager_llmnr_ipv6_tcp_fd(m);
 if (r == -EADDRINUSE)
 goto eaddrinuse;
-if (r < 0)
+if (r == -EAFNOSUPPORT)
+log_warning("Failed to setup LLMNR IPv6 TCP socket: %s", 
strerror(-r));
+else if (r < 0)
 return r;
 
 return 0;
-- 
2.1.0.rc1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] systemd-resolved fails without IPv6

[Michael Olbrich]

Hi,

Without IPv6 systemd-resolved fails to start with:

Failed to start manager: Address family not supported by protocol

strace tells me this is the errno from socket():
[...]
socket(PF_INET6, SOCK_DGRAM|SOCK_CLOEXEC|SOCK_NONBLOCK, IPPROTO_IP) = -1 
EAFNOSUPPORT (Address family not supported by protocol)
[...]

I think this is from manager_llmnr_ipv6_udp_fd().

Is IPv6 now mandatory, or should this be fixed?

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] assert() when restarting systemd-networkd

[Michael Olbrich]

On Wed, Jul 30, 2014 at 06:09:44PM +0200, Tom Gundersen wrote:
> On Sun, Jul 20, 2014 at 3:12 PM, Michael Olbrich
>  wrote:
> > with the current git master (v215-293-g4e6029435111) restarting
> > systemd-networkd triggers an assert() here:
> >
> > In netdev_join_handler():
> > assert(IN_SET(link->state, LINK_STATE_ENSLAVING, LINK_STATE_FAILED,
> >   LINK_STATE_LINGER));
> >
> > gdb tells me that link->state is LINK_STATE_SETTING_ADDRESSES here.
> >
> > It doesn't happen all the time, but when it happens, then the assert()
> > tiggers until the start limit kicks in.
> 
> I reworked this stuff a bit. Can you reproduce with current git?

I've tested git commit bf1594f54ea4b49eee95a16796ec11c55314b2a4.

Now eth0 is not configured at all:
$ ip l
1: lo:  mtu 65536 qdisc noqueue state UNKNOWN mode 
DEFAULT group default 
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0:  mtu 1500 qdisc noop state DOWN mode DEFAULT 
group default qlen 1000
link/ether 52:54:00:12:34:56 brd ff:ff:ff:ff:ff:ff
3: vlan0@eth0:  mtu 1500 qdisc noqueue 
state UNKNOWN mode DEFAULT group default 
link/ether 5e:69:2b:9a:93:07 brd ff:ff:ff:ff:ff:ff

So I can't test the original issue. That only occurs if eth0 gets an
address from the dhcp server.

The log looks like this:

Aug 01 00:00:00 BaseKit systemd[1]: Starting Network Service...
Aug 01 00:00:00 BaseKit systemd-networkd[355]: timestamp of 
'/lib/systemd/network' changed
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : loaded macvlan
Aug 01 00:00:00 BaseKit systemd-networkd[355]: sd-rtnl: discarding 20 bytes of 
incoming message
Aug 01 00:00:00 BaseKit systemd-networkd[355]: eth0: link 2 added
Aug 01 00:00:00 BaseKit systemd-networkd[355]: eth0: udev 
initialized link
Aug 01 00:00:00 BaseKit systemd-networkd[355]: eth0: saved original 
MTU: 1500
Aug 01 00:00:00 BaseKit systemd-networkd[355]: eth0: flags change: 
+MULTICAST +BROADCAST
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : link 1 added
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : udev 
initialized link
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : saved original 
MTU: 0
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : flags change: 
+LOOPBACK +UP +LOWER_UP +RUNNING
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : gained carrier
Aug 01 00:00:00 BaseKit systemd[1]: Started Network Service.
Aug 01 00:00:00 BaseKit systemd-networkd[355]: eth0: link state is 
up-to-date
Aug 01 00:00:00 BaseKit systemd-networkd[355]: eth0: found matching 
network '/lib/systemd/network/eth0.network'
Aug 01 00:00:00 BaseKit systemd-networkd[355]: eth0: enslaving by 
'vlan0'
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : creating
Aug 01 00:00:00 BaseKit systemd-networkd[355]: Sent message type=method_call 
sender=n/a destination=org.freedesktop.DBus object=/org/freedesktop/DBus 
interface=org.freedesktop.DBus member=Hello cookie=1 reply_cookie=0 error=n/a
Aug 01 00:00:00 BaseKit systemd-networkd[355]: Got message type=method_return 
sender=org.freedesktop.DBus destination=:1.1 object=n/a interface=n/a 
member=n/a cookie=1 reply_cookie=1 error=n/a
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : getting 
address failed: Device or resource busy
Aug 01 00:00:00 BaseKit systemd-networkd[355]: Got message type=signal 
sender=org.freedesktop.DBus destination=:1.1 object=/org/freedesktop/DBus 
interface=org.freedesktop.DBus member=NameAcquired cookie=2 reply_cookie=0 
error=n/a
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : link state is 
up-to-date
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : unmanaged
Aug 01 00:00:00 BaseKit systemd-networkd[355]: sd-rtnl: discarding 20 bytes of 
incoming message
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : added address: 
::1/128
Aug 01 00:00:00 BaseKit systemd-networkd[355]: lo  : added address: 
127.0.0.1/8
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : link 3 added
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : udev 
initialized link
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : netdev has 
index 3
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : netdev ready
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : saved original 
MTU: 1500
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : flags change: 
+MULTICAST +BROADCAST
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : created
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : link state is 
up-to-date
Aug 01 00:00:00 BaseKit systemd-networkd[355]: vlan0   : found matching

Re: [systemd-devel] [patch] #include src/shared/missing.h in src/shared/util.h for missing struct file_handle definition

[Michael Olbrich]

On Fri, Aug 01, 2014 at 06:37:51PM +0300, Samuli Suominen wrote:
> On 01/08/14 18:31, Simon McVittie wrote:
> > On 01/08/14 15:53, Simon McVittie wrote:
> >> Best-practice in Autotools projects seems to be to include config.h at
> >> the very top of every .c file, whether it is currently needed or not.
> > Sorry, I'd missed that systemd uses "cc -include
> > $(top_builddir)/config.h ...", which is even better. My issues with
> > btrfs_ioctl_vol_args must have been caused by something else.
> >
> 
> nod. I've just tested the original patch I posted, seems to work here...
> I suppose it's best to apply the initlal patch first, and see if someone
> gets a failed build...

is HAVE_LINUX_BTRFS_H defined for you? I've tried the same thing, and it
failed for me, but only with old kernel headers without linux/btrfs.h

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] tmpfiles: only execute chmod()/chown() when needed

[Michael Olbrich]

On Fri, Jul 11, 2014 at 03:05:05PM +0200, Michael Olbrich wrote:
> This avoids errors like this, when the paths are already there with the
> correct permissions and owner:
> 
> chmod(/var/spool) failed: Read-only file system

Ping. These warnings are rather annoying and they make it hard to find
those that are actually a problem :-/.

Michael

> ---
>  src/tmpfiles/tmpfiles.c | 36 +---
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
> index 68cfa55..4f41f28 100644
> --- a/src/tmpfiles/tmpfiles.c
> +++ b/src/tmpfiles/tmpfiles.c
> @@ -453,35 +453,41 @@ finish:
>  }
>  
>  static int item_set_perms(Item *i, const char *path) {
> +struct stat st;
> +mode_t old_m = ~0;
> +uid_t old_uid = -1;
> +gid_t old_gid = -1;
> +
>  assert(i);
>  assert(path);
>  
> +if (stat(path, &st) >= 0) {
> +old_m = st.st_mode & 0;
> +old_uid = st.st_uid;
> +old_gid = st.st_gid;
> +}
>  /* not using i->path directly because it may be a glob */
>  if (i->mode_set) {
>  mode_t m = i->mode;
>  
> -if (i->mask_perms) {
> -struct stat st;
> -
> -if (stat(path, &st) >= 0) {
> -if (!(st.st_mode & 0111))
> -m &= ~0111;
> -if (!(st.st_mode & 0222))
> -m &= ~0222;
> -if (!(st.st_mode & 0444))
> -m &= ~0444;
> -if (!S_ISDIR(st.st_mode))
> -m &= ~07000; /* remove 
> sticky/sgid/suid bit, unless directory */
> -}
> +if (i->mask_perms && old_m != ~0) {
> +if (!(st.st_mode & 0111))
> +m &= ~0111;
> +if (!(st.st_mode & 0222))
> +m &= ~0222;
> +if (!(st.st_mode & 0444))
> +m &= ~0444;
> +if (!S_ISDIR(st.st_mode))
> +m &= ~07000; /* remove sticky/sgid/suid bit, 
> unless directory */
>  }
>  
> -if (chmod(path, m) < 0) {
> +if (m != old_m && chmod(path, m) < 0) {
>  log_error("chmod(%s) failed: %m", path);
>  return -errno;
>  }
>  }
>  
> -if (i->uid_set || i->gid_set)
> +if ((i->uid_set || i->gid_set) && (i->uid != old_uid || i->gid != 
> old_gid))
>  if (chown(path,
>i->uid_set ? i->uid : (uid_t) -1,
>i->gid_set ? i->gid : (gid_t) -1) < 0) {
> -- 
> 2.0.1
> 
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> 

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

[Michael Olbrich]

Hi,

On Fri, Jul 25, 2014 at 11:00:27AM +0200, Brendan Hide wrote:
> On 24/07/14 14:05, Marcel Holtmann wrote:
> >[snip]
> >>... you already are getting
> >>assigned the correct IP address at first, and then only later does
> >>this change. What is causing your DHCP server to send out a different
> >>IP address later on, I don't know, but it seems unlikely that this is
> >>anything we can fix in the client.
> >I have my doubts as well here. Seems like a misconfigured DHCP server.
> 
> I feel like I'm flogging a dead horse - on the other hand it is
> twitching just that little bit more than I'm comfortable with. ;)
> 
> Since Michael specified that he has configured the DHCP lease to
> last only 10 minutes, it stands to reason that he is getting the new
> *different* IP address after ten minutes. This time period is highly
> unusual - but for testing purposes appropriate. So far there is
> nothing to suggest that the DHCP server is doing anything wrong.
> 
> Michael, can you confirm the above?

Not quite. When networkd sends a request to renew the address, then I keep
the same address. However, when networkd is restarted it starts with a
DHCP discover.
Most of the time I still get the same address, but occasionally I get a new
one. I'm not sure what triggers this. It feels like the whole
discover/ofer/request/ack sequence takes longer for a new address, but I'vv
not measured this.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

[Michael Olbrich]

Hi,

On Thu, Jul 24, 2014 at 11:58:15PM +0200, Tom Gundersen wrote:
> On Thu, Jul 24, 2014 at 10:54 PM, Zbigniew Jędrzejewski-Szmek
>  wrote:
> > On Thu, Jul 24, 2014 at 09:18:14PM +0200, Tom Gundersen wrote:
> >> A quick fix would be to set
> >>
> >> /proc/sys/net/ipv4/conf/all/promote_secondaries
> >>
> >> to 1.
> > Wouldn't it be nice to set it to 1 always. The default setting of 0
> > is a brain damaged trap.
> 
> Yeah, I have been trying to find some info about why one might want it
> to be 0, to me it makes no sense at all. So far I found no reasons to
> keep this default, so unless someone has any new info I'll flip the
> switch.

Something I noted, when I tested this. When the old address expires then I
get this in the log:
systemd-networkd[573]: eth0: removed address: 192.168.51.139/24
systemd-networkd[573]: eth0: added address: 192.168.51.140/24

That looks correct, 192.168.51.139 ist the one that expires and
192.168.51.140 is promoted.

What I'm _not_ seeing, and what usually comes when anything else changes in
the network configuration is:
systemd-timesyncd[348]: Network configuration changed, trying to establish 
connection.

I would expect, that systemd-timesyncd should be notified in this case as
well, right?

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

[Michael Olbrich]

On Thu, Jul 24, 2014 at 02:05:24PM +0200, Marcel Holtmann wrote:
> > Right, I spoke too quickly. What I don't want is to preserve state
> > between reboots, preserving it between restarts of networkd would
> > indeed be fine. We already serialize the dhcp leases to /run, so I'd
> > be happy to take a patch to deserialize these again when networkd
> > starts.
> 
> I am not sure this is useful either. It feels like a bandaid for the time
> where networkd does not have an API to reload configuration changes. Killing
> networkd should give you a clean start from the configuration files and not
> magic cached value somewhere. Otherwise you end up with funny side effects
> all the time and have to explain to users to clear some /run states. Or ask
> them "to switch the computer off and back on again".
> 
> When you are opting for DHCP instead of static addresses, then there is
> really no guarantee for addresses staying the same. At least not for the
> client to know. So unless you own the DHCP server and configure it with
> static assignment. In all cases you have to confirm with the DHCP server that
> you address is still valid.

Getting a new IP address is not the biggest problem. But right now, when
the valid_lft for the _old_ address is over, then then _new_ address is
removed as well and the interface has no address at all! That is not
acceptable. I can live with a different new address for now, but not with
no address at all.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

[Michael Olbrich]

On Wed, Jul 23, 2014 at 04:46:26PM +0200, Marcel Holtmann wrote:
> Hi Michael,
> 
> >>> I think the lease should be remembered and reused in this case.
> >> 
> >> Hm, this sounds like a bug somewhere. When the new discover is sent
> >> out it should send the same identifying information to the server, and
> >> hence be given the same lease back again. Wireshark should tell you if
> >> the fault is networkd's or the DHCP server.
> > 
> > I get the same address at first, but after several minutes the DHCP server
> > decides to offer a new address. I should note, that I have a 10 minute
> > lease time for debugging purposes, so that might make the problem more
> > prominent. I'll see if I can figure out what happens here.
> 
> look at your DHCP server and see what lease time it really hands out after 
> reboot.
> 
> However this is between you and your DHCP server. If you configure a lease
> time of 10 minutes, then that is the only guaranteed time for a given IP
> address. There is no mandate that the server has to give you the same address
> after 10 minutes when you ask again. It is valid to just get a different one.
> And that many home routers try to give you back the same one does not mean
> that they are required to do so.
> 
> The nice DHCP servers will remember your Ethernet address and/or identity
> information and give you back your old IP address. Either with the left over
> lease time or with a brand new lease time. There is really no need to store
> this information on disk. If the lease expired the information on disk are
> stale as well. And since our DHCP implementation is so fast, it makes really
> no difference.
> 
> It is safer start out with a brand new DHCP lease instead of having to deal
> with renewal during boot. At least that way you know the DHCP server is still
> there and you have a valid IP address. Just re-using a stored IP with a
> left-over lease is not safe anyway. You never know what changed in the
> network when you were off.

I think there was some misunderstanding here. I don't want to keep the
lease across reboots. I don't care about that. I think networkd should
remember the lease when restarting networkd only and not send a new dhcp
discover.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

[Michael Olbrich]

On Wed, Jul 23, 2014 at 12:47:37PM +0200, Tom Gundersen wrote:
> On Wed, Jul 23, 2014 at 9:50 AM, Michael Olbrich
>  wrote:
> > I've been experimenting with systemd-networkd to see where it fits my
> > use-cases. I'm looking for some insight if the issues I'm seeing are bugs,
> > features just not implemented yet or if my use-case is out of scope for
> > networkd.
> > The most common use-case I have is rather simple: One ethernet interface
> > that can be configured either with a static IP address or via DHCP. The
> > configuration usually comes from some custom configuration data, so I'll
> > probably have a default config as part of the rootfs and generate the real
> > config during startup in /run. This works rather well and is much nicer
> > than some custom shell scripts.
> > The interesting part is, when the configuration changes: I'm working with
> > embedded devices and network access is rarely part of the primary function
> > of the device. As a result, rebooting the device to change the network
> > config is not acceptable.
> >
> > So I tried to change a static IP address by changing the config and
> > restarting networkd. The result was an interface with two addresses. And
> > worse, the old address was still the preferred on.
> > I realize, that a smooth transition is probably impossible for complex
> > configuration possibilities of networkd. But I don't need smooth. How about
> > optionally shutting down an interface when networkd stops?
> > ShutdownWithNetworkd=yes or something like that in the config file?
> 
> Dynamic changing of settings has not been implemented yet, we are so
> far only focussing on the static setups. We definitely will cover more
> dynamic things in the future though, such as your usecase.
> 
> We'll have a dbus interface, and not use stop/restart as a
> configuration mechanism, so I don't think tearing things down on
> shutdown of the demon makes much sense.

That sounds interesting.

> In the meantime, you could hack this by doing:
> 
> systemctl stop systemd-networkd
> ip link address flush dev 
> systemctl start systemd-networkd

I know, but that's not very nice, and I need to handle nfs-root special
cases manually...

> > Another thing I noticed is with DHCP without changing the configuration:
> > Networkd forgets the lease during restart and sends a dhcp discover. Then
> > the server might offer a different address. The problem is, that the new
> > address is in the same subnet as the old address, so it is added as
> > secondary address. When the valid_lft of the old address expires then
> > _both_ addresses are removed :-/.
> 
> Thanks for this report, we should figure out a way that the new
> address is not lost when the first one is.

I looked at the code, but I need to learn more about netlink before I
understand the little details there.

> > I think the lease should be remembered and reused in this case.
> 
> Hm, this sounds like a bug somewhere. When the new discover is sent
> out it should send the same identifying information to the server, and
> hence be given the same lease back again. Wireshark should tell you if
> the fault is networkd's or the DHCP server.

I get the same address at first, but after several minutes the DHCP server
decides to offer a new address. I should note, that I have a 10 minute
lease time for debugging purposes, so that might make the problem more
prominent. I'll see if I can figure out what happens here.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Changing configurations with networkd

[Michael Olbrich]

Hi,

I've been experimenting with systemd-networkd to see where it fits my
use-cases. I'm looking for some insight if the issues I'm seeing are bugs,
features just not implemented yet or if my use-case is out of scope for
networkd.
The most common use-case I have is rather simple: One ethernet interface
that can be configured either with a static IP address or via DHCP. The
configuration usually comes from some custom configuration data, so I'll
probably have a default config as part of the rootfs and generate the real
config during startup in /run. This works rather well and is much nicer
than some custom shell scripts.
The interesting part is, when the configuration changes: I'm working with
embedded devices and network access is rarely part of the primary function
of the device. As a result, rebooting the device to change the network
config is not acceptable.

So I tried to change a static IP address by changing the config and
restarting networkd. The result was an interface with two addresses. And
worse, the old address was still the preferred on.
I realize, that a smooth transition is probably impossible for complex
configuration possibilities of networkd. But I don't need smooth. How about
optionally shutting down an interface when networkd stops?
ShutdownWithNetworkd=yes or something like that in the config file?

Another thing I noticed is with DHCP without changing the configuration:
Networkd forgets the lease during restart and sends a dhcp discover. Then
the server might offer a different address. The problem is, that the new
address is in the same subnet as the old address, so it is added as
secondary address. When the valid_lft of the old address expires then
_both_ addresses are removed :-/.
I think the lease should be remembered and reused in this case.

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] assert() when restarting systemd-networkd

[Michael Olbrich]

Hi,

with the current git master (v215-293-g4e6029435111) restarting
systemd-networkd triggers an assert() here:

In netdev_join_handler():
assert(IN_SET(link->state, LINK_STATE_ENSLAVING, LINK_STATE_FAILED,
  LINK_STATE_LINGER));

gdb tells me that link->state is LINK_STATE_SETTING_ADDRESSES here.

It doesn't happen all the time, but when it happens, then the assert()
tiggers until the start limit kicks in.

My configuration looks like this:
 eth0.network---
[Match]
Name=eth0

[Network]
DHCP=v4
IPv4LL=yes
MACVLAN=vlan0
 vlan0.netdev
[NetDev]
Name=vlan0
Kind=macvlan
[MACVLAN]
Mode=private
 vlan0.network---
[Match]
Name=vlan0

[Network]
Address=10.1.42.4/24
-

After adding Environment=SYSTEMD_LOG_LEVEL=debug to the service the journal
looks like this:

Jul 01 00:13:39 BaseKit systemd[1]: Starting Network Service...
Jul 01 00:13:39 BaseKit systemd-networkd[553]: timestamp of 
'/lib/systemd/network' changed
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : loaded macvlan
Jul 01 00:13:39 BaseKit systemd-networkd[553]: sd-rtnl: discarding 20 bytes of 
incoming message
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : link 3 added
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : udev 
initialized link
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : netdev has 
index 3
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : flags change: 
+UP +LOWER_UP +RUNNING +MULTICAST +BROADCAST
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : gained carrier
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: link 2 added
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: udev 
initialized link
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: flags change: 
+UP +LOWER_UP +RUNNING +MULTICAST +BROADCAST
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: gained carrier
Jul 01 00:13:39 BaseKit systemd-networkd[553]: lo  : link 1 added
Jul 01 00:13:39 BaseKit systemd-networkd[553]: lo  : udev 
initialized link
Jul 01 00:13:39 BaseKit systemd-networkd[553]: lo  : flags change: 
+LOOPBACK +UP +LOWER_UP +RUNNING
Jul 01 00:13:39 BaseKit systemd-networkd[553]: lo  : gained carrier
Jul 01 00:13:39 BaseKit systemd[1]: Started Network Service.
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : link state is 
up-to-date
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : found matching 
network '/lib/systemd/network/vlan0.network'
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : joined netdev
Jul 01 00:13:39 BaseKit systemd-networkd[553]: vlan0   : setting 
addresses
Jul 01 00:13:39 BaseKit systemd-networkd[553]: Sent message type=method_call 
sender=n/a destination=org.freedesktop.DBus object=/org/freedesktop/DBus 
interface=org.freedesktop.DBus member=Hello cookie=1 reply_cookie=0 error=n/a
Jul 01 00:13:39 BaseKit systemd-networkd[553]: Got message type=method_return 
sender=org.freedesktop.DBus destination=:1.26 object=n/a interface=n/a 
member=n/a cookie=1 reply_cookie=1 error=n/a
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: getting 
address failed: Device or resource busy
Jul 01 00:13:39 BaseKit systemd-networkd[553]: Got message type=signal 
sender=org.freedesktop.DBus destination=:1.26 object=/org/freedesktop/DBus 
interface=org.freedesktop.DBus member=NameAcquired cookie=2 reply_cookie=0 
error=n/a
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: link state is 
up-to-date
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: found matching 
network '/lib/systemd/network/eth0.network'
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: acquiring IPv4 
link-local address
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: acquiring 
DHCPv4 lease
Jul 01 00:13:39 BaseKit systemd-networkd[553]: DHCP CLIENT (0x97290dbb): 
STARTED on ifindex 2 with address 52:54:0:12:34:56
Jul 01 00:13:39 BaseKit systemd-networkd[553]: eth0: enslaving by 
'vlan0'
Jul 01 00:13:39 BaseKit systemd-networkd[553]: IPv4LL: PROBE
Jul 01 00:13:39 BaseKit systemd-networkd[553]: DHCP CLIENT (0x97290dbb): 
DISCOVER
Jul 01 00:13:39 BaseKit systemd-networkd[553]: lo  : getting 
address failed: Device or resource busy
Jul 01 00:13:39 BaseKit systemd-networkd[553]: DHCP CLIENT (0x97290dbb): OFFER
Jul 01 00:13:39 BaseKit systemd-networkd[553]: lo  : link state is 
up-to-date
Jul 01 00:13:39 BaseKit systemd-networkd[553]: lo  : unmanaged
Jul 01 00:13:39 BaseKit systemd-networkd[553]: DHCP CLIENT (0x97290dbb): 
REQUEST (requesting)
Jul 01 00:13:39 BaseKit systemd-networkd[553]: DHCP CLIENT (0x97290dbb): ACK
Jul 01 00:13:39 BaseKit systemd-networkd[553]: DHCP CLIENT (0x97290dbb): lease 
expires in 4min 31.

[systemd-devel] [PATCH] install: systemd-timesyncd.service is enabled by sysinit.target

[Michael Olbrich]

systemd-timesyncd.service has a "WantedBy=sysinit.target" so the
initially generated link should match that.
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index db846ad..d1e3b0b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4507,7 +4507,7 @@ nodist_systemunit_DATA += \
units/systemd-timesyncd.service
 
 GENERAL_ALIASES += \
-   $(systemunitdir)/systemd-timesyncd.service 
$(pkgsysconfdir)/system/multi-user.target.wants/systemd-timesyncd.service
+   $(systemunitdir)/systemd-timesyncd.service 
$(pkgsysconfdir)/system/sysinit.target.wants/systemd-timesyncd.service
 
 EXTRA_DIST += \
units/systemd-timesyncd.service.in
-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] tmpfiles: only execute chmod()/chown() when needed

[Michael Olbrich]

On Fri, Jul 11, 2014 at 03:52:21PM -0700, Colin Walters wrote:
> On Fri, Jul 11, 2014, at 03:04 PM, Dave Reisner wrote:
> > No way. This precludes tmpfiles from creating directories in /run.
> 
> Yeah that suggestion would break other stuff too, ignore it.
> 
> This does get into the ostree commit I linked to though; we could just
> have systemd mount /var as a tmpfs if the filesystem is readonly.  
> 
> I'm guessing Michael is doing something like generating a disk image
> that's composed using traditional rpm/dpkg type tooling which lays out
> skeleton directories in /var, and then booting it as readonly.  That
> sort of situation is why we changed OSTree to auto-mount a /var tmpfs,
> and it's worth thinking about at least if systemd should do the same.

Yes, I'm generating the directories as part of an rootfs image. I don't
really need everything that tmp-files checks for, but I'd rather avoid
patching systemd, and a few extra directories are not a problem.

> The main concern I had is that if you're booting the system readonly to
> try to say recover the journald logs, it'd be kind of annoying to have
> them vanish...
> 
> Another option is to have systemd-tmpfiles silently ignore EROFS.

No, it should warn if _real_ changes fail, but right now chown/chmod are
executed and fail, even if directories/files already exist with the correct
permissions.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] units/serial-getty@.service: use the default RestartSec

[Michael Olbrich]

For pluggable ttys such as USB serial devices, the getty is restarted
and exits in a loop until the remove event reaches systemd. Under
certain circumstances the restart loop can overload the system in a
way that prevents the remove event from reaching systemd for a long
time (e.g. at least several minutes on a small embedded system).

Use the default RestartSec to prevent the restart loop from
overloading the system. Serial gettys are interactive units, so
waiting an extra 100ms really doesn't make a difference anyways
compared to the time it takes the user to log in.
--

I'm not sure why this fails now, but I'm not sure if that was because of
systemd (I used v204 before) or the Kernel.
Either way I think this is a good idea to make things more robust.

Michael

 units/serial-getty@.service.m4 | 1 -
 1 file changed, 1 deletion(-)

diff --git a/units/serial-getty@.service.m4 b/units/serial-getty@.service.m4
index 4ac51e7..4522d0d 100644
--- a/units/serial-getty@.service.m4
+++ b/units/serial-getty@.service.m4
@@ -25,7 +25,6 @@ IgnoreOnIsolate=yes
 ExecStart=-/sbin/agetty --keep-baud 115200,38400,9600 %I $TERM
 Type=idle
 Restart=always
-RestartSec=0
 UtmpIdentifier=%I
 TTYPath=/dev/%I
 TTYReset=yes
-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] architecture: fix building for big-endian PowerPC with gcc

[Michael Olbrich]

On Fri, Jul 11, 2014 at 04:01:32PM +0200, Lennart Poettering wrote:
> On Fri, 11.07.14 15:43, Dan Horák (d...@danny.cz) wrote:
> > > Hmm, the sources currently use three different ways to detect
> > > endianess:
> > > 
> > > 1) WORDS_BIGENDIAN (which appears to be an autoconf thing actually,
> > >enabled via AC_C_BIGENDIAN, which we never call currently)
> > > 2) __BTYE_ORDER == __BIG_ENDIAN which appears to be be defined in
> > >endian.h always by glibc
> > > 3) __BIG_ENDIAN__ appears to be quite common too? gcc thing?
> > > 
> > > Also this:
> > > 
> > > http://sourceforge.net/p/predef/wiki/Endianness/
> > > 
> > > We should probably stick to one of these things. Probably #2 or #3...
> > > 
> > > I can't find any normative documentation about these things. Given
> > > that the #2 is at least unconditional part of glibc, I'd say to
> > > convert everything to that. Would that work for you on ppc-be too?
> > 
> > Jakub explained us endianity checks with GCC in
> > https://bugzilla.redhat.com/show_bug.cgi?id=962091#c48
> 
> Apparently #2 is and OK check according to that.
> 
> I have now changed git to use that for all its checks. Michael, check if
> this now makes things work for you correctly.

It compiles again for me. But i noticed, that endian.h is not explicitly
included. Are you sure that it's always included indirectly?

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] architecture: fix building for big-endian PowerPC with gcc

[Michael Olbrich]

On Fri, Jul 11, 2014 at 03:27:56PM +0200, Lennart Poettering wrote:
> On Fri, 11.07.14 15:04, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> 
> > gcc does not define WORDS_BIGENDIAN for big-endian PowerPC. It defines
> > __BIG_ENDIAN__ instead. So also check for __BIG_ENDIAN__ to determine if
> > the system is big-endian.
> 
> Hmm, the sources currently use three different ways to detect endianess:
> 
> 1) WORDS_BIGENDIAN (which appears to be an autoconf thing actually,
>enabled via AC_C_BIGENDIAN, which we never call currently)
> 2) __BTYE_ORDER == __BIG_ENDIAN which appears to be be defined in
>endian.h always by glibc
> 3) __BIG_ENDIAN__ appears to be quite common too? gcc thing?
> 
> Also this:
> 
> http://sourceforge.net/p/predef/wiki/Endianness/
> 
> We should probably stick to one of these things. Probably #2 or #3...
> 
> I can't find any normative documentation about these things. Given that
> the #2 is at least unconditional part of glibc, I'd say to convert
> everything to that. Would that work for you on ppc-be too?

Yes that's defined correctly.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] tmpfiles: only execute chmod()/chown() when needed

[Michael Olbrich]

This avoids errors like this, when the paths are already there with the
correct permissions and owner:

chmod(/var/spool) failed: Read-only file system
---
 src/tmpfiles/tmpfiles.c | 36 +---
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 68cfa55..4f41f28 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -453,35 +453,41 @@ finish:
 }
 
 static int item_set_perms(Item *i, const char *path) {
+struct stat st;
+mode_t old_m = ~0;
+uid_t old_uid = -1;
+gid_t old_gid = -1;
+
 assert(i);
 assert(path);
 
+if (stat(path, &st) >= 0) {
+old_m = st.st_mode & 0;
+old_uid = st.st_uid;
+old_gid = st.st_gid;
+}
 /* not using i->path directly because it may be a glob */
 if (i->mode_set) {
 mode_t m = i->mode;
 
-if (i->mask_perms) {
-struct stat st;
-
-if (stat(path, &st) >= 0) {
-if (!(st.st_mode & 0111))
-m &= ~0111;
-if (!(st.st_mode & 0222))
-m &= ~0222;
-if (!(st.st_mode & 0444))
-m &= ~0444;
-if (!S_ISDIR(st.st_mode))
-m &= ~07000; /* remove 
sticky/sgid/suid bit, unless directory */
-}
+if (i->mask_perms && old_m != ~0) {
+if (!(st.st_mode & 0111))
+m &= ~0111;
+if (!(st.st_mode & 0222))
+m &= ~0222;
+if (!(st.st_mode & 0444))
+m &= ~0444;
+if (!S_ISDIR(st.st_mode))
+m &= ~07000; /* remove sticky/sgid/suid bit, 
unless directory */
 }
 
-if (chmod(path, m) < 0) {
+if (m != old_m && chmod(path, m) < 0) {
 log_error("chmod(%s) failed: %m", path);
 return -errno;
 }
 }
 
-if (i->uid_set || i->gid_set)
+if ((i->uid_set || i->gid_set) && (i->uid != old_uid || i->gid != 
old_gid))
 if (chown(path,
   i->uid_set ? i->uid : (uid_t) -1,
   i->gid_set ? i->gid : (gid_t) -1) < 0) {
-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] architecture: fix building for big-endian PowerPC with gcc

[Michael Olbrich]

gcc does not define WORDS_BIGENDIAN for big-endian PowerPC. It defines
__BIG_ENDIAN__ instead. So also check for __BIG_ENDIAN__ to determine if
the system is big-endian.
---
 src/shared/architecture.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/shared/architecture.h b/src/shared/architecture.h
index 4821d5d..ba42761 100644
--- a/src/shared/architecture.h
+++ b/src/shared/architecture.h
@@ -80,7 +80,7 @@ Architecture uname_architecture(void);
 #  define native_architecture() ARCHITECTURE_X86
 #  define LIB_ARCH_TUPLE "i386-linux-gnu"
 #elif defined(__powerpc64__)
-#  if defined(WORDS_BIGENDIAN)
+#  if defined(WORDS_BIGENDIAN) || defined(__BIG_ENDIAN__)
 #define native_architecture() ARCHITECTURE_PPC64
 #define LIB_ARCH_TUPLE "ppc64-linux-gnu"
 #  else
@@ -88,7 +88,7 @@ Architecture uname_architecture(void);
 #error "Missing LIB_ARCH_TUPLE for PPC64LE"
 #  endif
 #elif defined(__powerpc__)
-#  if defined(WORDS_BIGENDIAN)
+#  if defined(WORDS_BIGENDIAN) || defined(__BIG_ENDIAN__)
 #define native_architecture() ARCHITECTURE_PPC
 #define LIB_ARCH_TUPLE "powerpc-linux-gnu"
 #  else
-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] install: enable timesyncd by default

[Michael Olbrich]

This treats it similarly to networkd, resolved and others and it matches
what 90-systemd.preset does.
---
 Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 4b292b2..bfef1c3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4301,6 +4301,9 @@ rootlibexec_PROGRAMS += \
 nodist_systemunit_DATA += \
units/systemd-timesyncd.service
 
+GENERAL_ALIASES += \
+   $(systemunitdir)/systemd-timesyncd.service 
$(pkgsysconfdir)/system/multi-user.target.wants/systemd-timesyncd.service
+
 EXTRA_DIST += \
units/systemd-timesyncd.service.in
 
-- 
2.0.0

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] systemd-networkd-wait-online and network.target

[Michael Olbrich]

Hi,

Commit 58e027023b47b32e42cf93dd4a629b869ee1ef25 'units: order
network-online.target after network.target' added "Before=network.target"
dependency to systemd-networkd-wait-online.service. Is that correct? If I
understand the documentation correctly, then network.target should not be
delayed. Maybe this was supposed to be "Afternetwork.target"?

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2 2/2] service: rename StartLimitAction enum to FailureAction

[Michael Olbrich]

It's used for the FailureAction property as well.
---
Changes since v1:
- rebased on latest master
- also rename things in src/test/test-tables.c

 src/core/dbus-service.c   |  6 +++---
 src/core/load-fragment-gperf.gperf.m4 |  4 ++--
 src/core/load-fragment.c  |  4 ++--
 src/core/load-fragment.h  |  2 +-
 src/core/service.c| 32 
 src/core/service.h| 24 
 src/test/test-tables.c|  2 +-
 7 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 2f9c25a..093289f 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -34,7 +34,7 @@ static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_type, 
service_type, ServiceType
 static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, service_result, 
ServiceResult);
 static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_restart, service_restart, 
ServiceRestart);
 static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_notify_access, notify_access, 
NotifyAccess);
-static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_start_limit_action, 
start_limit_action, StartLimitAction);
+static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_failure_action, 
failure_action, FailureAction);
 
 const sd_bus_vtable bus_service_vtable[] = {
 SD_BUS_VTABLE_START(0),
@@ -49,9 +49,9 @@ const sd_bus_vtable bus_service_vtable[] = {
 BUS_PROPERTY_DUAL_TIMESTAMP("WatchdogTimestamp", offsetof(Service, 
watchdog_timestamp), 0),
 SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, 
offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, 
offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
-SD_BUS_PROPERTY("StartLimitAction", "s", 
property_get_start_limit_action, offsetof(Service, start_limit_action), 
SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, 
offsetof(Service, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Service, 
reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST),
-SD_BUS_PROPERTY("FailureAction", "s", property_get_start_limit_action, 
offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("FailureAction", "s", property_get_failure_action, 
offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, 
offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, 
offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RemainAfterExit", "b", bus_property_get_bool, 
offsetof(Service, remain_after_exit), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index da6df5c..b8d8738 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -179,9 +179,9 @@ Service.TimeoutStopSec,  
config_parse_service_timeout,   0,
 Service.WatchdogSec, config_parse_sec,   0,
 offsetof(Service, watchdog_usec)
 Service.StartLimitInterval,  config_parse_sec,   0,
 offsetof(Service, start_limit.interval)
 Service.StartLimitBurst, config_parse_unsigned,  0,
 offsetof(Service, start_limit.burst)
-Service.StartLimitAction,config_parse_start_limit_action,0,
 offsetof(Service, start_limit_action)
+Service.StartLimitAction,config_parse_failure_action,0,
 offsetof(Service, start_limit_action)
 Service.RebootArgument,  config_parse_string,0,
 offsetof(Service, reboot_arg)
-Service.FailureAction,   config_parse_start_limit_action,0,
 offsetof(Service, failure_action)
+Service.FailureAction,   config_parse_failure_action,0,
 offsetof(Service, failure_action)
 Service.Type,config_parse_service_type,  0,
 offsetof(Service, type)
 Service.Restart, config_parse_service_restart,   0,
 offsetof(Service, restart)
 Service.PermissionsStartOnly,config_parse_bool,  0,
 offsetof(Service, permissions_start_only)
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index c604f90..6c92935 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -2

[systemd-devel] [PATCH v2 1/2] service: add FailureAction= option

[Michael Olbrich]

It has the same possible values as StartLimitAction= and is executed
immediately if a service fails.
---
Changes since v1:
- rebased on latest master

 man/systemd.service.xml   | 11 ++
 src/core/dbus-service.c   |  1 +
 src/core/load-fragment-gperf.gperf.m4 |  1 +
 src/core/service.c| 39 ++-
 src/core/service.h|  2 ++
 5 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index a2a1b6b..af32ccb 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -1030,6 +1030,17 @@ ExecStart=/bin/echo $ONE $TWO ${TWO}
 command.
 
 
+
+FailureAction=
+Configure the action
+to take when the service enters a failed
+state. Takes the same values as
+StartLimitAction=
+and executes the same actions.
+Defaults to none.
+
+
+
 
 
 Check
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 45bfecf..2f9c25a 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -51,6 +51,7 @@ const sd_bus_vtable bus_service_vtable[] = {
 SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, 
offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitAction", "s", 
property_get_start_limit_action, offsetof(Service, start_limit_action), 
SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Service, 
reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("FailureAction", "s", property_get_start_limit_action, 
offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, 
offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, 
offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RemainAfterExit", "b", bus_property_get_bool, 
offsetof(Service, remain_after_exit), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index cb98c83..da6df5c 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -181,6 +181,7 @@ Service.StartLimitInterval,  config_parse_sec,  
 0,
 Service.StartLimitBurst, config_parse_unsigned,  0,
 offsetof(Service, start_limit.burst)
 Service.StartLimitAction,config_parse_start_limit_action,0,
 offsetof(Service, start_limit_action)
 Service.RebootArgument,  config_parse_string,0,
 offsetof(Service, reboot_arg)
+Service.FailureAction,   config_parse_start_limit_action,0,
 offsetof(Service, failure_action)
 Service.Type,config_parse_service_type,  0,
 offsetof(Service, type)
 Service.Restart, config_parse_service_restart,   0,
 offsetof(Service, restart)
 Service.PermissionsStartOnly,config_parse_bool,  0,
 offsetof(Service, permissions_start_only)
diff --git a/src/core/service.c b/src/core/service.c
index 4ebce6a..593237a 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1840,6 +1840,8 @@ static int cgroup_good(Service *s) {
 return !r;
 }
 
+static int service_execute_action(Service *s, StartLimitAction action, const 
char *reason, bool log_action_none);
+
 static void service_enter_dead(Service *s, ServiceResult f, bool 
allow_restart) {
 int r;
 assert(s);
@@ -1849,6 +1851,9 @@ static void service_enter_dead(Service *s, ServiceResult 
f, bool allow_restart)
 
 service_set_state(s, s->result != SERVICE_SUCCESS ? SERVICE_FAILED : 
SERVICE_DEAD);
 
+if (s->result != SERVICE_SUCCESS)
+service_execute_action(s, s->failure_action, "failed", false);
+
 if (allow_restart &&
 !s->forbid_restart &&
 (s->restart == SERVICE_RESTART_ALWAYS ||
@@ -2371,22 +2376,19 @@ fail:
 service_enter_stop(s, SERVICE_FAILURE_RESOURCES);
 }
 
-static int service_start_limit_test(Service *s) {
+static int service_execute_action(Service *s, StartLimitAction action, const 
char *reason, bool log_action_none) {
 assert(s);
 
-if (ratelimit_test(&s->sta

[systemd-devel] [PATCH 2/2] service: rename StartLimitAction enum to FailureAction

[Michael Olbrich]

It's used for the FailureAction property as well.
---
 src/core/dbus-service.c   |  6 +++---
 src/core/load-fragment-gperf.gperf.m4 |  4 ++--
 src/core/load-fragment.c  |  4 ++--
 src/core/load-fragment.h  |  2 +-
 src/core/service.c| 28 ++--
 src/core/service.h| 24 
 6 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 1e710e3..335f4b2 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -34,7 +34,7 @@ static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_type, 
service_type, ServiceType
 static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, service_result, 
ServiceResult);
 static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_restart, service_restart, 
ServiceRestart);
 static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_notify_access, notify_access, 
NotifyAccess);
-static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_start_limit_action, 
start_limit_action, StartLimitAction);
+static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_failure_action, 
failure_action, FailureAction);
 
 const sd_bus_vtable bus_service_vtable[] = {
 SD_BUS_VTABLE_START(0),
@@ -49,8 +49,8 @@ const sd_bus_vtable bus_service_vtable[] = {
 BUS_PROPERTY_DUAL_TIMESTAMP("WatchdogTimestamp", offsetof(Service, 
watchdog_timestamp), 0),
 SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, 
offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, 
offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
-SD_BUS_PROPERTY("StartLimitAction", "s", 
property_get_start_limit_action, offsetof(Service, start_limit_action), 
SD_BUS_VTABLE_PROPERTY_CONST),
-SD_BUS_PROPERTY("FailureAction", "s", property_get_start_limit_action, 
offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, 
offsetof(Service, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("FailureAction", "s", property_get_failure_action, 
offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, 
offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, 
offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RemainAfterExit", "b", bus_property_get_bool, 
offsetof(Service, remain_after_exit), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index dc83247..b9ee482 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -179,8 +179,8 @@ Service.TimeoutStopSec,  
config_parse_service_timeout,   0,
 Service.WatchdogSec, config_parse_sec,   0,
 offsetof(Service, watchdog_usec)
 Service.StartLimitInterval,  config_parse_sec,   0,
 offsetof(Service, start_limit.interval)
 Service.StartLimitBurst, config_parse_unsigned,  0,
 offsetof(Service, start_limit.burst)
-Service.StartLimitAction,config_parse_start_limit_action,0,
 offsetof(Service, start_limit_action)
-Service.FailureAction,   config_parse_start_limit_action,0,
 offsetof(Service, failure_action)
+Service.StartLimitAction,config_parse_failure_action,0,
 offsetof(Service, start_limit_action)
+Service.FailureAction,   config_parse_failure_action,0,
 offsetof(Service, failure_action)
 Service.Type,config_parse_service_type,  0,
 offsetof(Service, type)
 Service.Restart, config_parse_service_restart,   0,
 offsetof(Service, restart)
 Service.PermissionsStartOnly,config_parse_bool,  0,
 offsetof(Service, permissions_start_only)
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index c604f90..6c92935 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -2000,7 +2000,7 @@ int config_parse_unit_condition_null(const char *unit,
 }
 
 DEFINE_CONFIG_PARSE_ENUM(config_parse_notify_access, notify_access, 
NotifyAccess, "Failed to parse notify access specifier");
-DEFINE_CONFIG_PARSE_ENUM(config_parse_start_limit_action, start_limit_action, 
StartLimitAction, "Failed to parse start limit action specifier");
+DEFINE_CONFIG_PARSE_ENUM(config_parse_fa

[systemd-devel] [PATCH 1/2] service: add FailureAction= option

[Michael Olbrich]

It has the same possible values as StartLimitAction= and is executed
immediately if a service fails.
---
 man/systemd.service.xml   | 11 +++
 src/core/dbus-service.c   |  1 +
 src/core/load-fragment-gperf.gperf.m4 |  1 +
 src/core/service.c| 35 +++
 src/core/service.h|  2 ++
 5 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 5b3afb8..0eb02bc 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -1017,6 +1017,17 @@ ExecStart=/bin/echo $ONE $TWO ${TWO}
 none.
 
 
+
+FailureAction=
+Configure the action
+to take when the service enters a failed
+state. Takes the same values as
+StartLimitAction=
+and executes the same actions.
+Defaults to none.
+
+
+
 
 
 Check
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 0451790..1e710e3 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -50,6 +50,7 @@ const sd_bus_vtable bus_service_vtable[] = {
 SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, 
offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, 
offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitAction", "s", 
property_get_start_limit_action, offsetof(Service, start_limit_action), 
SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("FailureAction", "s", property_get_start_limit_action, 
offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, 
offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, 
offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RemainAfterExit", "b", bus_property_get_bool, 
offsetof(Service, remain_after_exit), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index dbb5d13..dc83247 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -180,6 +180,7 @@ Service.WatchdogSec, config_parse_sec,  
 0,
 Service.StartLimitInterval,  config_parse_sec,   0,
 offsetof(Service, start_limit.interval)
 Service.StartLimitBurst, config_parse_unsigned,  0,
 offsetof(Service, start_limit.burst)
 Service.StartLimitAction,config_parse_start_limit_action,0,
 offsetof(Service, start_limit_action)
+Service.FailureAction,   config_parse_start_limit_action,0,
 offsetof(Service, failure_action)
 Service.Type,config_parse_service_type,  0,
 offsetof(Service, type)
 Service.Restart, config_parse_service_restart,   0,
 offsetof(Service, restart)
 Service.PermissionsStartOnly,config_parse_bool,  0,
 offsetof(Service, permissions_start_only)
diff --git a/src/core/service.c b/src/core/service.c
index ae3695a..a71885c 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1835,6 +1835,8 @@ static int cgroup_good(Service *s) {
 return !r;
 }
 
+static int service_execute_action(Service *s, StartLimitAction action, const 
char *reason, bool log_action_none);
+
 static void service_enter_dead(Service *s, ServiceResult f, bool 
allow_restart) {
 int r;
 assert(s);
@@ -1844,6 +1846,9 @@ static void service_enter_dead(Service *s, ServiceResult 
f, bool allow_restart)
 
 service_set_state(s, s->result != SERVICE_SUCCESS ? SERVICE_FAILED : 
SERVICE_DEAD);
 
+if (s->result != SERVICE_SUCCESS)
+service_execute_action(s, s->failure_action, "failed", false);
+
 if (allow_restart &&
 !s->forbid_restart &&
 (s->restart == SERVICE_RESTART_ALWAYS ||
@@ -2366,18 +2371,15 @@ fail:
 service_enter_stop(s, SERVICE_FAILURE_RESOURCES);
 }
 
-static int service_start_limit_test(Service *s) {
+static int service_execute_action(Service *s, StartLimitAction action, const 
char *reason, bool log_action_none) {
 assert(s);
 
-if (ratelimit_test(&s->start_limi

Re: [systemd-devel] [RFC PATCH] service: add FailureAction= option

[Michael Olbrich]

On Fri, Apr 11, 2014 at 04:07:46PM +0200, Lennart Poettering wrote:
> On Fri, 11.04.14 09:48, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > > > +else if (allow_restart &&
> > > 
> > > I would drop the "else" here, I think. Is there a reason not to do the
> > > restart thing anyway? If it is configured, it should run I think, just
> > > in case the failure action doesn't work or so...
> > 
> > Are you sure? With Restart=always and FailureAction=reboot this would
> > try to start the unit while shutting down. Will the Conflicts= from the
> > default dependencies handle this correctly?
> 
> It should handle this correctly, and if it doesn't we should fix this. I
> mean, my thinking here is that combining FailureAction= and
> Restart=failure might not make much sense but there isn't really a
> strong reason to totally prohibit it...

With lots of debugging enabled it looks like this:

wd-test.service failed, rebooting.
Trying to enqueue job reboot.target/start/replace
Installed new job reboot.target/start as 157
Installed new job systemd-reboot.service/start as 158
Installed new job shutdown.target/start as 159
[...]
wd-test.service changed failed -> auto-restart
wd-test.service: cgroup is empty
Accepted new private connection.
wd-test.service holdoff time over, scheduling restart.
Trying to enqueue job wd-test.service/restart/fail
[...]
wd-test.service failed to schedule restart job: Transaction is destructive.
wd-test.service changed auto-restart -> failed
Unit wd-test.service entered failed state.
wd-test.service failed, rebooting.
Trying to enqueue job reboot.target/start/replace
Merged into installed job reboot.target/start as 157
Merged into installed job systemd-reboot.service/start as 158
Merged into installed job shutdown.target/start as 159
[...]

The restart is canceled and the new reboot jobs are merged into the already
running jobs. I'd say the falls under 'handle this correctly'.
I'll send out a new patch when I'm done testing.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC PATCH] service: add FailureAction= option

[Michael Olbrich]

On Fri, Apr 11, 2014 at 03:34:42AM +0200, Lennart Poettering wrote:
> On Wed, 26.03.14 10:02, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> 
> > It has the same possible values as StartLimitAction= and is executed
> > immediately if a service fails.
> 
> I think the enum type should probably be renamed to FailureAction, since
> that now sounds like the more generic name. 

Ok.

> > ---
> > 
> > Hi Lennart,
> > 
> > Something like this maybe? I'm not quite sure about the condition in
> > service_enter_dead(). I don't think the action should be executed when the
> > service is explicitly stopped. Maybe it should depend on !forbid_restart?
> > 
> > If you like, I'll add some documentation. An maybe a follow-up patch to
> > rename the StartLimitAction type? To what though?
> 
> > index ae3695a..ab161a5 100644
> > --- a/src/core/service.c
> > +++ b/src/core/service.c
> > @@ -1835,6 +1835,8 @@ static int cgroup_good(Service *s) {
> >  return !r;
> >  }
> >  
> > +static int service_execute_action(Service *s, StartLimitAction action, 
> > const char *reason);
> > +
> >  static void service_enter_dead(Service *s, ServiceResult f, bool 
> > allow_restart) {
> >  int r;
> >  assert(s);
> > @@ -1844,7 +1846,9 @@ static void service_enter_dead(Service *s, 
> > ServiceResult f, bool allow_restart)
> >  
> >  service_set_state(s, s->result != SERVICE_SUCCESS ? SERVICE_FAILED 
> > : SERVICE_DEAD);
> >  
> > -if (allow_restart &&
> > +if (s->result != SERVICE_SUCCESS && s->failure_action != 
> > SERVICE_START_LIMIT_NONE)
> > +service_execute_action(s, s->failure_action,
> > "failed");
> 
> I'd prefer to move the check for SERVICE_START_LIMIT_NONE to the top of 
> service_execute_action().

Hmmm, the check is already there, but for the start limit it produces a
warning. I'll see if I can find a nice way to handle this.

> > +else if (allow_restart &&
> 
> I would drop the "else" here, I think. Is there a reason not to do the
> restart thing anyway? If it is configured, it should run I think, just
> in case the failure action doesn't work or so...

Are you sure? With Restart=always and FailureAction=reboot this would
try to start the unit while shutting down. Will the Conflicts= from the
default dependencies handle this correctly?

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] systemctl: delete REBOOT_PARAM_FILE if no parameter is specified

[Michael Olbrich]

On Tue, Mar 25, 2014 at 02:15:44PM +0100, Michael Olbrich wrote:
> And move it to sperate function.

Ping! No comments at all?

Michael

> ---
>  src/shared/util.c | 16 
>  src/shared/util.h |  2 ++
>  src/systemctl/systemctl.c |  9 +++--
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/src/shared/util.c b/src/shared/util.c
> index ffe6624..0260d17 100644
> --- a/src/shared/util.c
> +++ b/src/shared/util.c
> @@ -6391,3 +6391,19 @@ void hexdump(FILE *f, const void *p, size_t s) {
>  s -= 16;
>  }
>  }
> +
> +int update_reboot_param_file(const char *param)
> +{
> +int r = 0;
> +
> +if (param) {
> +
> +r = write_string_file(REBOOT_PARAM_FILE, param);
> +if (r < 0)
> +log_error("Failed to write reboot param to "
> +  REBOOT_PARAM_FILE": %s", strerror(-r));
> +} else
> +unlink(REBOOT_PARAM_FILE);
> +
> +return r;
> +}
> diff --git a/src/shared/util.h b/src/shared/util.h
> index 90464c9..a557f70 100644
> --- a/src/shared/util.h
> +++ b/src/shared/util.h
> @@ -922,3 +922,5 @@ uint64_t physical_memory(void);
>  char* mount_test_option(const char *haystack, const char *needle);
>  
>  void hexdump(FILE *f, const void *p, size_t s);
> +
> +int update_reboot_param_file(const char *param);
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index 0887bc3..89de20e 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -5973,13 +5973,10 @@ static int halt_parse_argv(int argc, char *argv[]) {
>  }
>  }
>  
> -if (arg_action == ACTION_REBOOT && argc == optind + 1) {
> -r = write_string_file(REBOOT_PARAM_FILE, argv[optind]);
> -if (r < 0) {
> -log_error("Failed to write reboot param to "
> -  REBOOT_PARAM_FILE": %s", strerror(-r));
> +if (arg_action == ACTION_REBOOT && (argc == optind || argc == optind 
> + 1)) {
> +r = update_reboot_param_file(argc == optind + 1 ? 
> argv[optind] : NULL);
> +if (r < 0)
>  return r;
> -}
>  } else if (optind < argc) {
>  log_error("Too many arguments.");
>  return -EINVAL;
> -- 
> 1.9.1
> 
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> 

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] systemd v212 build error

[Michael Olbrich]

Hi,

compiling systemd v212 fails here with:
[...]
src/libsystemd/sd-rtnl/rtnl-message.c: In function 'sd_rtnl_message_append_u8':
src/libsystemd/sd-rtnl/rtnl-message.c:462:38: error: 'IFLA_IPTUN_TTL' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:462:38: note: each undeclared identifier 
is reported only once for each function it appears in
src/libsystemd/sd-rtnl/rtnl-message.c:463:38: error: 'IFLA_IPTUN_TOS' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:464:38: error: 'IFLA_IPTUN_PROTO' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:465:38: error: 'IFLA_IPTUN_PMTUDISC' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:466:38: error: 'IFLA_IPTUN_ENCAP_LIMIT' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c: In function 'sd_rtnl_message_append_u16':
src/libsystemd/sd-rtnl/rtnl-message.c:508:45: error: 'IFLA_IPTUN_FLAGS' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:511:45: error: 'IFLA_IPTUN_6RD_PREFIXLEN' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:512:45: error: 
'IFLA_IPTUN_6RD_RELAY_PREFIXLEN' undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c: In function 'sd_rtnl_message_append_u32':
src/libsystemd/sd-rtnl/rtnl-message.c:561:38: error: 'IFLA_IPTUN_LOCAL' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:562:38: error: 'IFLA_IPTUN_REMOTE' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:564:38: error: 'IFLA_IPTUN_FLAGS' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:565:38: error: 'IFLA_IPTUN_FLOWINFO' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c:566:38: error: 'IFLA_GRE_FLOWINFO' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c: In function 
'sd_rtnl_message_append_in6_addr':
src/libsystemd/sd-rtnl/rtnl-message.c:687:38: error: 'IFLA_IPTUN_6RD_PREFIX' 
undeclared (first use in this function)
src/libsystemd/sd-rtnl/rtnl-message.c: In function 
'sd_rtnl_message_enter_container':
src/libsystemd/sd-rtnl/rtnl-message.c:976:79: error: 'IFLA_BRIDGE_MAX' 
undeclared (first use in this function)
make[3]: *** [src/libsystemd/sd-rtnl/libsystemd_internal_la-rtnl-message.lo] 
Error 1
make[3]: *** Waiting for unfinished jobs
make[2]: *** [all-recursive] Error 1
make[1]: *** [all] Error 2
[...]

>From what I can tell these where added in Linux 3.7, so the corresponding
headers are currently needed. Should these be added to missing.h? I think
we can use IFLA_IPTUN_MAX/IFLA_BRIDGE_MAX if the enums are needed.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC PATCH] service: add FailureAction= option

[Michael Olbrich]

On Wed, Mar 26, 2014 at 10:19:53AM +, "Jóhann B. Guðmundsson" wrote:
> On 03/26/2014 09:02 AM, Michael Olbrich wrote:
> >It has the same possible values as StartLimitAction= and is executed
> >immediately if a service fails.
> >---
> >
> >Hi Lennart,
> >
> >Something like this maybe? I'm not quite sure about the condition in
> >service_enter_dead(). I don't think the action should be executed when the
> >service is explicitly stopped. Maybe it should depend on !forbid_restart?
> >
> >If you like, I'll add some documentation. An maybe a follow-up patch to
> >rename the StartLimitAction type? To what though?
> 
> Dont we already have OnFailure= for this ( or can be used for this
> instead of introducing FailureAction= which can be confusing to
> users )?

No. Take a look at the possible actions. This is intended to be used e.g.
in combination with watchdogs. A unit that may or may not start cannot
express 'reboot right now'.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [RFC PATCH] service: add FailureAction= option

[Michael Olbrich]

It has the same possible values as StartLimitAction= and is executed
immediately if a service fails.
---

Hi Lennart,

Something like this maybe? I'm not quite sure about the condition in
service_enter_dead(). I don't think the action should be executed when the
service is explicitly stopped. Maybe it should depend on !forbid_restart?

If you like, I'll add some documentation. An maybe a follow-up patch to
rename the StartLimitAction type? To what though?

Regards,
Michael

 src/core/dbus-service.c   |  1 +
 src/core/load-fragment-gperf.gperf.m4 |  1 +
 src/core/service.c| 31 ---
 src/core/service.h|  2 ++
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 0451790..1e710e3 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -50,6 +50,7 @@ const sd_bus_vtable bus_service_vtable[] = {
 SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, 
offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, 
offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitAction", "s", 
property_get_start_limit_action, offsetof(Service, start_limit_action), 
SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("FailureAction", "s", property_get_start_limit_action, 
offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, 
offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, 
offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RemainAfterExit", "b", bus_property_get_bool, 
offsetof(Service, remain_after_exit), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index dbb5d13..dc83247 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -180,6 +180,7 @@ Service.WatchdogSec, config_parse_sec,  
 0,
 Service.StartLimitInterval,  config_parse_sec,   0,
 offsetof(Service, start_limit.interval)
 Service.StartLimitBurst, config_parse_unsigned,  0,
 offsetof(Service, start_limit.burst)
 Service.StartLimitAction,config_parse_start_limit_action,0,
 offsetof(Service, start_limit_action)
+Service.FailureAction,   config_parse_start_limit_action,0,
 offsetof(Service, failure_action)
 Service.Type,config_parse_service_type,  0,
 offsetof(Service, type)
 Service.Restart, config_parse_service_restart,   0,
 offsetof(Service, restart)
 Service.PermissionsStartOnly,config_parse_bool,  0,
 offsetof(Service, permissions_start_only)
diff --git a/src/core/service.c b/src/core/service.c
index ae3695a..ab161a5 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1835,6 +1835,8 @@ static int cgroup_good(Service *s) {
 return !r;
 }
 
+static int service_execute_action(Service *s, StartLimitAction action, const 
char *reason);
+
 static void service_enter_dead(Service *s, ServiceResult f, bool 
allow_restart) {
 int r;
 assert(s);
@@ -1844,7 +1846,9 @@ static void service_enter_dead(Service *s, ServiceResult 
f, bool allow_restart)
 
 service_set_state(s, s->result != SERVICE_SUCCESS ? SERVICE_FAILED : 
SERVICE_DEAD);
 
-if (allow_restart &&
+if (s->result != SERVICE_SUCCESS && s->failure_action != 
SERVICE_START_LIMIT_NONE)
+service_execute_action(s, s->failure_action, "failed");
+else if (allow_restart &&
 !s->forbid_restart &&
 (s->restart == SERVICE_RESTART_ALWAYS ||
  (s->restart == SERVICE_RESTART_ON_SUCCESS && s->result == 
SERVICE_SUCCESS) ||
@@ -2366,18 +2370,14 @@ fail:
 service_enter_stop(s, SERVICE_FAILURE_RESOURCES);
 }
 
-static int service_start_limit_test(Service *s) {
+static int service_execute_action(Service *s, StartLimitAction action, const 
char *reason) {
 assert(s);
 
-if (ratelimit_test(&s->start_limit))
-return 0;
-
-switch (s->start_limit_action) {
+switch (action) {
 
 case SERVICE_START_LIMIT_NONE:
 log_warning_unit(UNIT(s)->id,
- "%s start request repeated too quickly, 
refusing to start.",
- UNIT(s)->id);
+ "%s %s, re

Re: [systemd-devel] [RFC PATCH] service: add Restart option to execute StartLimitAction immediately

[Michael Olbrich]

Hi,

On Mon, Mar 24, 2014 at 08:30:09PM +0100, Lennart Poettering wrote:
> On Thu, 20.03.14 12:52, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> > The idea is to reboot immediately when a service crashes or the watchdog
> > triggers. This is useful in embedded scenarios when there is only one
> > important service. There are use-cases where rebooting immediately instead
> > of trying to restart the application first makes sense.
> > The environment of the restarted application is not well defined. The
> > watchdog is for unexpected failures. So making sure that the application
> > behaves correctly can be difficult. When rebooting only takes a few
> > seconds, doing so may be more robust than trying to recover from an
> > undefined state.
> > 
> > This is an RFC for now. Mostly because I think the configuration is rather
> > awkward like this.
> > Hooking into Restart/StartLimitAction was the easiest way to handle this in
> > the code. But it doesn't feel natural to configure it like this. Any Ideas
> > on how to  express this in the unit file?
> 
> Sounds useful, but I think it would be better to generalize the "action"
> concept and then expose FailureAction= in addition to
> StartLimitAction=. 

So basically in service_enter_dead():

if (s->result != SERVICE_SUCCESS)
execute_failure_action(...);

Right?

Hmmm, that would mean, I need to make sure that my application can never
stop successfully on its own. That's not possible right now, right? I can
redefine failures as success with SuccessExitStatus= but not the other way
around.

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] service: add support for reboot argument when triggered by StartLimitAction=

[Michael Olbrich]

When rebooting with systemctl, an optional argument can be passed to the
reboot system call. This makes it possible the specify the argument in a
service file and use it when the service triggers a restart.
This is useful to distinguish between manual reboots and reboots caused by
failing services.
---
 man/systemd.service.xml   | 13 +
 src/core/dbus-service.c   |  1 +
 src/core/load-fragment-gperf.gperf.m4 |  1 +
 src/core/service.c| 16 
 src/core/service.h|  1 +
 5 files changed, 32 insertions(+)

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 5b3afb8..a2a1b6b 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -1017,6 +1017,19 @@ ExecStart=/bin/echo $ONE $TWO ${TWO}
 none.
 
 
+
+RebootArgument=
+Configure the optional
+argument for the
+
reboot2
+system call if
+StartLimitAction=
+is a reboot action. This works just
+like the optional argument to
+systemctl reboot
+command.
+
+
 
 
 Check
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 0451790..45bfecf 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -50,6 +50,7 @@ const sd_bus_vtable bus_service_vtable[] = {
 SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, 
offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, 
offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitAction", "s", 
property_get_start_limit_action, offsetof(Service, start_limit_action), 
SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Service, 
reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, 
offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, 
offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RemainAfterExit", "b", bus_property_get_bool, 
offsetof(Service, remain_after_exit), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index dbb5d13..cb98c83 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -180,6 +180,7 @@ Service.WatchdogSec, config_parse_sec,  
 0,
 Service.StartLimitInterval,  config_parse_sec,   0,
 offsetof(Service, start_limit.interval)
 Service.StartLimitBurst, config_parse_unsigned,  0,
 offsetof(Service, start_limit.burst)
 Service.StartLimitAction,config_parse_start_limit_action,0,
 offsetof(Service, start_limit_action)
+Service.RebootArgument,  config_parse_string,0,
 offsetof(Service, reboot_arg)
 Service.Type,config_parse_service_type,  0,
 offsetof(Service, type)
 Service.Restart, config_parse_service_restart,   0,
 offsetof(Service, restart)
 Service.PermissionsStartOnly,config_parse_bool,  0,
 offsetof(Service, permissions_start_only)
diff --git a/src/core/service.c b/src/core/service.c
index ae3695a..4ebce6a 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -24,6 +24,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "manager.h"
 #include "unit.h"
@@ -300,6 +302,9 @@ static void service_done(Unit *u) {
 free(s->status_text);
 s->status_text = NULL;
 
+free(s->reboot_arg);
+s->reboot_arg = NULL;
+
 s->exec_runtime = exec_runtime_unref(s->exec_runtime);
 exec_command_free_array(s->exec_command, _SERVICE_EXEC_COMMAND_MAX);
 s->control_command = NULL;
@@ -2372,6 +2377,10 @@ static int service_start_limit_test(Service *s) {
 if (ratelimit_test(&s->start_limit))
 return 0;
 
+if (s->start_limit_action == SERVICE_START_LIMIT_REBOOT ||
+s->start_limit_action == SERVICE_START_LIMIT_REBOOT_FORCE)
+update_reboot_param_file(s->reboot_arg);
+
 switch (s->start_limit_action) {
 
 case 

[systemd-devel] [PATCH 1/2] systemctl: delete REBOOT_PARAM_FILE if no parameter is specified

[Michael Olbrich]

And move it to sperate function.
---
 src/shared/util.c | 16 
 src/shared/util.h |  2 ++
 src/systemctl/systemctl.c |  9 +++--
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index ffe6624..0260d17 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -6391,3 +6391,19 @@ void hexdump(FILE *f, const void *p, size_t s) {
 s -= 16;
 }
 }
+
+int update_reboot_param_file(const char *param)
+{
+int r = 0;
+
+if (param) {
+
+r = write_string_file(REBOOT_PARAM_FILE, param);
+if (r < 0)
+log_error("Failed to write reboot param to "
+  REBOOT_PARAM_FILE": %s", strerror(-r));
+} else
+unlink(REBOOT_PARAM_FILE);
+
+return r;
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index 90464c9..a557f70 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -922,3 +922,5 @@ uint64_t physical_memory(void);
 char* mount_test_option(const char *haystack, const char *needle);
 
 void hexdump(FILE *f, const void *p, size_t s);
+
+int update_reboot_param_file(const char *param);
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 0887bc3..89de20e 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -5973,13 +5973,10 @@ static int halt_parse_argv(int argc, char *argv[]) {
 }
 }
 
-if (arg_action == ACTION_REBOOT && argc == optind + 1) {
-r = write_string_file(REBOOT_PARAM_FILE, argv[optind]);
-if (r < 0) {
-log_error("Failed to write reboot param to "
-  REBOOT_PARAM_FILE": %s", strerror(-r));
+if (arg_action == ACTION_REBOOT && (argc == optind || argc == optind + 
1)) {
+r = update_reboot_param_file(argc == optind + 1 ? argv[optind] 
: NULL);
+if (r < 0)
 return r;
-}
 } else if (optind < argc) {
 log_error("Too many arguments.");
 return -EINVAL;
-- 
1.9.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] service: add support for reboot argument when triggered by StartLimitAction=

[Michael Olbrich]

When rebooting with systemctl, an optional argument can be passed to the
reboot system call. This makes it possible the specify the argument in a
service file and use it when the service triggers a restart.
This is useful to distinguish between manual reboots and reboots caused by
failing services.
---
 man/systemd.service.xml   | 13 +
 src/core/dbus-service.c   |  1 +
 src/core/load-fragment-gperf.gperf.m4 |  1 +
 src/core/service.c| 23 ++-
 src/core/service.h|  1 +
 5 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 5b3afb8..a2a1b6b 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -1017,6 +1017,19 @@ ExecStart=/bin/echo $ONE $TWO ${TWO}
 none.
 
 
+
+RebootArgument=
+Configure the optional
+argument for the
+
reboot2
+system call if
+StartLimitAction=
+is a reboot action. This works just
+like the optional argument to
+systemctl reboot
+command.
+
+
 
 
 Check
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 0451790..45bfecf 100644
--- a/src/core/dbus-service.c
+++ b/src/core/dbus-service.c
@@ -50,6 +50,7 @@ const sd_bus_vtable bus_service_vtable[] = {
 SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, 
offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, 
offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("StartLimitAction", "s", 
property_get_start_limit_action, offsetof(Service, start_limit_action), 
SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Service, 
reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, 
offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, 
offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY("RemainAfterExit", "b", bus_property_get_bool, 
offsetof(Service, remain_after_exit), SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index 55fd3da..defd66e 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -183,6 +183,7 @@ Service.WatchdogSec, config_parse_sec,  
 0,
 Service.StartLimitInterval,  config_parse_sec,   0,
 offsetof(Service, start_limit.interval)
 Service.StartLimitBurst, config_parse_unsigned,  0,
 offsetof(Service, start_limit.burst)
 Service.StartLimitAction,config_parse_start_limit_action,0,
 offsetof(Service, start_limit_action)
+Service.RebootArgument,  config_parse_string,0,
 offsetof(Service, reboot_arg)
 Service.Type,config_parse_service_type,  0,
 offsetof(Service, type)
 Service.Restart, config_parse_service_restart,   0,
 offsetof(Service, restart)
 Service.PermissionsStartOnly,config_parse_bool,  0,
 offsetof(Service, permissions_start_only)
diff --git a/src/core/service.c b/src/core/service.c
index 78a2e06..9026dc5 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -24,6 +24,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "manager.h"
 #include "unit.h"
@@ -2369,6 +2371,18 @@ static int service_start_limit_test(Service *s) {
 if (ratelimit_test(&s->start_limit))
 return 0;
 
+if ((s->start_limit_action == SERVICE_START_LIMIT_REBOOT ||
+ s->start_limit_action == SERVICE_START_LIMIT_REBOOT_FORCE) && 
s->reboot_arg) {
+int r;
+
+r = write_string_file(REBOOT_PARAM_FILE, s->reboot_arg);
+if (r < 0) {
+log_error("Failed to write reboot param to "
+  REBOOT_PARAM_FILE": %s", strerror(-r));
+return r;
+}
+}
+
 switch (s->start_limit_action) {
 
 case SERVICE_START_LIMIT_NONE:
@@ -2404,

[systemd-devel] [RFC PATCH] service: add Restart option to execute StartLimitAction immediately

[Michael Olbrich]

---

Hi,

The idea is to reboot immediately when a service crashes or the watchdog
triggers. This is useful in embedded scenarios when there is only one
important service. There are use-cases where rebooting immediately instead
of trying to restart the application first makes sense.
The environment of the restarted application is not well defined. The
watchdog is for unexpected failures. So making sure that the application
behaves correctly can be difficult. When rebooting only takes a few
seconds, doing so may be more robust than trying to recover from an
undefined state.

This is an RFC for now. Mostly because I think the configuration is rather
awkward like this.
Hooking into Restart/StartLimitAction was the easiest way to handle this in
the code. But it doesn't feel natural to configure it like this. Any Ideas
on how to  express this in the unit file?

Regards,
Michael

 src/core/service.c | 23 ++-
 src/core/service.h |  1 +
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/core/service.c b/src/core/service.c
index 78a2e06..ef494ba 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1832,6 +1832,8 @@ static int cgroup_good(Service *s) {
 return !r;
 }
 
+static int service_start_limit_execute(Service *s);
+
 static void service_enter_dead(Service *s, ServiceResult f, bool 
allow_restart) {
 int r;
 assert(s);
@@ -1843,6 +1845,10 @@ static void service_enter_dead(Service *s, ServiceResult 
f, bool allow_restart)
 
 if (allow_restart &&
 !s->forbid_restart &&
+s->restart == SERVICE_RESTART_FAIL) {
+service_start_limit_execute(s);
+} else if (allow_restart &&
+!s->forbid_restart &&
 (s->restart == SERVICE_RESTART_ALWAYS ||
  (s->restart == SERVICE_RESTART_ON_SUCCESS && s->result == 
SERVICE_SUCCESS) ||
  (s->restart == SERVICE_RESTART_ON_FAILURE && s->result != 
SERVICE_SUCCESS) ||
@@ -2363,12 +2369,9 @@ fail:
 service_enter_stop(s, SERVICE_FAILURE_RESOURCES);
 }
 
-static int service_start_limit_test(Service *s) {
+static int service_start_limit_execute(Service *s) {
 assert(s);
 
-if (ratelimit_test(&s->start_limit))
-return 0;
-
 switch (s->start_limit_action) {
 
 case SERVICE_START_LIMIT_NONE:
@@ -2416,6 +2419,15 @@ static int service_start_limit_test(Service *s) {
 return -ECANCELED;
 }
 
+static int service_start_limit_test(Service *s) {
+assert(s);
+
+if (ratelimit_test(&s->start_limit))
+return 0;
+
+return service_start_limit_execute(s);
+}
+
 static int service_start(Unit *u) {
 Service *s = SERVICE(u);
 int r;
@@ -3754,7 +3766,8 @@ static const char* const 
service_restart_table[_SERVICE_RESTART_MAX] = {
 [SERVICE_RESTART_ON_FAILURE] = "on-failure",
 [SERVICE_RESTART_ON_WATCHDOG] = "on-watchdog",
 [SERVICE_RESTART_ON_ABORT] = "on-abort",
-[SERVICE_RESTART_ALWAYS] = "always"
+[SERVICE_RESTART_ALWAYS] = "always",
+[SERVICE_RESTART_FAIL] = "fail"
 };
 
 DEFINE_STRING_TABLE_LOOKUP(service_restart, ServiceRestart);
diff --git a/src/core/service.h b/src/core/service.h
index 1992926..2cdc7ad 100644
--- a/src/core/service.h
+++ b/src/core/service.h
@@ -56,6 +56,7 @@ typedef enum ServiceRestart {
 SERVICE_RESTART_ON_WATCHDOG,
 SERVICE_RESTART_ON_ABORT,
 SERVICE_RESTART_ALWAYS,
+SERVICE_RESTART_FAIL,
 _SERVICE_RESTART_MAX,
 _SERVICE_RESTART_INVALID = -1
 } ServiceRestart;
-- 
1.9.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] networkd: fix typo

[Michael Olbrich]

It's HAVE_SPLIT_USR not HAVE_SPLIT_USER
---
 src/network/networkd-manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index c730e71..ea414b1 100644
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -33,7 +33,7 @@ const char* const network_dirs[] = {
 "/etc/systemd/network",
 "/run/systemd/network",
 "/usr/lib/systemd/network",
-#ifdef HAVE_SPLIT_USER
+#ifdef HAVE_SPLIT_USR
 "/lib/systemd/network",
 #endif
 NULL};
-- 
1.9.0

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [patch] Fix AC_PATH_PROG usage in configure.ac for systems with (still) bin vs. sbin distiction

[Michael Olbrich]

On Mon, Mar 10, 2014 at 02:13:38PM +0200, Samuli Suominen wrote:
> On 10/03/14 13:23, Michael Olbrich wrote:
> > On Sun, Mar 09, 2014 at 08:49:58PM +0100, Michael Biebl wrote:
> >> 2014-03-08 8:52 GMT+01:00 Samuli Suominen :
> >>> If eg. setcap is in /sbin and user is building as a normal user without
> >>> $PATH having /sbin, the build system
> >>> will default to /usr/sbin/setcap as it's defined in AC_PATH_PROG and
> >>> fail during the build with 'setcap: command not found'
> >>>
> >>> For example, my $PATH as normal user:
> >>>
> >>> $ echo $PATH
> >>> /usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/4.8.2
> >>>
> >>> I see Debian and Ubuntu carries a patch that changes these hardcoded
> >>> paths to what they have, but that's equally
> >>> unwise.
> >> We do patch those defaults so we don't have to actually build-depend
> >> on all those packages.
> >> Your patch doesn't really help with that.
> > You don't need a patch for that. Just set the corresponding configure cache
> > value:
> > export ac_cv_path_QUOTAON=/sbin/quotaon
> > [...]
> > ./configure ...
> >
> >
> 
> I'm aware of the possibility for exporting ac_cv_ values (those that are
> present in eg. config.log after ./configure)
> 
> But the problem for what the patch was submitted remains, when setcap
> is in /sbin/setcap instead of /usr/sbin/setcap, and the build system lacks
> the capability of checking sbin directories, it will set it to wrong value,
> and then try to *use it* during the build:
> Makefile.am:-$(SETCAP) cap_dac_override,cap_sys_ptrace=ep
> $(DESTDIR)$(bindir)/systemd-detect-virt
> resulting in a 'command not found' message, however because it's
> prefixed with -
> it's non-fatal, but that's how you miss it to begin with.
> 
> Why not search sbin directories, if the patch is this easy?

I was talking about the Debian patch, not your patch.

Regards.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [patch] Fix AC_PATH_PROG usage in configure.ac for systems with (still) bin vs. sbin distiction

[Michael Olbrich]

On Sun, Mar 09, 2014 at 08:49:58PM +0100, Michael Biebl wrote:
> 2014-03-08 8:52 GMT+01:00 Samuli Suominen :
> > If eg. setcap is in /sbin and user is building as a normal user without
> > $PATH having /sbin, the build system
> > will default to /usr/sbin/setcap as it's defined in AC_PATH_PROG and
> > fail during the build with 'setcap: command not found'
> >
> > For example, my $PATH as normal user:
> >
> > $ echo $PATH
> > /usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/4.8.2
> >
> > I see Debian and Ubuntu carries a patch that changes these hardcoded
> > paths to what they have, but that's equally
> > unwise.
> 
> We do patch those defaults so we don't have to actually build-depend
> on all those packages.
> Your patch doesn't really help with that.

You don't need a patch for that. Just set the corresponding configure cache
value:
export ac_cv_path_QUOTAON=/sbin/quotaon
[...]
./configure ...

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH RFC] namespace: make sure ReadWriteDirectories are actually writable

[Michael Olbrich]

On Fri, Dec 27, 2013 at 05:52:16PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> On Fri, Dec 27, 2013 at 10:18:30AM +0100, Michael Olbrich wrote:
> > Currently adding directories to ReadWriteDirectories= only restores the
> > original mount flags. So e.g. setting ReadOnlyDirectories=/usr and
> > ReadWriteDirectories=/usr/local works as expected if the underlying file
> > system was writable. However, setting ReadWriteDirectories= has no effect
> > if the underlying file system is mounted read-only.
> > This patch changes that by explicitly remounting the bind mount.
> This doesn't feel right. This means that running any service with 
> ReadWriteDirectories
> set would mean mounting partitions rw. That would be a significant change
> of semantics for current users. I think you should have an fstab entry
> with the rw flag. You can use noauto and it won't be mounted by default,
> and systemd should mount it automatically for you if you start the service.

That doesn't help. I need it for the rootfs. This is for embedded systems.
Most applications are not allowed to write to the rootfs. This can only be
ensured if the rootfs is mounted read-only.
If changing the semantics is not acceptable how about a new option like
ForceReadWriteDirectories= or something like that?

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH RFC] namespace: make sure ReadWriteDirectories are actually writable

[Michael Olbrich]

Currently adding directories to ReadWriteDirectories= only restores the
original mount flags. So e.g. setting ReadOnlyDirectories=/usr and
ReadWriteDirectories=/usr/local works as expected if the underlying file
system was writable. However, setting ReadWriteDirectories= has no effect
if the underlying file system is mounted read-only.
This patch changes that by explicitly remounting the bind mount.
---

Hi,

I'm no expert when it comes to mount flags or namespaces. In my use case
the rootfs is mounted read-only. I'd like to make it writeable for some
selected services. I can do this manually but it would be nicer to let
systemd do it for me. This works for me. I have no idea if there are better
ways to do this.

Regards,
Michael

 src/core/namespace.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/core/namespace.c b/src/core/namespace.c
index 85147be..cc3ae51 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -188,6 +188,21 @@ static int make_read_only(BindMount *m) {
 return 0;
 }
 
+static int make_read_write(BindMount *m) {
+int r;
+
+assert(m);
+
+if (m->mode != READWRITE)
+return 0;
+
+r = mount(NULL, m->path, NULL, MS_REMOUNT|MS_REC, NULL);
+if (r < 0 && !(m->ignore && errno == ENOENT))
+return -errno;
+
+return 0;
+}
+
 int setup_namespace(
 char** read_write_dirs,
 char** read_only_dirs,
@@ -258,6 +273,9 @@ int setup_namespace(
 r = make_read_only(m);
 if (r < 0)
 goto fail;
+r = make_read_write(m);
+if (r < 0)
+goto fail;
 }
 
 /* Remount / as the desired mode */
-- 
1.8.5.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] service: don't try to kill the service more than once when the watchdog timeout hits

[Michael Olbrich]

Hi,

On Wed, Jul 17, 2013 at 03:53:09AM +0200, Lennart Poettering wrote:
> On Wed, 12.06.13 01:22, Michael Olbrich (m.olbr...@pengutronix.de) wrote:
> 
> > If ExecStopPost= is defined then it is executed after SIGKILL. Otherwise
> > another round of SIGTERM/SIGSTOP is started which is rather useless when
> > the watchdog timeout hits.
> > So go directly to the final SIGKILL if ExecStopPost= is not defined.
> 
> Hmm, why not go always directly into SERVICE_FINAL_SIGKILL? Why bother
> with SERVICE_STOP_SIGKILL at all? What am I missing?

I think a small summery is necessary here:
The original watchdog code just called service_enter_dead(). This had the
problem that no cleanup happened. The process was not killed.
Then I posted a patch to go irectly into SERVICE_FINAL_SIGKILL to fix that.
It got applied.
Then you changed it to SERVICE_STOP_SIGKILL so that ExecStopPost= is still
called.
However, this has a annoying side effect. Consider a service that enters
service_handle_watchdog() with a process that cannot be killed. What
happens is this:

service_handle_watchdog()
STOP_SIGKILL
wait
ExecStopPost= if available
FINAL_SIGTERM
wait
FINAL_SIGKILL
wait
service_enter_dead()

Without a ExecStopPost= defined anything after the first wait is just more
useless waiting.
I wrote this patch to avoid waiting longer than necessary but still be able
to run ExecStopPost=.
I later realized that this problem is not limited to the watchdog usecase.
So I wrote a second patch (it should be applied instead of this one) that
changes the sequence to:

[...]
STOP_SIGKILL
wait
if (ExecStopPost= is available) {
ExecStopPost=
FINAL_SIGTERM
wait
FINAL_SIGKILL
wait
}
service_enter_dead()

I sent it as a reply to the first patch. I've also attached it for
reference.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
>From 86efc9efe5a365de219457d033844cb1022fdacb Mon Sep 17 00:00:00 2001
From: Michael Olbrich 
Date: Wed, 12 Jun 2013 08:41:16 +0200
Subject: [PATCH] service: don't enter a second SIGTERM/SIGKILL cycle if no
 ExecStopPost= process is defined

It won't help if the main process is still there and there is no new
process to kill.

Signed-off-by: Michael Olbrich 
---
 src/core/service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/service.c b/src/core/service.c
index 2bc0dc5..b98f11a 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1987,7 +1987,7 @@ static void service_enter_stop_post(Service *s, ServiceResult f) {
 
 service_set_state(s, SERVICE_STOP_POST);
 } else
-service_enter_signal(s, SERVICE_FINAL_SIGTERM, SERVICE_SUCCESS);
+service_enter_dead(s, SERVICE_SUCCESS, true);
 
 return;
 
-- 
1.8.3.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] service: don't enter a second SIGTERM/SIGKILL cycle if no ExecStopPost= process is defined

[Michael Olbrich]

On Wed, Jun 12, 2013 at 08:45:35AM +0200, Michael Olbrich wrote:
> It won't help if the main process is still there and there is no new
> process to kill.
> ---
> 
> Hi,
> 
> The second SIGTERM/SIGKILL is to kill ExecStopPost= if necessary, right? In
> that case, this is a better solution.

Can anyone comment on this? The current state is rather annoying.

Michael

>  src/core/service.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/core/service.c b/src/core/service.c
> index dadd981..1319696 100644
> --- a/src/core/service.c
> +++ b/src/core/service.c
> @@ -1968,7 +1968,7 @@ static void service_enter_stop_post(Service *s, 
> ServiceResult f) {
>  
>  service_set_state(s, SERVICE_STOP_POST);
>  } else
> -service_enter_signal(s, SERVICE_FINAL_SIGTERM, 
> SERVICE_SUCCESS);
> +service_enter_dead(s, SERVICE_SUCCESS, true);
>  
>  return;
>  
> -- 
> 1.8.2.rc2
> 
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> 

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] service: don't enter a second SIGTERM/SIGKILL cycle if no ExecStopPost= process is defined

[Michael Olbrich]

It won't help if the main process is still there and there is no new
process to kill.
---

Hi,

The second SIGTERM/SIGKILL is to kill ExecStopPost= if necessary, right? In
that case, this is a better solution.

Michael

 src/core/service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/service.c b/src/core/service.c
index dadd981..1319696 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1968,7 +1968,7 @@ static void service_enter_stop_post(Service *s, 
ServiceResult f) {
 
 service_set_state(s, SERVICE_STOP_POST);
 } else
-service_enter_signal(s, SERVICE_FINAL_SIGTERM, 
SERVICE_SUCCESS);
+service_enter_dead(s, SERVICE_SUCCESS, true);
 
 return;
 
-- 
1.8.2.rc2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] start/stop messages when activating a target?

[Michael Olbrich]

Hi,

On Wed, Jun 12, 2013 at 07:02:34AM +0400, Andrey Borzenkov wrote:
> В Wed, 12 Jun 2013 01:32:37 +0200
> Michael Olbrich  пишет:
> 
> > Hi,
> > 
> > When booting or during shutdown systemd prints the start stop messages for
> > the services. Is it possible to get those messages when activating a target
> > with systemctl?
> > 
> 
> systemctl already prints status of job unless called with --no-block.
> Do you want additional status messages on console?

Not for me. "systemctl {start,stop} " never prints anything. It
just blocks until the unit is started/stoped.
Hmmm, I'm working on embedded systems here, and a lot of stuff is disabled.
But which configure option could cause something like this?

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] start/stop messages when activating a target?

[Michael Olbrich]

Hi,

When booting or during shutdown systemd prints the start stop messages for
the services. Is it possible to get those messages when activating a target
with systemctl?

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] service: don't try to kill the service more than once when the watchdog timeout hits

[Michael Olbrich]

If ExecStopPost= is defined then it is executed after SIGKILL. Otherwise
another round of SIGTERM/SIGSTOP is started which is rather useless when
the watchdog timeout hits.
So go directly to the final SIGKILL if ExecStopPost= is not defined.
---

Hi,

I did some more testing with this. I think this is better. Running
ExecStopPost= if available is a good idea, but don't waste any time
otherwise.

Michael

 src/core/service.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/core/service.c b/src/core/service.c
index dadd981..984993f 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -249,7 +249,10 @@ static void service_handle_watchdog(Service *s) {
 offset = now(CLOCK_MONOTONIC) - s->watchdog_timestamp.monotonic;
 if (offset >= s->watchdog_usec) {
 log_error_unit(UNIT(s)->id, "%s watchdog timeout!", 
UNIT(s)->id);
-service_enter_signal(s, SERVICE_STOP_SIGKILL, 
SERVICE_FAILURE_WATCHDOG);
+if (s->exec_command[SERVICE_EXEC_STOP_POST])
+service_enter_signal(s, SERVICE_STOP_SIGKILL, 
SERVICE_FAILURE_WATCHDOG);
+else
+service_enter_signal(s, SERVICE_FINAL_SIGKILL, 
SERVICE_FAILURE_WATCHDOG);
 return;
 }
 
-- 
1.8.2.rc2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Solution proposal for bug 56109

[Michael Olbrich]

Hi,

On Thu, May 16, 2013 at 06:23:15PM +0200, markoho...@freenet.de wrote:
> I recently stumbled over the bug with the watchdog mechanism that has already 
> been reported to free desktop bugzilla (56109).
>  
> I analyzed the bug and came to a simple solution for solving it.
>  
> First, what I think is going on:
> -    watchdog timeout is detected in service_handle_watchdog(), 
> service_enter_dead(…) is called
> -    service_enter_dead() sets the service state to auto_restart
> -    triggered by a timer, service_enter_restart is called
> -    service_enter_restart  schedules a restart job
> -    systemd splits up the jobs into a stop and a start job and schedules 
> both
> -    the stop job lasts to a call of service_stop()
> -    here it begins to get interesting:
> -    based on the AUTO_RESTART state, this function decides to go 
> directly into dead state, nothing of the normal stopping procedure is done. 
> This is probably because in most cases that cause a restart to be scheduled 
> the stop proceeding is done automatically (for instance in case of a killed 
> or normally exiting service.). But this is not true for a watchdog timeout. 
> Nothing of the stop proceeding is executed in case of such a timeout. So the 
> process that missed to send the watchdog event is going on to life (in which 
> state ever). No one is cleaning up. A second instance of the service is 
> started.

I'm quite sure this worked correctly at some point, when I first
implemented it. But I can reproduce the Problem here.

> My suggestion to solve this:
>  
> Changes are needed in service.c in service_stop(…).
>  
> change:
> /* A restart will be scheduled or is in progress. */
>     if (s->state == SERVICE_AUTO_RESTART) {
>     service_set_state(s, SERVICE_DEAD);
>     return 0;
>     }
>  
> to:
> /* A restart will be scheduled or is in progress. 
>    In all cases but the watchdog timeout, stop is already progressed 
> by systemd automatically*/
>     if (s->state == SERVICE_AUTO_RESTART && s->result != 
> SERVICE_FAILURE_WATCHDOG) {
>     service_set_state(s, SERVICE_DEAD);
>     return 0;
>     }
>  
> and change:
>  
> assert(s->state == SERVICE_RUNNING ||
>  s->state == SERVICE_EXITED);
>  
>  
> to:
> assert(s->state == SERVICE_RUNNING ||
>    s->state == SERVICE_AUTO_RESTART ||
>    s->state == SERVICE_EXITED);
>  
> I tested the following:
> -    the watchdog mechanism is now actually stopping / killing the 
> service in case it is not sending the watchdog event right in time
> -    a restart triggered by a killed service works like before
>  
> Hopefully, I didn’t miss some side effects caused by my changes.
>  
>  
> Any opinions on my proposed changes?

No, this is wrong. The change should happen in service_handle_watchdog().
Calling service_enter_dead() is wrong. Calling service_enter_stop() there
would probably create more or less what your change does.
But I think this is still wrong. When this code is executed then the
service is in an undefined state. Calling the normal shutdown mechanism can
imho actually be harmful. I think just killing the process here is correct.
I've sent a patch, that does this.

Btw, I've tested both versions with a worst-case failure (a process that
cannot be killed): With your code it waits 4x TimeoutStopSec before the
service is started again. With my patch it's 1x TimeoutStopSec.


Regards,
Michael


-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] service: kill processes with SIGKILL on watchdog failure

[Michael Olbrich]

Just calling service_enter_dead() does not kill any processes.
As a result, the old process may still be running when the new one is
started.
After a watchdog failure the service is in an undefined state.
Using the normal shutdown mechanism makes no sense. Instead all processes
are just killed and the service can try to restart.
---
 src/core/service.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/core/service.c b/src/core/service.c
index 3617c24..4b0d4ae 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -235,7 +235,7 @@ static void service_stop_watchdog(Service *s) {
 s->watchdog_timestamp.monotonic = 0;
 }
 
-static void service_enter_dead(Service *s, ServiceResult f, bool 
allow_restart);
+static void service_enter_signal(Service *s, ServiceState state, ServiceResult 
f);
 
 static void service_handle_watchdog(Service *s) {
 usec_t offset;
@@ -249,7 +249,7 @@ static void service_handle_watchdog(Service *s) {
 offset = now(CLOCK_MONOTONIC) - s->watchdog_timestamp.monotonic;
 if (offset >= s->watchdog_usec) {
 log_error_unit(UNIT(s)->id, "%s watchdog timeout!", 
UNIT(s)->id);
-service_enter_dead(s, SERVICE_FAILURE_WATCHDOG, true);
+service_enter_signal(s, SERVICE_FINAL_SIGKILL, 
SERVICE_FAILURE_WATCHDOG);
 return;
 }
 
-- 
1.8.2.rc2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] Is the format of hwdb.bin platform independent?

[Michael Olbrich]

Hi,

I'm using udev/systemd in embedded scenarios and I don't like generating
hwdb.bin on the target hardware. So I built a host udevadm to create it. It
is save to do that? Generating hwdb.bin on x86_64 and using it on ARM seems
to work, but what about endianness?

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] build problems without largefile support

[Michael Olbrich]

Hi,

I tried to compile systemd with "--disable-largefile" and I got this:
[...]
src/shared/util.c: In function 'parse_bytes':
src/shared/util.c:2270:17: error: overflow in implicit constant conversion 
[-Werror=overflow]
src/shared/util.c:2271:17: error: overflow in implicit constant conversion 
[-Werror=overflow]
src/shared/util.c:2272:17: error: overflow in implicit constant conversion 
[-Werror=overflow]
src/shared/util.c: In function 'format_bytes':
src/shared/util.c:4854:17: error: overflow in implicit constant conversion 
[-Werror=overflow]
src/shared/util.c:4855:17: error: overflow in implicit constant conversion 
[-Werror=overflow]
src/shared/util.c:4856:17: error: overflow in implicit constant conversion 
[-Werror=overflow]
[...]

The constants for T/P/E don't fit in a 23bit off_t. That's why largefile
was introduced in the first place.
I'm not sure how to fix this. Maybe just use 'ifdef' and not support those
formats?

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] ratelimit: fix off-by-one

[Michael Olbrich]

The current code might make sense during startup for service restart
limiting:
With burst=1 after starting num is 1, so the next ('first') restart is
still accepted ( 1 <= 1). However, once interval has expired, num is 1
after the first restart, so the second restart is also accepted.

This change is also useful in combination with watchdogs:
With burst=1 and a very large interval the system can be restarted on
the first failure.
---
 src/shared/ratelimit.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/ratelimit.c b/src/shared/ratelimit.c
index 1054d52..01b62b7 100644
--- a/src/shared/ratelimit.c
+++ b/src/shared/ratelimit.c
@@ -46,7 +46,7 @@ bool ratelimit_test(RateLimit *r) {
 goto good;
 }
 
-if (r->num <= r->burst)
+if (r->num < r->burst)
 goto good;
 
 return false;
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Suggestion on StartLimitAction=

[Michael Olbrich]

On Fri, Feb 01, 2013 at 03:22:49PM +0100, Allmeroth, Robert wrote:
> On embedded systems it is sometimes not very useful to call shutdown scripts 
> or reboot(2) in case of a watchdog failure / failed service (re)starting,
> e.g. FS/Flash corruption, FS driver failure, special HW init failure
> 
> - Typically important file systems are read-only on embedded systems so it 
> would
>   not harm the system to do a hard reset.
> - The processor on which systemd is running on is not necessarily the 
> PowerController, a call
>   to reboot(2) would only stop/reboot that node. But other nodes like FPGA, 
> ASIC, DSP also
>   need a reset.
> 
> Does it make sense to you to have a StartLimitAction= where the systemd stops 
> sending 
> heartbeats to the kernel? (e.g. StartLimitAction = StopKernelHeartbeat 
> )
> In this case the watchdog on the PowerController is able to react very fast 
> accordingly.
> 
> Of course this will only work if the kernel watchdog module is already 
> connected to the PowerController.
> If not - the only escape is the 'expect watchdog register timer' of the 
> PowerController. 
> In general we would like to avoid running into the expect timer because we 
> have no hint why the system 
> did not respond.

Have you looked at StartLimitAction=reboot-immediate? This should restart
your system immediately without stopping services etc.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] service: make sure the watchdog timer is not restarted while stopping

[Michael Olbrich]

A watchdog notification may be handled after the watchdog timer was stopped
while stopping the service. As a result the timer is restarted and the
service may be restarted as well.
The watchdog timestamp is initially set during startup in
service_enter_start_post() and cleared when the timer is stopped. Therefore
it can be used as an indication if the timer should be reset.
---
 src/core/service.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/core/service.c b/src/core/service.c
index 593946e..baddc5a 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -3400,7 +3400,8 @@ static void service_notify_message(Unit *u, pid_t pid, 
char **tags) {
 if (strv_find(tags, "WATCHDOG=1")) {
 log_debug_unit(u->id,
"%s: got WATCHDOG=1", u->id);
-service_reset_watchdog(s);
+if (dual_timestamp_is_set(&s->watchdog_timestamp))
+service_reset_watchdog(s);
 }
 
 /* Notify clients about changed status or main pid */
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] service: really stop watchdog timer when stopping

[Michael Olbrich]

For services without ExecStop= the state SERVICE_STOP is never entered. as
a result the watchdog timer is not stopped and the service is restarted (if
it is configuered to restart).
Stopping the watchdog timer for SERVICE_STOP_SIGTERM as well fixes this.
---
 src/core/service.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/service.c b/src/core/service.c
index ee5a1a4..593946e 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1550,7 +1550,7 @@ static void service_set_state(Service *s, ServiceState 
state) {
 service_connection_unref(s);
 }
 
-if (state == SERVICE_STOP)
+if (state == SERVICE_STOP || state == SERVICE_STOP_SIGTERM)
 service_stop_watchdog(s);
 
 /* For the inactive states unit_notify() will trim the cgroup,
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [HEADS-UP] systemd down/upstream meeting at FOSDEM?

[Michael Olbrich]

Hi,

On Tue, Jan 22, 2013 at 11:11:22PM +0100, Lennart Poettering wrote:
> I just learned that a number of systemd downstream and upstream folks
> are attending FOSDEM. We'll at least have Michael Biebl, Tollef Fog
> Heen, Colin Guthrie, Kay Sievers, Harald Hoyer and myself around.
> 
> We were wondering whether it might be a good idea to meet up for some
> kind of systemd BOF at FOSDEM? My current thinking goes towards maybe
> lunch on saturday, but details about time and location are to be
> determined.
> 
> Anybody else is coming who'd like to join us? Anybody from ArchLinux
> attending FOSDEM? SUSE? Or the other distributions?

I'd like to come, but I'm giving a talk from 12 to 1pm on Saturday, so it
depends on the exact time.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] make sure __NR_name_to_handle_at is correctly defined

[Michael Olbrich]

341 is only valid for x86, so don't use it for other architectures.
Add the correct numbers for ARM and PowerPC while at it.
---
 src/shared/missing.h |   14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/shared/missing.h b/src/shared/missing.h
index 1a31066..1965319 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -212,10 +212,22 @@ static inline pid_t gettid(void) {
 #  ifndef __NR_name_to_handle_at
 #define __NR_name_to_handle_at 303
 #  endif
-#else
+#elif __i386__
 #  ifndef __NR_name_to_handle_at
 #define __NR_name_to_handle_at 341
 #  endif
+#elif defined __arm__
+#  ifndef __NR_name_to_handle_at
+#define __NR_name_to_handle_at 370
+#  endif
+#elif defined __powerpc__
+#  ifndef __NR_name_to_handle_at
+#define __NR_name_to_handle_at 345
+#  endif
+#else
+#  ifndef __NR_name_to_handle_at
+#error __NR_name_to_handle_at is not defined
+#  endif
 #endif
 
 #if !HAVE_DECL_NAME_TO_HANDLE_AT
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] build error

[Michael Olbrich]

Hi,

The current systemd from git (v194-102-gd671da8) fails to build with:

make[2]: *** No rule to make target `man/sd_journal_query_unique.3', needed by 
`all-am'.  Stop.
make[2]: *** Waiting for unfinished jobs

I think man/sd_journal_query_unique.xml should have been added in
3c1668d journal: add ability to list values a specified field can take in all 
entries of the journal

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] journald leaking maps?

[Michael Olbrich]

On Fri, Oct 12, 2012 at 03:58:06PM +0100, Colin Guthrie wrote:
> 'Twas brillig, and Michael Olbrich at 12/10/12 09:52 did gyre and gimble:
> > On Fri, Oct 12, 2012 at 04:31:46AM -0400, Dave Reisner wrote:
> >> On Fri, Oct 12, 2012 at 09:19:08AM +0100, Colin Guthrie wrote:
> >>> 'Twas brillig, and David Strauss at 12/10/12 05:39 did gyre and gimble:
> >>>> On Thu, Oct 11, 2012 at 3:31 PM, Colin Guthrie  
> >>>> wrote:
> >>>>> Something is obviously not good there! journald is using something in
> >>>>> the region of 250MB res.
> >>>>>
> >>>>> What's the best way to debug this?
> >>>>
> >>>> What version are you on? The Fedora 17 journal does excessive mapping
> >>>> that's fixed in current versions.
> >>>
> >>> Arg, I forgot that detail - I'm on 194, no patches to journal related
> >>> stuff applied.
> >>>
> >>> As others are not seeing it jump out at them, I figured it might be due
> >>> to just using /run and not flushing it to disk etc.
> >>>
> > 
> > Well the mapped memory is the same physical memory, that is used in the
> > tmpfs. You'll probably run out of memory there first, before journald runs
> > out of virtual address space. And that will happen, even if journald does
> > not map it. Setting RuntimeMaxUse in journald.conf should help here.
> 
> Well perhaps, but with systemd 189 (I think, perhaps a bit earlier) I
> certainly didn't see this kind of memory usage. I'm certainly hitting
> OOMs much more regularly now unless I restart systemd occasionally.
> 
> Also, that explanation doesn't really explain:
> 
> 
> [root@jimmy ~]# cat /proc/$(pidof systemd-journald)/status | grep Vm
> VmPeak:666 kB
> VmSize:662 kB

I've tried to flood the journal with messages for a while now. VmPeak is
stable and VmSize grows until it reaches VmPeak and the drops
significantly.
This looks like some limit is reached and then journald releases most of
its memory.

> VmLck:   0 kB
> VmPin:   0 kB
> VmHWM:  253692 kB
> VmRSS:  227680 kB
> VmData:960 kB
> VmStk: 136 kB
> VmExe: 164 kB
> VmLib:2804 kB
> VmPTE:   12876 kB
> VmSwap:172 kB
> 
> [root@jimmy ~]# cat /proc/$(pidof systemd-journald)/maps | grep -c /run/log/
> 1939

This goes up and down with VmSize for me.

> [root@jimmy ~]# du -sh /run/log/
> 10M   /run/log/
> 
> 
> So, I've got about 2k mmaps, using about 230Mb RSS and yet my journal
> data is only 10M in tmpfs... doesn't sound right to me!
> 
> Unless all this RSS memory is just fake and pretend and just different
> maps onto that 10MB and it's really "free", but my understanding was
> that it was the VmSize that was all the virtual mmap stuff, not the
> VmRSS... please correct me if I'm wrong.

VmSize is, as far as I know the currently used virtual memory. This does
not mean that there is any real memory associated with it. VmRSS is the
total actual memory that is mapped into the address space of the
application.
Usually mapping a lot of files is no problem. When the memory is needed
then the kernel just frees some pages. After all, it can just read the
files from disk again when needed.
On a tmpfs however thats not possible, there is no persistent storage
backing the files.

What I think happens for you is the following:
For some reason, journald is not releasing old mappings. It still rotates
files in /run/log/journal/, so "du -sh /run/log/" seems correct. However,
the old mappings mean, that even though the file is deleted, its content is
still there. So the mappings are for files in /run/log/journal/, they just
don't show up because there is no directory entry for them any more.

I guess the algorithm that decides when to release old mappings is broken
for you.

Regards,
Michael


-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel