[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191028032203.GA28082@Gentoo>
Date: Mon, 28 Oct 2019 08:52:08 +0530
From: Bhaskar Chowdhury <unixbhaskar@...il.com>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: yamada.masahiro@...ionext.com, michal.lkml@...kovi.net,
bfields@...ldses.org, linux-kbuild@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] scripts:prune-kernel:prune old kernels and modules dir
Thank you Randy to point out the mistakes, will correct it and send it
for your review. Below mention things am doing...
On 11:24 Sun 27 Oct 2019, Randy Dunlap wrote:
>On 10/24/19 10:47 PM, Bhaskar Chowdhury wrote:
>> This patch will remmove old kernel and associated modules directory from
>> the system.
>>
>> Few interaction with the script ,below
>>
>> 1) ✔ ~/git-linux/linux-kbuild [master ↑·18|✔]
>> 10:40 $ ./scripts/prune-kernel
>>
>>
>> 2)10:41 $ ./scripts/prune-kernel -h
>> You need to use this script like this :
>>
>> ./scripts/prune-kernel -r kernel_version modules_directory_name
>> ./scripts/prune-kernel -i option for interactive way to use it.
>>
>>
>> 3) 10:41 $ ./scripts/prune-kernel -r 5.2.2 5.2.2-gentoo
>> Removed kernel version:5.2.2 and modules directory:5.2.2-gentoo from
>> the system.
>>
>>
>> 4)10:41 $ ./scripts/prune-kernel -i
>>
>>
>> Want to removing old kernels and modules dir [YN]: Y
>> Please give another version to remove: 5.2.2
>
>These 2 lines above could (should) be combined into one line. E.g.:
>
>Enter kernel version to remove or blank/emtpy to exit:
>
>> /boot/vmlinuz-5.3.7-050307-generic
>> /boot/vmlinuz-5.3.6-050306-generic
>> find: ‘/boot/efi’: Permission denied
>
>too noisy.
>
modified , it will not spit this again.
>> Please give the full modules directory name to remove: 5.2.2-gentoo
>> 5.3.6-050306-generic
>> 5.3.7-050307-generic
>
>too noisy.
>
Same as above ...modified and will not spit again.
>>
>>
>> Removed kernel version:5.2.2 and associated modules:5.2.2-gentoo
>> ..Done.
>>
>>
>> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@...il.com>
>> ---
>> To Bruce,
>> I have incorporated all the changes you asked for ,kindly review.
>>
>> scripts/prune-kernel | 75 +++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 56 insertions(+), 19 deletions(-)
>>
>> diff --git a/scripts/prune-kernel b/scripts/prune-kernel
>> index e8aa940bc0a9..292ba70d7770 100755
>> --- a/scripts/prune-kernel
>> +++ b/scripts/prune-kernel
>> @@ -1,21 +1,58 @@
>> #!/bin/bash
>> # SPDX-License-Identifier: GPL-2.0
>> -
>> -# because I use CONFIG_LOCALVERSION_AUTO, not the same version again and
>> -# again, /boot and /lib/modules/ eventually fill up.
>> -# Dumb script to purge that stuff:
>> -
>> -for f in "$@"
>> -do
>> - if rpm -qf "/lib/modules/$f" >/dev/null; then
>> - echo "keeping $f (installed from rpm)"
>> - elif [ $(uname -r) = "$f" ]; then
>> - echo "keeping $f (running kernel) "
>> - else
>> - echo "removing $f"
>> - rm -f "/boot/initramfs-$f.img" "/boot/System.map-$f"
>> - rm -f "/boot/vmlinuz-$f" "/boot/config-$f"
>> - rm -rf "/lib/modules/$f"
>> - new-kernel-pkg --remove $f
>> - fi
>> -done
>> +#This script will delete old kernels and modules directory related to it interactively.
>> +#if you choose "-i" as interactive otherwise it will just go ahead and do the stuff once
>> +#you mentione the kernel_version and modules_directory_name as parameter.
>
> mention
>or better:
> enter
>
Replaced with mentioned word i.e enter.
>> +flag=$1
>
>$flag is not used anywhere.
>
For interactive use case we need that.
>> +kernel_ver=$2
>
>remove_old_kernel() uses $kernel_version, not $kernel_ver.
>
>> +modules_dir_name=$3
>
>remove_old_modules_dir() uses $modules_version, not $modules_dir_name.
>
>> +boot_dir=/boot
>> +modules_dir=/lib/modules
>need a blank line here.
Fixed.
>
>> +remove_old_kernel() {
>> + cd $boot_dir
>> + find $boot_dir -name "vmlinuz-*" -type f -exec ls -1 {} \;
>
Removed.
>why the 'ls'? too noisy IMO.
>
>> + rm -If vmlinuz-$kernel_version System.map-$kernel_version config-$kernel_version
>> + return 0
>> +}
>
>need a blank line here.
>
Fixed.
>> +remove_old_modules_dir() {
>> + cd $modules_dir
>> + find $modules_dir -maxdepth 0 -type d -exec ls -1 {} \;
>
>'ls' is too noisy.
Removed.
>
>> + rm -rf $modules_version
>> + return 0
>> +}
>> +while getopts :hir opt;
>
>$opt is not used anywhere.
It is required, without it , looks like this:
getopts: usage: getopts optstring name [arg]
>
>Does 'getopt's support long option strings? E.g.,
>--interactive, --help, --remove.
>
Yes ..
✔ ~/git-linux/linux-kbuild [master ↑·27|✚ 1]
08:44 $ ./scripts/prune-kernel --interactive
Enter kernel version to remove or blank/emtpy to exit: 5.2.2
Please give the full modules directory name to remove: 5.2.2-whatever
Removed kernel version: and associated modules:5.2.2-whatever ..Done.
>> +do
>
>'do' has a trailing space after it. drop it.
>
Certain thing ..this space and tab killing me...how fix these damn thing
Randy??
>> + case "$1" in
>
>Is $1 the same as $flag here?
Yes and change it to that variable.
>> + -i | --interactive)
>
>bad indentation above.
>
Fixed.
>> + printf "\n\n Want to removing old kernels and modules dir [YN]: %s"
>> + read response
>> + if [[ $response == "Y" ]];then
>> + printf "Please give another version to remove: %s"
>> + read kernel_version
>> + remove_old_kernel
>> + printf "Please give the full modules directory name to remove: %s"
>> + read modules_version
>> + remove_old_modules_dir
>> + printf "\n\n\n Removed kernel version:$kernel_version and associated modules:$modules_version ..Done. \n\n"
>> + elif [[ $response == "N" ]];then
>> + exit 1
>> + fi
>> + ;;
>> + -h | --help)
>
>bad indentation.
>
Fixed.
>> +
>
>line above is just a bunch of spaces. bad.
>
>> + printf "You need to use this script like this :\n
>
>use tabs to indent, not spaces.
>
Fixed.
>> + $0 -r kernel_version modules_directory_name\n
>> + $0 -i option for interactive way to use it.\n\n"
>
>inconsistent indentation.
>
Fixed.
>> +
>
>line above is just spaces. Empty lines are OK, even good for
>readablility, but they should not end with spaces or tabs.
>
Fixed.
>> + exit 1
>> + ;;
>> + -r | --remove)
>> + shift $(( OPTIND - 1 ))
>
>What is the purpose of the shift, when this case ends with exit?
>
Positional parameter shifting
>> + rm -f kernel_ver
>
> $kernel_ver
>
>Probably need to cd $boot_dir for above 'rm'.
>or just rm -f $boot_dir/$kernel_ver
>
Fixed.
>> + cd $modules_dir
>> + rm -rf $modules_dir_name
>> + printf "Removed kernel version:$kernel_ver and modules directory:$modules_dir_name from the system.\n\n"
>> + exit 0
>> + ;;
>> + esac
>
>esac indentation does not match case.
>
Fixed
>> + done
>
>done indentation does not match do.
>
Fixed
>
>Nack.
>
>--
>~Randy
Bhaskar
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists