[patch] incorrect root device while /dev/ on tmpfs

[patch] incorrect root device while /dev/ on tmpfs

[Vasily Averin]

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

While /dev mounted as tmpfs it does not contain /dev/disk/by-uuid directory,
10_linux script did not check it properly and do not set root device via UUID.
Therefore following kernel boot can fail (incorrect root device) if device names
was changed.

Attached path fixes this issue. Please review and commit it

thank you,
	Vasily Averin


[-- Attachment #2: grub2-tmpdevuuid.patch --]
[-- Type: text/plain, Size: 601 bytes --]

diff -up ./util/grub.d/10_linux.in.tmpdevuuid ./util/grub.d/10_linux.in
--- ./util/grub.d/10_linux.in.tmpdevuuid	2009-08-29 20:05:06.000000000 +0400
+++ ./util/grub.d/10_linux.in	2009-09-03 11:07:34.000000000 +0400
@@ -36,7 +36,7 @@ case ${GRUB_DEVICE} in
 esac
 
 if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
-    || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" ; then
+    || ( test -d /dev/disk/by-uuid  && ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}") ; then
   LINUX_ROOT_DEVICE=${GRUB_DEVICE}
 else
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}

Re: [patch] incorrect root device while /dev/ on tmpfs

[Felix Zielcke]

Am Samstag, den 17.10.2009, 16:24 +0400 schrieb Vasily Averin:
> While /dev mounted as tmpfs it does not contain /dev/disk/by-uuid
> directory,
> 10_linux script did not check it properly and do not set root device
> via UUID.
> Therefore following kernel boot can fail (incorrect root device) if
> device names
> was changed.
> 
> Attached path fixes this issue. Please review and commit it

On Debian at least /dev is always a udev managed tmpfs, except in the
case you boot from a livecd and chroot of course.
I fail to see why the check if /dev/disk/by-uuid is a directory does fix
anything.
If /dev/disk/by-uuid/abc123 doestn't exit, then this implies
that /dev/disk/by-uuid is not a directory.
Or is this some bug in the test command we need to workaround?

-- 
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

Re: [patch] incorrect root device while /dev/ on tmpfs

[Colin Watson]

On Sat, Oct 17, 2009 at 02:50:41PM +0200, Felix Zielcke wrote:
> If /dev/disk/by-uuid/abc123 doestn't exit, then this implies
> that /dev/disk/by-uuid is not a directory.

I think you are misreading the original patch. Check it again ...

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: [patch] incorrect root device while /dev/ on tmpfs

[Felix Zielcke]

Am Samstag, den 17.10.2009, 13:56 +0100 schrieb Colin Watson:
> On Sat, Oct 17, 2009 at 02:50:41PM +0200, Felix Zielcke wrote:
> > If /dev/disk/by-uuid/abc123 doestn't exit, then this implies
> > that /dev/disk/by-uuid is not a directory.
> 
> I think you are misreading the original patch. Check it again ...
> 

Ok thanks to Vladimir on IRC I got it now.
But I don't think it's a good idea to use root=UUID=
if /dev/disk/by-uuid doestn't exist at all.

The only case I can imagine where this would work, would be booting from
a livecd, chrooting to the installed system, which still has the disk
devices in the static dev so you don't need to bind mount /dev.
But that should be only a one time situation.

If in the then booted system /dev/disk/by-uuid still doesn't exist, I
don't think the initrd supports root=UUID= and then every boot would
fail as long as GRUB_LINUX_DISABLE_UUID=true isn't set.

-- 
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer

Re: [patch] incorrect root device while /dev/ on tmpfs

[Colin Watson]

On Sat, Oct 17, 2009 at 04:24:53PM +0400, Vasily Averin wrote:
> While /dev mounted as tmpfs it does not contain /dev/disk/by-uuid directory,
> 10_linux script did not check it properly and do not set root device via UUID.
> Therefore following kernel boot can fail (incorrect root device) if device names
> was changed.
> 
> Attached path fixes this issue. Please review and commit it

> diff -up ./util/grub.d/10_linux.in.tmpdevuuid ./util/grub.d/10_linux.in
> --- ./util/grub.d/10_linux.in.tmpdevuuid	2009-08-29 20:05:06.000000000 +0400
> +++ ./util/grub.d/10_linux.in	2009-09-03 11:07:34.000000000 +0400
> @@ -36,7 +36,7 @@ case ${GRUB_DEVICE} in
>  esac
>  
>  if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
> -    || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" ; then
> +    || ( test -d /dev/disk/by-uuid  && ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}") ; then
>    LINUX_ROOT_DEVICE=${GRUB_DEVICE}
>  else
>    LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}

It seems to me that this eliminates the safety check to ensure that
Linux actually recognises this UUID. If for some reason GRUB and Linux
are out of sync in how they calculate UUIDs, this will probably result
in an unbootable system.

Would it help to double-check with blkid in this case, or something like
that?

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: [patch] incorrect root device while /dev/ on tmpfs

[Vladimir 'phcoder' Serbinenko]

Vasily Averin wrote:
> While /dev mounted as tmpfs it does not contain /dev/disk/by-uuid directory,
> 10_linux script did not check it properly and do not set root device via UUID.
> Therefore following kernel boot can fail (incorrect root device) if device names
> was changed.
>
>   
I would be inclined to say that for mainline presence of
/dev/disk/by-uuid is a reasonable indication that initrd supports root
specified by UUID. If it's not present then we should assume initrd is
dumb and doesn't recognises UUID= and just hardcode the device. The
distributors can modify it as they see fit.
> Attached path fixes this issue. Please review and commit it
>
> thank you,
> 	Vasily Averin
>
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git