OK, thanks for the suggestion, will append following commit message in
next patch:
We need to make systemd-run execute in non-blocking mode, and do not
synchronously wait for the operation to finish, because udev expect the
command line in RUN to be finished immediately, however kdumpctl
reload may take 0.5-1s for an ordinary reload, or even slower on some
machines. So we give systemd-run an explicit --no-block option to run
in non-blocking mode. Without --no-blocking, systemd-run will verify,
enqueue and wait for the operation to finish.
By using the --no-block option, systemd-run will only verify and enqueue
the unit then return. In this way we make sure the command is
executed asynchronously, and the status will be monitored and logged by
systemd, which is reliable and non-blocking.
Another thing to mention is that --no-block is only needed after
systemd-v220,
before v220 systemd-run uses non-blocking mode by default and
--no-block option is not available on earlier systemd versions.
On Mon, Oct 29, 2018 at 6:56 PM Dave Young <dyoung(a)redhat.com> wrote:
On 10/25/18 at 02:21pm, Kairui Song wrote:
> According to udev's man page, PROGRAM is either used to determine
> device's name or whether the device matches the rule. So we should
> use RUN insteand. Meanwhile, both RUN / PROGRAM only accepts very
> short-running foreground tasks, but kdump restart may take a long
> time if there are any device changes that will lead to image rebuild,
> which may lead to buggy behavior.
>
> On the other hand, memory / CPU hot plug should never trigger a
> initramfs rebuild.
>
> To solve this problem, we will use new introduced "kdumpctl reload"
> instead, and use systemd-run to create a transient service unit for
> the reload and run it in no-block mode, so udev won't be blocked by
> anything.
Kairui, can you add more words here what is the behavior of "--no-block"
and why we have to use it?
>
> Also reformat the udev rules to a more maintanceable format.
>
> Signed-off-by: Kairui Song <kasong(a)redhat.com>
> ---
> 98-kexec.rules | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/98-kexec.rules b/98-kexec.rules
> index e32ee13..62ddd69 100644
> --- a/98-kexec.rules
> +++ b/98-kexec.rules
> @@ -1,4 +1,12 @@
> -SUBSYSTEM=="cpu", ACTION=="add", PROGRAM="/bin/systemctl
try-restart kdump.service"
> -SUBSYSTEM=="cpu", ACTION=="remove",
PROGRAM="/bin/systemctl try-restart kdump.service"
> -SUBSYSTEM=="memory", ACTION=="online",
PROGRAM="/bin/systemctl try-restart kdump.service"
> -SUBSYSTEM=="memory", ACTION=="offline",
PROGRAM="/bin/systemctl try-restart kdump.service"
> +SUBSYSTEM=="cpu", ACTION=="add", GOTO="kdump_reload"
> +SUBSYSTEM=="cpu", ACTION=="remove",
GOTO="kdump_reload"
> +SUBSYSTEM=="memory", ACTION=="online",
GOTO="kdump_reload"
> +SUBSYSTEM=="memory", ACTION=="offline",
GOTO="kdump_reload"
> +
> +GOTO="kdump_reload_end"
> +
> +LABEL="kdump_reload"
> +
> +RUN+="/usr/bin/systemctl is-active kdump.service &&
/usr/bin/systemd-run --no-block /sbin/kdumpctl reload"
> +
> +LABEL="kdump_reload_end"
> --
> 2.17.1
>
Thanks
Dave
--
Best Regards,
Kairui Song