[arch-projects] [netcfg] [PATCH 3/3] systemd: add alternative units with proper dependencies

[Dave Reisner]

On Thu, Oct 25, 2012 at 12:27:41AM +0200, Thomas Bächler wrote:
> Until now, netcfg might fail to start because it is started too early (this actually
> happened to me quite regularly). To fix this, we need to make systemd aware of the
> dependencies of a profile.
> 
> This commit adds a systemd generator that creates profiles with the names
>    netcfg-profile-$PROFILE.service
> on boot. The ugly part: You cannot permanently enable such a profile, as
> it is generated during boot. To enable it, put it in NETWORKS=() in
> /etc/conf.d/netcfg and run
>    systemctl enable netcfg-profiles.target
> If you add a new profile, you need to reload the systemd configuration
> so the service file shows up.
> 
> While this scheme is less intuitive than netcfg at profilename.service, it
> is the only way to provide proper dependencies during boot.
> 
> Due to a bug in systemd's dependency resolution with devices, this patch
> requires systemd v195 to work.
> ---
>  Makefile                       |  2 ++
>  systemd/netcfg-generator       | 58 ++++++++++++++++++++++++++++++++++++++++++
>  systemd/netcfg-profiles.target |  5 ++++
>  3 files changed, 65 insertions(+)
>  create mode 100755 systemd/netcfg-generator
>  create mode 100644 systemd/netcfg-profiles.target
> 
> diff --git a/Makefile b/Makefile
> index bbada3f..3a80c41 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -37,7 +37,9 @@ install: install-docs
>  	install -d $(DESTDIR)/usr/lib/systemd/system
>  	install -m644 \
>  	    systemd/*.service \
> +	    systemd/*.target \
>  	    $(DESTDIR)/usr/lib/systemd/system/
> +	install -Dm755 systemd/netcfg-generator /usr/lib/systemd/system-generators/netcfg-generator
>  
>  install-docs: docs
>  	install -d $(DESTDIR)/usr/share/man/{man5,man8}
> diff --git a/systemd/netcfg-generator b/systemd/netcfg-generator
> new file mode 100755
> index 0000000..983dceb
> --- /dev/null
> +++ b/systemd/netcfg-generator
> @@ -0,0 +1,58 @@
> +#!/bin/bash
> +
> +[[ -d $1 ]] || exit 1
> +
> +. /usr/lib/network/network
> +
> +profile_tmpl_unit="[Unit]
> +Description=Netcfg networking service for profile @PROFILE@
> +SourcePath=/etc/network.d/@PROFILE@
> +Before=network.target
> +Wants=network.target"
> +
> +profile_tmpl_service="[Service]
> +Type=oneshot
> +RemainAfterExit=yes
> +ExecStart=/usr/bin/netcfg check-iface @PROFILE@
> +ExecStop=-/usr/bin/netcfg down @PROFILE@
> +KillMode=none"
> +
> +for pf in /etc/network.d/*; do
> +    [[ -f ${pf} && -r ${pf} ]] || continue
> +    p=${pf#/etc/network.d/}

I think you just want to use p=${pf##*/}.

> +    load_profile ${p}
> +    systemd_devices=
> +    case ${CONNECTION} in
> +        bond)
> +            for iface in ${SLAVE_INTERFACES[@]}; do
> +                systemd_devices="${systemd_devices}sys-subsystem-net-devices-${iface}.device "

Please use arrays for this...

systemd_devices+=(....)

> +            done
> +            ;;
> +        bridge)
> +            for iface in ${BRIDGE_INTERFACES[@]}; do
> +                systemd_devices="${systemd_devices}sys-subsystem-net-devices-${iface}.device "
> +            done
> +            ;;
> +        ethernet|pppoe|wireless)
> +            systemd_devices="sys-subsystem-net-devices-${INTERFACE}.device"
> +            ;;
> +        vlan)
> +            systemd_devices="sys-subsystem-net-devices-${VLAN_PHYS_DEV}.device"
> +            ;;
> +    esac
> +    systemd_devices=${systemd_devices% }
> +    if [[ ${systemd_devices} ]]; then
> +        bindsto="BindsTo=${systemd_devices}"
> +        after="After=${systemd_devices}"

Array here too...

extra_deps+=("BindsTo=${systemd_devices[*]}" "After=${systemd_devices[*]}")


> +    else
> +        bindsto=
> +        after=
> +    fi
> +    echo -e "${profile_tmpl_unit//@PROFILE@/$p}\n${bindsto}\n${after}\n\n${profile_tmpl_service//@PROFILE@/$p}" > $1/netcfg-profile-$p.service

This is dirty (and pretty much unreadable). I suggest using a function
along the lines of:

  write_template() {
    local profile=$1; shift
    cat <<EOF
  [Unit]
  Description=Netcfg profile for $profile
  SourcePath=/etc/network.d/$profile
  Before=network.target
  Wants=network.target
  EOF

    (( $# )) && printf '%s\n' "$@" ''

    cat <<EOF
  [Service]
  Type=oneshot
  RemainAfterExit=yes
  ExecStart=/usr/bin/netcfg check-iface $profile
  ExecStop=-/usr/bin/netcfg down $profile
  KillMode=none
  EOF

  }

And then you can just do something like:

  write_template "$1" "${extra_deps[@]}" >"$1/netcfg-profile-$p.service"

> +done
> +
> +. /etc/conf.d/netcfg
> +mkdir $1/netcfg-profiles.target.wants
> +for network in "${NETWORKS[@]}"; do
> +    ln -s $1/netcfg-profile-$p.service $1/netcfg-profiles.target.wants/netcfg-profile-$p.service
> +done
> diff --git a/systemd/netcfg-profiles.target b/systemd/netcfg-profiles.target
> new file mode 100644
> index 0000000..ba310fa
> --- /dev/null
> +++ b/systemd/netcfg-profiles.target
> @@ -0,0 +1,5 @@
> +[Unit]
> +Description=Netcfg network profiles
> +
> +[Install]
> +WantedBy=multi-user.target
> -- 
> 1.8.0
>