On Tue, Apr 08, 2014 at 01:23:33PM -0400, Vivek Goyal wrote:
On Wed, Apr 09, 2014 at 12:27:29AM +0800, WANG Chao wrote:
> kdump-dep-generator is a systemd generator, used to write out kdump
> service dependencies.
>
> Currently it's only useful for ssh dump case. And in ssh dump case, it
> writes out a dependency which kdump.service "Wants"
> network-online.target:
>
> # ls -l /run/systemd/generator/kdump.service.wants/
> [..] network-online.target -> /usr/lib/systemd/system/network-online.target
>
> So that kdump.service will pull in network-online.target and delayed
> start until network-online.target is reached.
>
> In the future, we could use generator to write out kdump.service
> dynamically and get rid of the static defined kdump.service at all.
>
> v1->v2:
> Vivek: not using hardcoded run time generator path, use what systemd pass in.
>
> Signed-off-by: WANG Chao <chaowang(a)redhat.com>
> ---
> kdump-dep-generator.sh | 22 ++++++++++++++++++++++
> kexec-tools.spec | 3 +++
> 2 files changed, 25 insertions(+)
> create mode 100644 kdump-dep-generator.sh
>
> diff --git a/kdump-dep-generator.sh b/kdump-dep-generator.sh
> new file mode 100644
> index 0000000..b6fab2d
> --- /dev/null
> +++ b/kdump-dep-generator.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +# More details about systemd generator:
> +#
http://www.freedesktop.org/wiki/Software/systemd/Generators/
> +
> +. /usr/lib/kdump/kdump-lib.sh
> +
> +# If invokded with no arguments for testing purpose, output to /tmp to
> +# avoid overriding the existing.
> +dest_dir="/tmp"
> +
> +if [ -n "$1" ]; then
> + dest_dir=$1
> +fi
> +
> +systemd_dir=/usr/lib/systemd/system
Hi Chao,
I don't like this hardcoding of /usr dir either. I don't think we should
rely on hardcoded paths.
I am re-reading generators link and following is my understanding.
- generators are supposed to generate unit files. symlink document is
talking of is creating symlinks in other .wants/ dir of other unit
files and not vice-a versa.
- IIUC, there are 3 paths where unit files can be present. /usr/lib/,
/run/.. and /etc/systemd/. Here /usr has lowest prio, /run has higer
prio and /etc/systemd has highest prio.
To me that means if there are kdump.service unit files present in
all three pathes and if you do "systemctl enable kdump" then systmed
will first look into /etc, then in /run and then in /usr/ for a
kdump.service.
IOW, there is an hierarchy of unit files and hence we should not force
linking to a particular file in particular hiearchy. We should just
deal with unit file names and let systemd figure out which directory
to look into.
Chao, I just met lennart again and discussed this patch. He says that
creating symlink this way is fine. He mentioned that even though we
create a link to file in /usr/lib/ he will first look for that file
in /etc/systemd/ and use that if available otherwise will fall back
to the one in /usr/lib/. So ordering should not be a problem.
He was also fine with creating link to /usr/lib/systemd/ one as it
will always be present.
So I guess we are fine with this patch and you don't have to make further
modifications.
Thanks
Vivek
- So here is the proposal. I think let us generate a kdump.service
file
dynamically. I guess you can use /usr/lib/systemd/system/kdump.service
as template. And put Wants=network-online in that kdump.service file.
That way there will be two kdump.service files. One in /run/ and other
in /usr/ and one present in /run should override the one in /usr.
I think that's probably a better way to solve the problem.
Also if you are using existing kdump.service file as template, then you
can specify this in new kdump.service using SourcePath=.
Using existing file as template will help us so that if there are
any patches in kdump.service, we don't have to change kdump-dep-generator
all the time and make same changes there.
Thanks
Vivek
> +kdump_wants=$dest_dir/kdump.service.wants
> +
> +if is_ssh_dump_target; then
> + mkdir -p $kdump_wants
> + ln -sf $systemd_dir/network-online.target $kdump_wants/
> +fi
> diff --git a/kexec-tools.spec b/kexec-tools.spec
> index cfe14c4..e6ffade 100644
> --- a/kexec-tools.spec
> +++ b/kexec-tools.spec
> @@ -26,6 +26,7 @@ Source18: kdump.sysconfig.s390x
> Source19: eppic_030413.tar.gz
> Source20: kdump-lib.sh
> Source21: kdump-in-cluster-environment.txt
> +Source22: kdump-dep-generator.sh
>
> #######################################
> # These are sources for mkdumpramfs
> @@ -181,6 +182,7 @@ install -m 644 %{SOURCE14}
$RPM_BUILD_ROOT%{_sysconfdir}/udev/rules.d/98-kexec.r
> %endif
> install -m 644 %{SOURCE15} $RPM_BUILD_ROOT%{_mandir}/man5/kdump.conf.5
> install -m 644 %{SOURCE16} $RPM_BUILD_ROOT%{_unitdir}/kdump.service
> +install -m 755 -D %{SOURCE22}
$RPM_BUILD_ROOT%{_prefix}/lib/systemd/system-generators/kdump-dep-generator.sh
> mkdir -p $RPM_BUILD_ROOT/usr/sbin
> install -m 755 %{SOURCE17} $RPM_BUILD_ROOT/usr/sbin/rhcrashkernel-param
>
> @@ -320,6 +322,7 @@ done
> %{_mandir}/man8/*
> %{_mandir}/man5/*
> %{_unitdir}/kdump.service
> +%{_prefix}/lib/systemd/system-generators/kdump-dep-generator.sh
> %doc News
> %doc COPYING
> %doc TODO
> --
> 1.8.5.3