[PATCH] prevent duplicated entries due to symlinks

[PATCH] prevent duplicated entries due to symlinks

[Andreas Born]

[-- Attachment #1: Type: text/plain, Size: 1438 bytes --]

If there's both a symlink and a kernel at which the symlink is pointing 
in the list of detected kernels of 10_linux, two entries are created for 
actually the same kernel. This patch checks for this condition and only 
uses the symlink (usually the wanted behaviour), in case the target of 
the symlink doesn't exist, it uses neither.

Furthermore there may be kernel images named e.g. /boot/vmlinuz, so to 
detect those the patch doesn't require a hyphen in the kernel name.
In this case the sed used to determine the kernel version, won't return 
as expected an empty string. Therefore I replaced it by another one with 
the wanted behaviour.
If the kernel version can be empty we don't want to have a "GNU/Linux, 
linux " menuentry. Accordingly this patch adds a check for empty kernel 
version, so that only "GNU/Linux" will be displayed.

Finally there's quite a bunch of other names for initrds. This patch 
adds support for initrd.gz and initrd.splash. Both initrd.img and 
initrd.gz become or were yet supported with two different version 
positions and without version at all. For initrd.splash this isn't 
needed because it's not kernel version dependent.

This is my first contribution and although I tried to inform myself as 
good as possible and double-checked everything, I could have missed 
something to you probably obvious. If there are any such problems, let 
me know, I'll do my best to get it sorted out.

Andreas


[-- Attachment #2: 10_linux.diff --]
[-- Type: text/plain, Size: 2837 bytes --]

diff --git a/ChangeLog b/ChangeLog
index 9097f25..ddf1c3e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-05-01  Andreas Born  <futur.andy@googlemail.com>
+
+    * util/grub.d/10_linux.in: Prevent duplicated entries because both
+	symlink and symlinked kernel are detected, only use the symlink
+	then. Detect also kernels without a hyphen in a name and use an
+	empty version for those kernels. Don't display linux <kernelver> in
+	the	menuentry title if version is empty. Detect more initrds e.g.
+	initrd.gz.
+
 2009-04-30  David S. Miller  <davem@davemloft.net>
 
 	* util/hostdisk.c (device_is_wholedisk): New function.
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index c2da413..66768eb 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -112,9 +112,18 @@ EOF
 EOF
 }
 
-list=`for i in /boot/vmlinu[xz]-* /vmlinu[xz]-* ; do
+list=`for i in /boot/vmlinu[xz]* /vmlinu[xz]* ; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
+for i in $list ; do
+  if test -h $i ; then
+    target="`readlink -f $i`"
+    list=`echo $list | tr ' ' '\n' | grep -vx $target | tr '\n' ' '`
+    if test ! -f $target ; then
+      list=`echo $list | tr ' ' '\n' | grep -vx $i | tr '\n' ' '`
+    fi
+  fi
+done
 
 while [ "x$list" != "x" ] ; do
   linux=`find_latest $list`
@@ -122,13 +131,16 @@ while [ "x$list" != "x" ] ; do
   basename=`basename $linux`
   dirname=`dirname $linux`
   rel_dirname=`make_system_path_relative_to_its_root $dirname`
-  version=`echo $basename | sed -e "s,^[^0-9]*-,,g"`
+  version=`echo $basename | sed -e "s,^[^\-]*-*,,"`
   alt_version=`echo $version | sed -e "s,\.old$,,g"`
   linux_root_device_thisversion="${LINUX_ROOT_DEVICE}"
 
   initrd=
   for i in "initrd.img-${version}" "initrd-${version}.img" \
-	   "initrd.img-${alt_version}" "initrd-${alt_version}.img"; do
+       "initrd.img-${alt_version}" "initrd-${alt_version}.img" \
+       "initrd.gz-${version}" "initrd-${version}.gz" \
+       "initrd.gz-${alt_version}" "initrd-${alt_version}.gz" \
+       "initrd.img" "initrd.gz" "initrd.splash"; do
     if test -e "${dirname}/${i}" ; then
       initrd="$i"
       break
@@ -140,10 +152,16 @@ while [ "x$list" != "x" ] ; do
     # "UUID=" magic is parsed by initrds.  Since there's no initrd, it can't work here.
     linux_root_device_thisversion=${GRUB_DEVICE}
   fi
+  
+  if test -n "${version}" ; then
+    title="${OS}, linux ${version}"
+  else
+    title="${OS}"
+  fi
 
-  linux_entry "${OS}, linux ${version}" \
+  linux_entry "${title}" \
       "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
-  linux_entry "${OS}, linux ${version} (recovery mode)" \
+  linux_entry "${title} (recovery mode)" \
       "single ${GRUB_CMDLINE_LINUX}"
 
   list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`

Re: [PATCH] prevent duplicated entries due to symlinks

[Pavel Roskin]

On Fri, 2009-05-01 at 22:51 +0200, Andreas Born wrote:
> If there's both a symlink and a kernel at which the symlink is pointing 
> in the list of detected kernels of 10_linux, two entries are created for 
> actually the same kernel. This patch checks for this condition and only 
> uses the symlink (usually the wanted behaviour), in case the target of 
> the symlink doesn't exist, it uses neither.
> 
> Furthermore there may be kernel images named e.g. /boot/vmlinuz, so to 
> detect those the patch doesn't require a hyphen in the kernel name.
> In this case the sed used to determine the kernel version, won't return 
> as expected an empty string. Therefore I replaced it by another one with 
> the wanted behaviour.
> If the kernel version can be empty we don't want to have a "GNU/Linux, 
> linux " menuentry. Accordingly this patch adds a check for empty kernel 
> version, so that only "GNU/Linux" will be displayed.

The result will be that we would have an entry without version instead
of an entry with a version.  That's hardly an improvement.  Can you give
an example where it would be useful?

> Finally there's quite a bunch of other names for initrds. This patch 
> adds support for initrd.gz and initrd.splash. Both initrd.img and 
> initrd.gz become or were yet supported with two different version 
> positions and without version at all. For initrd.splash this isn't 
> needed because it's not kernel version dependent.

Please specify where those names are used.  Can you give the
distribution name and version where such names are used?

What is initrd.splash?  Why does it need to be handled like initrd and
why is it always version independent?  Again, where is it used?

-- 
Regards,
Pavel Roskin

Re: [PATCH] prevent duplicated entries due to symlinks

[Andreas]

Pavel Roskin schrieb:
> On Fri, 2009-05-01 at 22:51 +0200, Andreas Born wrote:
>   
>> If there's both a symlink and a kernel at which the symlink is pointing 
>> in the list of detected kernels of 10_linux, two entries are created for 
>> actually the same kernel. This patch checks for this condition and only 
>> uses the symlink (usually the wanted behaviour), in case the target of 
>> the symlink doesn't exist, it uses neither.
>>
>> Furthermore there may be kernel images named e.g. /boot/vmlinuz, so to 
>> detect those the patch doesn't require a hyphen in the kernel name.
>> In this case the sed used to determine the kernel version, won't return 
>> as expected an empty string. Therefore I replaced it by another one with 
>> the wanted behaviour.
>> If the kernel version can be empty we don't want to have a "GNU/Linux, 
>> linux " menuentry. Accordingly this patch adds a check for empty kernel 
>> version, so that only "GNU/Linux" will be displayed.
>>     
>
> The result will be that we would have an entry without version instead
> of an entry with a version.  That's hardly an improvement.  Can you give
> an example where it would be useful?
>   
You have a symlink named /boot/vmlinuz which points at the current 
kernel version. Now you could of course find out which kernel version 
it's pointing at but that version could change anytime. /boot/vmlinu[zx] 
and /vmlinu[zx] (there should only exist one of them) are the only cases 
I can think of which have no version in their name. So you always know 
that the version-free entry always boots the kernel pointed at by 
/boot/vmlinuz, which is always the current kernel if your distro 
maintains the symlink. I see no regression there.
This is for example my Zenwalk boot entry:
'menuentry "Zenwalk GNU/Linux" {
    set root=(hd0,3)
    search --fs-uuid --set 408e3e53-d766-4592-bb12-31233bd415c0
    linux16    /boot/vmlinuz root=/dev/sda3 ro  splash=silent 
resume=/dev/sda7 vga=794
    initrd16    /boot/initrd.splash
}'
That's the symlink: /boot/vmlinuz -> vmlinuz-2.6.29.2


>   
>> Finally there's quite a bunch of other names for initrds. This patch 
>> adds support for initrd.gz and initrd.splash. Both initrd.img and 
>> initrd.gz become or were yet supported with two different version 
>> positions and without version at all. For initrd.splash this isn't 
>> needed because it's not kernel version dependent.
>>     
>
> Please specify where those names are used.  Can you give the
> distribution name and version where such names are used?
>   
Zenwalk, Slackware for example. I think more Slackware derivates are 
using it too.
> What is initrd.splash?  Why does it need to be handled like initrd and
> why is it always version independent?  Again, where is it used?
>
>   
initrd.splash is used by Zenwalk (others too) it contains only the data 
of one or multiple bootsplashes as returned by the "splash" command. 
It's used by bootsplash and doesn't contain any files, modules, etc.
The version independence is quite self-explaining now, I think. Unless 
bootsplash support was removed it works for any kernel version. And even 
if bootsplash support is removed it does no harm to load it.

Is it understandable now?

Andreas

Re: [PATCH] prevent duplicated entries due to symlinks

[Pavel Roskin]

On Mon, 2009-05-04 at 18:45 +0200, Andreas wrote:
> Pavel Roskin schrieb:
> > The result will be that we would have an entry without version instead
> > of an entry with a version.  That's hardly an improvement.  Can you give
> > an example where it would be useful?
> >   
> You have a symlink named /boot/vmlinuz which points at the current 
> kernel version. Now you could of course find out which kernel version 
> it's pointing at but that version could change anytime.

I see, you want to support loading the "default" kernel, i.e. the one
pointed to by the symlink.  This way, re-running grub-mkconfig won't be
necessary if a new kernel is installed.

>  /boot/vmlinu[zx] 
> and /vmlinu[zx] (there should only exist one of them) are the only cases 
> I can think of which have no version in their name. So you always know 
> that the version-free entry always boots the kernel pointed at by 
> /boot/vmlinuz, which is always the current kernel if your distro 
> maintains the symlink. I see no regression there.

Suppose I have Linux 2.6.29 and 2.6.28.  Your script makes entries for
"Linux default" and "Linux 2.6.28".  Then I install Linux 2.6.30 and
don't run grub-mkconfig.  Then I can boot Linux 2.6.30 and 2.6.28 form
the menu, but not 2.6.29.

Even if I run grub-mkconfig, there is still something I lose.  By
selecting "Linux default" I still don't know it it's Linux 2.6.30 or
something else.

For me, the convenience of not having to rerun grub-mkconfig doesn't
outweigh the convenience of knowing what I'm loading.

I know, it's have to argue about preferences, but if you want you
changes to be accepted, you need to present a good case.

I would consider placing the kernel pointed to by the vmlinuz link to
the first position of the Linux kernels.  Another approach would be to
have an entry for the link without suppressing any versioned entries.

> > Please specify where those names are used.  Can you give the
> > distribution name and version where such names are used?
> >   
> Zenwalk, Slackware for example. I think more Slackware derivates are 
> using it too.

Let's add support for more versioned names first.  That should be rather
non-controversial.

> > What is initrd.splash?  Why does it need to be handled like initrd and
> > why is it always version independent?  Again, where is it used?
> >
> >   
> initrd.splash is used by Zenwalk (others too) it contains only the data 
> of one or multiple bootsplashes as returned by the "splash" command. 
> It's used by bootsplash and doesn't contain any files, modules, etc.
> The version independence is quite self-explaining now, I think. Unless 
> bootsplash support was removed it works for any kernel version. And even 
> if bootsplash support is removed it does no harm to load it.
> 
> Is it understandable now?

It's quite strange that some distro would use the initrd functionality
purely for eye candy.  But if that's true, I'm fine with using
initrd.splash as the last resort if no other initrd image is found.

-- 
Regards,
Pavel Roskin

Re: [PATCH] prevent duplicated entries due to symlinks

[Andreas Born]

[-- Attachment #1: Type: text/plain, Size: 2603 bytes --]

Pavel Roskin schrieb:
> Suppose I have Linux 2.6.29 and 2.6.28.  Your script makes entries for
> "Linux default" and "Linux 2.6.28".  Then I install Linux 2.6.30 and
> don't run grub-mkconfig.  Then I can boot Linux 2.6.30 and 2.6.28 form
> the menu, but not 2.6.29.
>
> Even if I run grub-mkconfig, there is still something I lose.  By
> selecting "Linux default" I still don't know it it's Linux 2.6.30 or
> something else.
>
> For me, the convenience of not having to rerun grub-mkconfig doesn't
> outweigh the convenience of knowing what I'm loading.
>
> I know, it's have to argue about preferences, but if you want you
> changes to be accepted, you need to present a good case.
>   
It's also that kind of discussion which eventually separates the wheat 
from the chaff. Thus I welcome it.
> I would consider placing the kernel pointed to by the vmlinuz link to
> the first position of the Linux kernels.  Another approach would be to
> have an entry for the link without suppressing any versioned entries.
>   
Ok, I understand and if it is a preference why don't make it a 
preference. :)
You can find attached a new version of the patch, which per default will 
read the symlink and create an entry for the target, not the symlink. 
You can change this behaviour with "GRUB_USE_LINUX_SYMLINK=yes". If you 
define this environment variable with that value, it will use the 
behaviour I implemented before. This means it uses the symlink and 
ignores the kernel the symlink is currently pointing at.
I think this is the best solution, because it prevents duplicated 
entries (only one entry for symlink or symlink target) and it gives the 
user the choice which behaviour he wants (rerun of grub-mkconfig needed 
or not needed).
Is that solution all right?

>>> Please specify where those names are used.  Can you give the
>>> distribution name and version where such names are used?
>>>   
>>>       
>> Zenwalk, Slackware for example. I think more Slackware derivates are 
>> using it too.
>>     
>
> Let's add support for more versioned names first.  That should be rather
> non-controversial.
I see no more versioned names which could be added for the existing 
extension(s). There is initrd-<version>.<ext> and 
initrd.<ext>-<version>. Whereas <version> can either be $version or 
$alt_version and <ext> can be "img" or "gz" ("gz" is new) and at the 
very end as a last resort initrd.img initrd.gz and initrd.splash are 
tried in that order (that's also new). Roughly speaking only new 
fallback options.
Do you see a problem there or do you have a concrete versioned name to add?

Andreas

[-- Attachment #2: 10_linux.diff --]
[-- Type: text/plain, Size: 3185 bytes --]

diff --git a/ChangeLog b/ChangeLog
index 2203b68..f1606ba 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -0,0 +1,12 @@
+2009-05-05  Andreas Born  <futur.andy@googlemail.com>
+
+    * util/grub.d/10_linux.in: Prevent duplicated entries because both
+    symlink and symlinked kernel are detected.
+    Default: use only target of the symlink
+    GRUB_USE_LINUX_SYMLINK=yes: use only the symlink
+    Detect also kernels without a hyphen in a name and use an
+    empty version for those kernels.
+    Don't display ', linux <kernelver>' in the menuentry title if
+    version is empty.
+    Detect more initrds e.g. initrd.gz.
+
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index c2da413..f2649be 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -112,9 +112,32 @@ EOF
 EOF
 }
 
-list=`for i in /boot/vmlinu[xz]-* /vmlinu[xz]-* ; do
+list=`for i in /boot/vmlinu[xz]* /vmlinu[xz]* ; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
+	  
+for i in $list ; do
+  if ! test -h $i ; then
+    continue # no symlink
+  fi
+  target="`readlink -f $i`"
+  if test ! -f $target ; then
+    # inexistent target
+    list=`echo $list | tr ' ' '\n' | grep -vx $i | grep -vx $target \
+           | tr '\n' ' '`
+    continue
+  fi
+  
+  # default: use the target, not the symlink
+  if [ "${GRUB_USE_LINUX_SYMLINK}" = "yes" ] ; then
+    list=`echo $list | tr ' ' '\n' | grep -vx $target | tr '\n' ' '`
+  else
+    list=`echo $list | tr ' ' '\n' | grep -vx $i | tr '\n' ' '`
+    if ! echo $list | grep -qw $target ; then
+      list="$list $target"
+    fi
+  fi
+done
 
 while [ "x$list" != "x" ] ; do
   linux=`find_latest $list`
@@ -122,13 +145,16 @@ while [ "x$list" != "x" ] ; do
   basename=`basename $linux`
   dirname=`dirname $linux`
   rel_dirname=`make_system_path_relative_to_its_root $dirname`
-  version=`echo $basename | sed -e "s,^[^0-9]*-,,g"`
+  version=`echo $basename | sed -e "s,^[^\-]*-*,,"`
   alt_version=`echo $version | sed -e "s,\.old$,,g"`
   linux_root_device_thisversion="${LINUX_ROOT_DEVICE}"
 
   initrd=
   for i in "initrd.img-${version}" "initrd-${version}.img" \
-	   "initrd.img-${alt_version}" "initrd-${alt_version}.img"; do
+       "initrd.img-${alt_version}" "initrd-${alt_version}.img" \
+       "initrd.gz-${version}" "initrd-${version}.gz" \
+       "initrd.gz-${alt_version}" "initrd-${alt_version}.gz" \
+       "initrd.img" "initrd.gz" "initrd.splash"; do
     if test -e "${dirname}/${i}" ; then
       initrd="$i"
       break
@@ -140,10 +166,16 @@ while [ "x$list" != "x" ] ; do
     # "UUID=" magic is parsed by initrds.  Since there's no initrd, it can't work here.
     linux_root_device_thisversion=${GRUB_DEVICE}
   fi
+  
+  if test -n "${version}" ; then
+    title="${OS}, linux ${version}"
+  else
+    title="${OS}"
+  fi
 
-  linux_entry "${OS}, linux ${version}" \
+  linux_entry "${title}" \
       "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
-  linux_entry "${OS}, linux ${version} (recovery mode)" \
+  linux_entry "${title} (recovery mode)" \
       "single ${GRUB_CMDLINE_LINUX}"
 
   list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`

Re: [PATCH] prevent duplicated entries due to symlinks

[Andreas]

ping?
any problems still?

Andreas

Andreas Born schrieb:
> Pavel Roskin schrieb:
>> Suppose I have Linux 2.6.29 and 2.6.28.  Your script makes entries for
>> "Linux default" and "Linux 2.6.28".  Then I install Linux 2.6.30 and
>> don't run grub-mkconfig.  Then I can boot Linux 2.6.30 and 2.6.28 form
>> the menu, but not 2.6.29.
>>
>> Even if I run grub-mkconfig, there is still something I lose.  By
>> selecting "Linux default" I still don't know it it's Linux 2.6.30 or
>> something else.
>>
>> For me, the convenience of not having to rerun grub-mkconfig doesn't
>> outweigh the convenience of knowing what I'm loading.
>>
>> I know, it's have to argue about preferences, but if you want you
>> changes to be accepted, you need to present a good case.
>>   
> It's also that kind of discussion which eventually separates the wheat 
> from the chaff. Thus I welcome it.
>> I would consider placing the kernel pointed to by the vmlinuz link to
>> the first position of the Linux kernels.  Another approach would be to
>> have an entry for the link without suppressing any versioned entries.
>>   
> Ok, I understand and if it is a preference why don't make it a 
> preference. :)
> You can find attached a new version of the patch, which per default 
> will read the symlink and create an entry for the target, not the 
> symlink. You can change this behaviour with 
> "GRUB_USE_LINUX_SYMLINK=yes". If you define this environment variable 
> with that value, it will use the behaviour I implemented before. This 
> means it uses the symlink and ignores the kernel the symlink is 
> currently pointing at.
> I think this is the best solution, because it prevents duplicated 
> entries (only one entry for symlink or symlink target) and it gives 
> the user the choice which behaviour he wants (rerun of grub-mkconfig 
> needed or not needed).
> Is that solution all right?
>
>>>> Please specify where those names are used.  Can you give the
>>>> distribution name and version where such names are used?
>>>>         
>>> Zenwalk, Slackware for example. I think more Slackware derivates are 
>>> using it too.
>>>     
>>
>> Let's add support for more versioned names first.  That should be rather
>> non-controversial.
> I see no more versioned names which could be added for the existing 
> extension(s). There is initrd-<version>.<ext> and 
> initrd.<ext>-<version>. Whereas <version> can either be $version or 
> $alt_version and <ext> can be "img" or "gz" ("gz" is new) and at the 
> very end as a last resort initrd.img initrd.gz and initrd.splash are 
> tried in that order (that's also new). Roughly speaking only new 
> fallback options.
> Do you see a problem there or do you have a concrete versioned name to 
> add?
>
> Andreas

Re: [PATCH] prevent duplicated entries due to symlinks

[Pavel Roskin]

On Sun, 2009-05-10 at 00:20 +0200, Andreas wrote:
> ping?
> any problems still?

I don't think anyone is really interested in that change.

-- 
Regards,
Pavel Roskin

Re: [PATCH] prevent duplicated entries due to symlinks

[Michal Suchanek]

2009/5/4 Andreas <futur.andy@googlemail.com>:

>
> You have a symlink named /boot/vmlinuz which points at the current kernel
> version. Now you could of course find out which kernel version it's pointing
> at but that version could change anytime. /boot/vmlinu[zx] and /vmlinu[zx

Both are typically present on x86 and x86_64 systems running oprofile
due to oprofile limitations.

While you can technically store the uncompressed kernel anywhere
storing it together with the bootable kernel (and its config, symbol
list, ...) is suggested in oprofile documentation and it is place no
worse than any other.

Thanks

Michal