Hi Pingfan, I've merged this patch, so if you send V2 you can drop this one.

On Wed, Jul 1, 2020 at 10:59 AM Dave Young dyoung@redhat.com wrote:

On 06/30/20 at 11:05am, Pingfan Liu wrote:

...

Signed-off-by: Pingfan Liu piliu@redhat.com

kdump.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kdump.conf b/kdump.conf index 4586ccd..ed78507 100644 --- a/kdump.conf +++ b/kdump.conf @@ -79,7 +79,7 @@ # or script after the vmcore dump process terminates. # The exit status of the current dump process is fed to # the executable binary or script as its first argument. -# If /etc/kdump/post.d directory is exist, All files in +# If /etc/kdump/post.d directory exists, all files in # the directory are collectively sorted and executed in # lexical order, before binary or script specified

# kdump_post parameter is executed.

2.7.5 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org

Acked-by: Dave Young dyoung@redhat.com

Thanks Dave _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org

Hi Pingfan,

See one comment inline. On 06/30/20 at 11:05am, Pingfan Liu wrote:

If the directory /etc/kdump/pre.d is optional, then it is hard to tell between the following cases: -1. no directory "/etc/kdump/pre.d" -2. "rm -rf /etc/kdump/pre.d", which removes all scripts under pre.d For the second case, kdump.img should be rebuilt.

To bail out from this corner case, always creating pre.d and post.d.

Signed-off-by: Pingfan Liu piliu@redhat.com

kdumpctl | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/kdumpctl b/kdumpctl index 70fb551..5f1c956 100755 --- a/kdumpctl +++ b/kdumpctl @@ -345,6 +345,9 @@ check_files_modified() POST_FILES="$POST_FILES $file" fi done

• else

• echo "creating /etc/kdump/post.d"
  

• mkdir -p /etc/kdump/post.d
  

  fi if [ -d /etc/kdump/pre.d ]; then for file in /etc/kdump/pre.d/*; do

@@ -352,6 +355,9 @@ check_files_modified() PRE_FILES="$PRE_FILES $file" fi done

• else

• echo "creating /etc/kdump/pre.d"
  

• mkdir -p /etc/kdump/pre.d
  

In this way, the dir created can not be tracked by rpm, that means after uninstall kexec-tools, there will be something which are not cleaned

fi CORE_COLLECTOR=`grep ^core_collector $KDUMP_CONFIG_FILE | cut -d\ -f2` CORE_COLLECTOR=`type -P $CORE_COLLECTOR` -- 2.7.5 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org

Thanks Dave

On 07/01/2020 10:47 AM, Dave Young wrote:

Hi Pingfan,

See one comment inline. On 06/30/20 at 11:05am, Pingfan Liu wrote:

...

If the directory /etc/kdump/pre.d is optional, then it is hard to tell between the following cases: -1. no directory "/etc/kdump/pre.d" -2. "rm -rf /etc/kdump/pre.d", which removes all scripts under pre.d For the second case, kdump.img should be rebuilt.

To bail out from this corner case, always creating pre.d and post.d.

Signed-off-by: Pingfan Liu piliu@redhat.com

kdumpctl | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/kdumpctl b/kdumpctl index 70fb551..5f1c956 100755 --- a/kdumpctl +++ b/kdumpctl @@ -345,6 +345,9 @@ check_files_modified() POST_FILES="$POST_FILES $file" fi done

• else

• echo "creating /etc/kdump/post.d"
  

• mkdir -p /etc/kdump/post.d
  

  fi if [ -d /etc/kdump/pre.d ]; then for file in /etc/kdump/pre.d/*; do

@@ -352,6 +355,9 @@ check_files_modified() PRE_FILES="$PRE_FILES $file" fi done

• else

• echo "creating /etc/kdump/pre.d"
  

• mkdir -p /etc/kdump/pre.d
  

In this way, the dir created can not be tracked by rpm, that means after uninstall kexec-tools, there will be something which are not cleaned

After consideration, if a user creates scripts under /etc/kdump/pre.d, we had better to keep them untouched during un-instillation.

Then what about this: diff --git a/kdumpctl b/kdumpctl index 73f45d3..8b3c633 100755 --- a/kdumpctl +++ b/kdumpctl @@ -342,6 +342,9 @@ check_files_modified() POST_FILES="$POST_FILES $file" fi done + else + echo "creating /etc/kdump/post.d" + mkdir -p /etc/kdump/post.d fi if [ -d /etc/kdump/pre.d ]; then for file in /etc/kdump/pre.d/*; do @@ -349,6 +352,9 @@ check_files_modified() PRE_FILES="$PRE_FILES $file" fi done + else + echo "creating /etc/kdump/pre.d" + mkdir -p /etc/kdump/pre.d fi CORE_COLLECTOR=`grep ^core_collector $KDUMP_CONFIG_FILE | cut -d\ -f2` CORE_COLLECTOR=`type -P $CORE_COLLECTOR` diff --git a/kexec-tools.spec b/kexec-tools.spec index 49e1d46..900bbc4 100644 --- a/kexec-tools.spec +++ b/kexec-tools.spec @@ -182,6 +182,9 @@ make -C makedumpfile-1.6.7 LDFLAGS="$LDFLAGS -I../eppic/libeppic -L../eppic/libe %install mkdir -p -m755 $RPM_BUILD_ROOT/usr/sbin mkdir -p -m755 $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig +mkdir -p -m755 $RPM_BUILD_ROOT%{_sysconfdir}/kdump +mkdir -p -m755 $RPM_BUILD_ROOT%{_sysconfdir}/kdump/pre.d +mkdir -p -m755 $RPM_BUILD_ROOT%{_sysconfdir}/kdump/post.d mkdir -p -m755 $RPM_BUILD_ROOT%{_localstatedir}/crash mkdir -p -m755 $RPM_BUILD_ROOT%{_mandir}/man8/ mkdir -p -m755 $RPM_BUILD_ROOT%{_mandir}/man5/ @@ -350,6 +353,9 @@ done %endif %{dracutlibdir}/modules.d/* %dir %{_localstatedir}/crash +%dir %{_sysconfdir}/kdump +%dir %{_sysconfdir}/kdump/pre.d +%dir %{_sysconfdir}/kdump/post.d %{_mandir}/man8/kdumpctl.8.gz %{_mandir}/man8/kexec.8.gz %{_mandir}/man8/makedumpfile.8.gz (END)

Thanks, Pingfan

On 07/06/2020 02:00 PM, Kairui Song wrote:

On Fri, Jul 3, 2020 at 11:41 AM piliu piliu@redhat.com wrote:

...

On 07/01/2020 10:47 AM, Dave Young wrote:

...

Hi Pingfan,

See one comment inline. On 06/30/20 at 11:05am, Pingfan Liu wrote:

...

If the directory /etc/kdump/pre.d is optional, then it is hard to tell between the following cases: -1. no directory "/etc/kdump/pre.d" -2. "rm -rf /etc/kdump/pre.d", which removes all scripts under pre.d For the second case, kdump.img should be rebuilt.

To bail out from this corner case, always creating pre.d and post.d.

Signed-off-by: Pingfan Liu piliu@redhat.com

kdumpctl | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/kdumpctl b/kdumpctl index 70fb551..5f1c956 100755 --- a/kdumpctl +++ b/kdumpctl @@ -345,6 +345,9 @@ check_files_modified() POST_FILES="$POST_FILES $file" fi done

• else

•        echo "creating /etc/kdump/post.d"
  

•        mkdir -p /etc/kdump/post.d
  

  fi if [ -d /etc/kdump/pre.d ]; then for file in /etc/kdump/pre.d/*; do

@@ -352,6 +355,9 @@ check_files_modified() PRE_FILES="$PRE_FILES $file" fi done

• else

•        echo "creating /etc/kdump/pre.d"
  

•        mkdir -p /etc/kdump/pre.d
  

In this way, the dir created can not be tracked by rpm, that means after uninstall kexec-tools, there will be something which are not cleaned

After consideration, if a user creates scripts under /etc/kdump/pre.d, we had better to keep them untouched during un-instillation.

Then what about this: diff --git a/kdumpctl b/kdumpctl index 73f45d3..8b3c633 100755 --- a/kdumpctl +++ b/kdumpctl @@ -342,6 +342,9 @@ check_files_modified() POST_FILES="$POST_FILES $file" fi done

•   else
  

•           echo "creating /etc/kdump/post.d"
  

•           mkdir -p /etc/kdump/post.d
    fi
    if [ -d /etc/kdump/pre.d ]; then
            for file in /etc/kdump/pre.d/*; do
  

@@ -349,6 +352,9 @@ check_files_modified() PRE_FILES="$PRE_FILES $file" fi done

•   else
  

•           echo "creating /etc/kdump/pre.d"
  

•           mkdir -p /etc/kdump/pre.d
    fi
    CORE_COLLECTOR=`grep ^core_collector $KDUMP_CONFIG_FILE | cut
  

-d\ -f2` CORE_COLLECTOR=`type -P $CORE_COLLECTOR` diff --git a/kexec-tools.spec b/kexec-tools.spec index 49e1d46..900bbc4 100644 --- a/kexec-tools.spec +++ b/kexec-tools.spec @@ -182,6 +182,9 @@ make -C makedumpfile-1.6.7 LDFLAGS="$LDFLAGS -I../eppic/libeppic -L../eppic/libe %install mkdir -p -m755 $RPM_BUILD_ROOT/usr/sbin mkdir -p -m755 $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig +mkdir -p -m755 $RPM_BUILD_ROOT%{_sysconfdir}/kdump +mkdir -p -m755 $RPM_BUILD_ROOT%{_sysconfdir}/kdump/pre.d +mkdir -p -m755 $RPM_BUILD_ROOT%{_sysconfdir}/kdump/post.d mkdir -p -m755 $RPM_BUILD_ROOT%{_localstatedir}/crash mkdir -p -m755 $RPM_BUILD_ROOT%{_mandir}/man8/ mkdir -p -m755 $RPM_BUILD_ROOT%{_mandir}/man5/ @@ -350,6 +353,9 @@ done %endif %{dracutlibdir}/modules.d/* %dir %{_localstatedir}/crash +%dir %{_sysconfdir}/kdump +%dir %{_sysconfdir}/kdump/pre.d +%dir %{_sysconfdir}/kdump/post.d %{_mandir}/man8/kdumpctl.8.gz %{_mandir}/man8/kexec.8.gz %{_mandir}/man8/makedumpfile.8.gz (END)

Thanks, Pingfan _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org

Hi Pingfan , I think the new changes looks good, but maybe we can drop the "mkdir -p /etc/kdump/post.d" in script, user should not delete the post.d/pre.d directory, they should only delete the files in it.

Ok, thanks. I will update v2 according to your suggestion.

Best Regards, Pingfan

On 06/30/20 at 11:05am, Pingfan Liu wrote:

The following false alarm can be observed: /etc/kdump/pre.d/* is not executable Suppress it.

I'm confused about the log, not sure why it is false alarm. May need explain it a bit more..

Signed-off-by: Pingfan Liu piliu@redhat.com

dracut-module-setup.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 2454463..fbbd21a 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -419,7 +419,7 @@ kdump_install_pre_post_conf() { for file in /etc/kdump/pre.d/*; do if [ -x "$file" ]; then dracut_install $file

•        else
  

•        elif [ $file != "/etc/kdump/pre.d/*" ]; then
  

Do not get the purpose of above check :(

            echo "$file is not executable"
         fi
     done

@@ -429,7 +429,7 @@ kdump_install_pre_post_conf() { for file in /etc/kdump/post.d/*; do if [ -x "$file" ]; then dracut_install $file

•        else
  

•        elif [ $file != "/etc/kdump/post.d/*" ]; then
             echo "$file is not executable"
         fi
     done
  

-- 2.7.5 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org

Thanks Dave