On Mon, Dec 19, 2022 at 04:00:32PM +0100, Philipp Rudo wrote:
Hi Coiby,
a little nit picking from my side ;-)
On Fri, 16 Dec 2022 08:52:47 +0800
Coiby Xu <coxu(a)redhat.com> wrote:
> Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=2151500
>
> Currently, kdump initrd fails to be built when dumping vmcore to
> localhost via ssh or nfs,
>
> kdumpctl[3331]: Cannot get driver information: Operation not supported
> kdumpctl[1991]: dracut: Failed to get the driver of lo
> dracut[2020]: Failed to get the driver of lo
> kdumpctl[1775]: kdump: mkdumprd: failed to make kdump initrd
> kdumpctl[1775]: kdump: Starting kdump: [FAILED]
> systemd[1]: kdump.service: Main process exited, code=exited, status=1/FAILURE
> systemd[1]: kdump.service: Failed with result 'exit-code'.
> systemd[1]: Failed to start Crash recovery kernel arming.
> systemd[1]: kdump.service: Consumed 1.710s CPU time.
>
> This is because the loopback interface is used for transferring vmcore and
> ethtool can't get the driver of the loopback interface. In fact, once
> COFNIG_NET is enabled, the loopback device is enabled and there is no driver
> for the loopback device. So skip installing driver for the loopback device.
> The loopback interface is implemented in linux/drivers/net/loopback.c
> and always has the name "lo". So we can safely tell if a network
> interface is the loopback interface by its name.
>
> Fixes: a65dde2d ("Reduce kdump memory consumption by only installing needed NIC
drivers")
> Reported-by: Martin Pitt <mpitt(a)redhat.com>
> Reported-by: Rich Megginson <rmeggins(a)redhat.com>
> Signed-off-by: Coiby Xu <coxu(a)redhat.com>
> ---
> v2
> - distinguish the loopback interface simply by its name
> ---
> dracut-module-setup.sh | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> index 13e99015..aac3d8e5 100755
> --- a/dracut-module-setup.sh
> +++ b/dracut-module-setup.sh
> @@ -387,6 +387,7 @@ kdump_install_nic_driver() {
> _drivers=()
>
> for _netif in $1; do
> + [[ $_netif != lo ]] || continue
personally I find this double negative here hard to understand. So
instead of saying "if the _netif is not lo and the test fails" you
could just say "if the _netif is lo", aka.
[[ $_netif == "lo" ]] && continue
That would also better match the rest of the function.
Thanks for offering a way to improve the code readability!
Anyway, you choose what you prefer
Reviewed-by: Philipp Rudo <prudo(a)redhat.com>
And thanks for the code review!
> _driver=$(_get_nic_driver "$_netif")
> if [[ -z $_driver ]]; then
> derror "Failed to get the driver of $_netif"
> @@ -404,6 +405,7 @@ kdump_install_nic_driver() {
> _drivers+=("$_driver")
> done
>
> + [[ -n ${_drivers[*]} ]] || return
> instmods "${_drivers[@]}"
> }
>
--
Best regards,
Coiby