On 13:03 Wed 06 Nov 2019, Masahiro Yamada wrote:
On Wed, Nov 6, 2019 at 12:10 PM Bhaskar Chowdhury <unixbhaskar@xxxxxxxxx> wrote:On 11:53 Wed 06 Nov 2019, Masahiro Yamada wrote: >On Tue, Nov 5, 2019 at 11:32 AM J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: >> >> On Mon, Nov 04, 2019 at 06:03:13PM -0800, Randy Dunlap wrote: >> > On 11/1/19 11:30 PM, Bhaskar Chowdhury wrote: >> > > This patch allow you to remove old kernels and associated modules >> > > directory from the system.You can do it at once with the -r flag >> > > and interactively with the -i flag. >> > > >> > > Signed-off-by: Bhaskar Chowdhury <unixbhaskar@xxxxxxxxx> >> > > --- >> > > scripts/prune-kernel | 82 +++++++++++++++++++++++++++++++++++--------- >> > > 1 file changed, 65 insertions(+), 17 deletions(-) >> > >> > Hi, >> > I believe that this script now does what the patch author intends it to do. >> > It does have a few whitespace issues, but no big deals. (see below) >> >> My original comment stands: looks like it prompts for full module path >> and kernel versions which means it's no more convenient than just doing >> an "ls" and then removing the ones you want to. (In fact, with "rm" >> you'd also get the benefit of tab completion....) >> >> It's quite different from the original script and I don't really see the >> advantage. >> >> --b. > >I am with Bruce. > >This patch is trying to replace everything >with worse code. Well,Masahiro, I won't mind dropping the idea, which you already concluded.But, would you care to let me know how worse the code seems to be????As far as I understood this script, it is useful to delete stale versions with a single command. scripts/prune-kernel 5.2-rc1 5.2-rc2 5.2-rc3 This patch is dropping the 'for f in "$@"' loop, so you would end up with running this script multiple times. scripts/prune-kernel -r 5.2-rc1 5.2-rc1 scripts/prune-kernel -r 5.2-rc2 5.2-rc2 scripts/prune-kernel -r 5.2-rc3 5.2-rc3
Yes, true. You don't want all the stale kernels to be discarded at once, do you???
What is funny is, it takes the kernel_version and modules_version separately.
Lack of farsight of mine ...will take your suggestion. Should have realized it before, but was writing for different purpose.
And now it requires -r option for the default behavior.
Because other flags are involved and they do different things.
I see nothing cool overall.
Meh... I am interpreting you "cool" literal way...which you certainly didn't mean it...do you??
J. Bruce Fields suggested:
"But if somebody does actually use it as-is, it'd be nicer to keep the
current behavior and add an option ("-i" or something) for the
interactive behavior."
Yes, that's true ...and now I understood. On the contrary ,if I recollect properly, Bruce ,once told that he won't if do something which will be useful to the wider users..not sure that stands anymore.
I want to see a patch if and only if you can add -i without intrusively changing the current code.
Hopefully , you wish get fulfilled sooner than later.
Masahiro
Bhaskar
> >BTW. >Bruce, >Does the current script expect RHEL or something? >I do not see 'new-kernel-pkg' on my Ubuntu machine. > >It would still work with 'new-kernel-pkg: command not found' >warning. > >We could bypass it if we like. > >command -v new-kernel-pkg && new-kernel-pkg --remove $f > > > >Masahiro Yamada > > > >> > >> > Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> >> > >> > >> > > diff --git a/scripts/prune-kernel b/scripts/prune-kernel >> > > index e8aa940bc0a9..01d0778db71f 100755 >> > > --- a/scripts/prune-kernel >> > > +++ b/scripts/prune-kernel >> > > @@ -1,21 +1,69 @@ >> > > #!/bin/bash >> > > # SPDX-License-Identifier: GPL-2.0 >> > > +#This script will remove old kernels and modules directory related to it. >> > > +#"-r" or "--remove" show how to silently remove old kernel and modules dir. >> > > +# "-h" or "--help" show how to use this script or show without parameter. >> > > +#"-i" or "--interactive" show how to remove interactively. >> > > + >> > > +flag=$1 >> > > +kernel_version=$2 >> > > +modules_version=$3 >> > > +boot_dir=/boot >> > > +modules_dir=/lib/modules >> > > + >> > > +remove_old_kernel() { >> > > + cd $boot_dir >> > > + rm -If vmlinuz-$kernel_version System.map-$kernel_version config-$kernel_version >> > > + return 0 >> > > +} >> > > + >> > > +remove_old_modules_dir() { >> > > + cd $modules_dir >> > > + rm -rf $modules_version >> > > + return 0 >> > > +} >> > > + >> > > +usage() { >> > > + printf "Usage: $(basename $0) [-ri]\n" >> > > + printf "\n -r or --remove kernel_version modules_version\n" >> > > + printf "\n -i or --interactive do as interactive way\n" >> > > + return 0 >> > > +} >> > > + >> > > +case "$flag" in >> > > + -i | --interactive) >> > > + printf "\nEnter kernel version to remove or blank/empty to exit:" >> > > + read kernel_version >> > > + if [[ $kernel_version != "" ]]; then >> > > + remove_old_kernel >> > > + printf "\nRemoved kernel version:$kernel_version from the system.\n\n" >> > >> > space after ':' >> > >> > drop one \n above. >> > >> > > + printf "Please give the full modules directory name to remove:" >> > > + read modules_version >> > > + if [[ $modules_version != "" ]]; then >> > > + remove_old_modules_dir >> > > + printf "\n\nRemoved modules directory:$modules_version from the system.\n\n" >> > >> > space after ':' >> > >> > drop one \n above. >> > >> > > + else >> > > + exit 1 >> > > + fi >> > > + fi >> > > + ;; >> > > + -h | --help) >> > > + usage >> > > + exit 0 >> > > + ;; >> > > + -r | --remove) >> > > + if [[ $# -ne 3 ]]; then >> > > + printf "You need to provide kernel version and modules directory name.\n" >> > > + exit 1 >> > > + else >> > > + remove_old_kernel >> > > + remove_old_modules_dir >> > > + fi >> > > + ;; >> > > + *) >> > > + usage >> > > + exit 1 >> > > + ;; >> > > +esac >> > > >> > > -# 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: >> > > >> > >> > OK, the former script's loop is removed.. good. >> > But the 2 preceding blank lines are not removed, so the script >> > now ends with 2 unnecessary blank lines. >> > >> > > -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 >> > > -- >> > >> > >> > -- >> > ~Randy > > > >-- >Best Regards > >Masahiro Yamada-- Best Regards Masahiro Yamada
Attachment:
signature.asc
Description: PGP signature
