On Thu, Jul 26, 2018 at 7:49 AM, Dave Young <dyoung(a)redhat.com> wrote:
On 07/26/18 at 01:51am, Bhupesh Sharma wrote:
> Hello Kairui,
>
> Thanks for the patch.
>
> Can you please list the changes in v2 (as compared to v1) in the commit message.
>
> On Wed, Jul 25, 2018 at 4:25 PM, Kairui Song <kasong(a)redhat.com> wrote:
> > Currently, we only rebuilt kdump initramfs on config file change,
> > fs change, or watchdog related change. This will not cover the case
> > that hardware changed but fs layout and other configurations still
> > stays the same, and kdump may fail.
>
> Hmm. This is a chicken and egg issue. We can have modules being
> unloaded (waiting for dependencies or timeouts to be met) or loaded
> while we executing 'lsmod' via "kdumpctl restart" , then we will
miss
> those modules being unloaded/loaded.
We only handle the current loaded ones when service is starting. setup
can change after service started, it will be only detected when service
restarts eg. next reboot.
One way is restart the kdump service for every module load uevents and
handle it in udev rules, but the side effect is we will have a lot of
restart and rebuild, it is not acceptable. So we depend on service
restart to detect the new changes.
Right. So I am not sure we get desired results with the current
approach as well - if we have to wait for the next reboot.
May be we can think of a more comprehensive approach for handling this.
Thanks,
Bhupesh
>> Can we look at ways to determine the same as well or at least warn the
>> user that this is probably not the final/correct representation of the
>> modules being unloaded/loaded when 'kdumpctl restart' is being
>> executed.
>>
>> Thanks,
>> Bhupesh
>>
>> > To cover such case, we can detect and compare loaded kernel modules,
>> > if a hardware change requires the image to be rebuilt, loaded kernel
>> > modules must have changed.
>> >
>> > Starting from commit 7047294 dracut will record loaded kernel modules
>> > when the image is built if hostonly mode is enabled. With this patch,
>> > kdumpctl will compare the recorded value with currently loaded kernel
>> > modules, and rebuild the image on change.
>> >
>> > "kdumpctl start" will be a bit slower, as we have to call lsinitrd
one
>> > more time to get the loaded kernel modules list. I measure the time
>> > consumption and we have an overall 0.2s increased loading time.
>> >
>> > Time consumption of command "kdumpctl restart":
>> >
>> > Before:
>> > real 0m0.587s
>> > user 0m0.481s
>> > sys 0m0.102s
>> >
>> > After:
>> > real 0m0.731s
>> > user 0m0.591s
>> > sys 0m0.133s
>> >
>> > Time comsumption of command "kdumpctl restart" with image
rebuild:
>> >
>> > Before (force rebuild):
>> > real 0m10.972s
>> > user 0m8.966s
>> > sys 0m1.318s
>> >
>> > After (inserted ~100 new modules):
>> > real 0m11.220s
>> > user 0m9.387s
>> > sys 0m1.337s
>> >
>> > Signed-off-by: Kairui Song <kasong(a)redhat.com>
>> > ---
>> > kdumpctl | 30 ++++++++++++++++++++++++++++++
>> > 1 file changed, 30 insertions(+)
>> >
>> > diff --git a/kdumpctl b/kdumpctl
>> > index 13b8209..6a01c13 100755
>> > --- a/kdumpctl
>> > +++ b/kdumpctl
>> > @@ -458,6 +458,31 @@ check_wdt_modified()
>> > return 1
>> > }
>> >
>> > +check_kmodules_modified()
>> > +{
>> > + # always sort again to avoid LANG/LC inconsistent problem
>> > + local _old_modules="$(lsinitrd $TARGET_INITRD -f
/usr/lib/dracut/loaded-kernel-modules.txt | sort)"
>> > + local _new_modules="$(get_loaded_kernel_modules | sort)"
>> > +
>> > + [[ -z $_old_modules ]] && echo "Warning: Previous
loaded kernel module list is absent or empty"
>> > +
>> > + local _added_modules=$(comm -13 <(echo "$_old_modules")
<(echo "$_new_modules"))
>> > + local _dropped_modules=$(comm -23 <(echo
"$_old_modules") <(echo "$_new_modules"))
>> > +
>> > + if [ "$_old_modules" != "$_new_modules" ]; then
>> > + echo "Detected change(s) of loaded kernel modules
list:"
>> > + [[ -n $_added_modules ]] && for _module in
$_added_modules; do
>> > + echo " +$_module"
>> > + done
>> > + [[ -n $_dropped_modules ]] && for _module in
$_dropped_modules; do
>> > + echo " -$_module"
>> > + done
>> > + return 1
>> > + fi
>> > +
>> > + return 0
>> > +}
>> > +
>> > # returns 0 if system is not modified
>> > # returns 1 if system is modified
>> > # returns 2 if system modification is invalid
>> > @@ -485,6 +510,11 @@ check_system_modified()
>> > return 1
>> > fi
>> >
>> > + check_kmodules_modified
>> > + if [ $? -ne 0 ]; then
>> > + return 1
>> > + fi
>> > +
>> > return 0
>> > }
>> >
>> > --
>> > 2.17.1
>> > _______________________________________________
>> > kexec mailing list -- kexec(a)lists.fedoraproject.org
>> > To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>> > Fedora Code of Conduct:
https://getfedora.org/code-of-conduct.html
>> > List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
>> > List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.o...
>> _______________________________________________
>> kexec mailing list -- kexec(a)lists.fedoraproject.org
>> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>> Fedora Code of Conduct:
https://getfedora.org/code-of-conduct.html
>> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
>> List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.o...
>
> Thanks
> Dave