Re: [PATCH v3 2/2] kdumpctl: don't always rebuild when extra_modules is set

[Dave Young]

On 05/13/19 at 02:20pm, Kairui Song wrote:
> We don't necessarily have to always rebuild the initramfs when
> extra_modules is set, instead just detect if any module is updated
> and only rebuild initramfs if found any updated kernel module.
> Signed-off-by: Kairui Song
>
> kdumpctl | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/kdumpctl b/kdumpctl
> index 739f162..e77877a 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -334,6 +334,28 @@ check_files_modified()
>    files="$KDUMP_CONFIG_FILE $kdump_kernel $EXTRA_BINS $CORE_COLLECTOR"
>    [[ -e /etc/fstab ]] && files="$files /etc/fstab"
> # Check for any updated extra module
> EXTRA_MODULES="$(grep ^extra_modules $KDUMP_CONFIG_FILE | sed 's/^extra_modules\s*//')"
> if [ -n "$EXTRA_MODULES" ]; then
> if [ -e /lib/modules/$kdump_kver/modules.dep ]; then
> 	files="$files /lib/modules/$kdump_kver/modules.dep"
> fi
> for _module in $EXTRA_MODULES; do
> 	_module_file="$(modinfo --set-version "$kdump_kver" --filename "$_module" 2>/dev/null)"
> 	if [[ $? -eq 0 ]]; then
> 		files="$files $_module_file"
> 		for _dep_modules in $(modinfo -F depends $_module | tr ',' ' '); do
> 		    files="$files $(modinfo --set-version "$kdump_kver" --filename $_dep_modules 2>/dev/null)"
> 		done
> 	else
> 		# If it's not a module nor builtin, give an error
> 		if ( modprobe --set-version "$kdump_kver" --dry-run "$_module" &>/dev/null ); then
> 			echo "Module $_module not found"
> 		fi
> 	fi
> done
> fi
> check_exist "$files" && check_executable "$EXTRA_BINS"
>  [ $? -ne 0 ] && return 2
>
> @@ -563,7 +585,6 @@ check_system_modified()
>
> check_rebuild()
>  {
> local extra_modules
> local capture_capable_initrd="1"
> local _force_rebuild force_rebuild="0"
> local _force_no_rebuild force_no_rebuild="0"
>
> @@ -603,10 +624,6 @@ check_rebuild()
>    	return 0
>    fi
> #will rebuild every time if extra_modules are specified
> extra_modules=`grep ^extra_modules $KDUMP_CONFIG_FILE`
> [ -n "$extra_modules" ] && force_rebuild="1"
> #check to see if dependent files has been modified
>  #since last build of the image file
>  if [ -f $TARGET_INITRD ]; then
>
> -- 
> 2.20.1

The force rebuild in case extra_modules was inherited from RHEL6 code,
because there was not easy way to detect extra module directories
correctly probably the modprobe tools limitatioins.

Since this has been tested, and covered weak modules and other cases, I
think it is fine to drop the limitation.  But it would be good to add
above history info, also add some test info in patch log.

Otherwise:

Acked-by: Dave Young

Thanks
Dave