[PATCH] 10_linux: avoid multi-device root= kernel argument

[PATCH] 10_linux: avoid multi-device root= kernel argument

[Andrei Borzenkov]

If root filesystem is multidev btrfs, do not attempt to pass all devices as
kernel root= argument. This results in splitting command line in GRUB due to
embedded newline and even if we managed to quote it, kernel does not know how
to interpret it anyway. Multidev btrfs requires user space device scanning,
so passing single device would not work too.

This still respects user settings GRUB_DISABLE_LINUX_UUID. Not sure what we
should do in this case.

Closes: 45709

---
 util/grub.d/10_linux.in     | 4 +++-
 util/grub.d/20_linux_xen.in | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 859b608..5a78513 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -43,9 +43,11 @@ case ${GRUB_DEVICE} in
   ;;
 esac
 
+# btrfs may reside on multiple devices. We cannot pass them as value of root= parameter
+# and mounting btrfs requires user space scanning, so force UUID in this case.
 if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
     || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
-    || uses_abstraction "${GRUB_DEVICE}" lvm; then
+    || test -e "${GRUB_DEVICE}" && uses_abstraction "${GRUB_DEVICE}" lvm; then
   LINUX_ROOT_DEVICE=${GRUB_DEVICE}
 else
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index f532fb9..46045db 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -43,9 +43,11 @@ case ${GRUB_DEVICE} in
   ;;
 esac
 
+# btrfs may reside on multiple devices. We cannot pass them as value of root= parameter
+# and mounting btrfs requires user space scanning, so force UUID in this case.
 if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
     || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
-    || uses_abstraction "${GRUB_DEVICE}" lvm; then
+    || test -e "${GRUB_DEVICE}" && uses_abstraction "${GRUB_DEVICE}" lvm; then
   LINUX_ROOT_DEVICE=${GRUB_DEVICE}
 else
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
-- 
tg: (ff84a9b..) u/btrfs-multidev (depends on: master)

Re: [PATCH] 10_linux: avoid multi-device root= kernel argument

[Michael Chang]

On Thu, Jan 28, 2016 at 07:46:41PM +0300, Andrei Borzenkov wrote:
> If root filesystem is multidev btrfs, do not attempt to pass all devices as
> kernel root= argument. This results in splitting command line in GRUB due to
> embedded newline and even if we managed to quote it, kernel does not know how
> to interpret it anyway. Multidev btrfs requires user space device scanning,
> so passing single device would not work too.
> 
> This still respects user settings GRUB_DISABLE_LINUX_UUID. Not sure what we
> should do in this case.
> 
> Closes: 45709

JFYI, this patch also has verified to solve a reported installation
problem in Btrfs RAID 0 configuration in SUSE Linux. It would be great
to have this patch included in upstream.

Thanks,
Michael

> 
> ---
>  util/grub.d/10_linux.in     | 4 +++-
>  util/grub.d/20_linux_xen.in | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index 859b608..5a78513 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -43,9 +43,11 @@ case ${GRUB_DEVICE} in
>    ;;
>  esac
>  
> +# btrfs may reside on multiple devices. We cannot pass them as value of root= parameter
> +# and mounting btrfs requires user space scanning, so force UUID in this case.
>  if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
>      || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
> -    || uses_abstraction "${GRUB_DEVICE}" lvm; then
> +    || test -e "${GRUB_DEVICE}" && uses_abstraction "${GRUB_DEVICE}" lvm; then
>    LINUX_ROOT_DEVICE=${GRUB_DEVICE}
>  else
>    LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
> index f532fb9..46045db 100644
> --- a/util/grub.d/20_linux_xen.in
> +++ b/util/grub.d/20_linux_xen.in
> @@ -43,9 +43,11 @@ case ${GRUB_DEVICE} in
>    ;;
>  esac
>  
> +# btrfs may reside on multiple devices. We cannot pass them as value of root= parameter
> +# and mounting btrfs requires user space scanning, so force UUID in this case.
>  if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
>      || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
> -    || uses_abstraction "${GRUB_DEVICE}" lvm; then
> +    || test -e "${GRUB_DEVICE}" && uses_abstraction "${GRUB_DEVICE}" lvm; then
>    LINUX_ROOT_DEVICE=${GRUB_DEVICE}
>  else
>    LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
> -- 
> tg: (ff84a9b..) u/btrfs-multidev (depends on: master)
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] 10_linux: avoid multi-device root= kernel argument

[Olaf Hering]

Am 28.01.2016 um 17:46 schrieb Andrei Borzenkov:
> This still respects user settings GRUB_DISABLE_LINUX_UUID. Not sure what we
> should do in this case.

grub cant possibly know what the device name of the target OS is. This
is especially true if it tries to guess what the OS on partitions other
than the current / might expect. So:
never append any root= or resume= to the OS cmdline. Its the task of the
initrd to find the relevant block devices.

Olaf

Re: [PATCH] 10_linux: avoid multi-device root= kernel argument

[Andrei Borzenkov]

On Fri, Jan 29, 2016 at 11:54 AM, Olaf Hering <olaf@aepfle.de> wrote:
> Am 28.01.2016 um 17:46 schrieb Andrei Borzenkov:
>> This still respects user settings GRUB_DISABLE_LINUX_UUID. Not sure what we
>> should do in this case.
>
> grub cant possibly know what the device name of the target OS is. This
> is especially true if it tries to guess what the OS on partitions other
> than the current / might expect. So:

it never does it for partitions other than current.

> never append any root= or resume= to the OS cmdline. Its the task of the
> initrd to find the relevant block devices.
>
> Olaf
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] 10_linux: avoid multi-device root= kernel argument

[Olaf Hering]

On Fri, Jan 29, Andrei Borzenkov wrote:

> On Fri, Jan 29, 2016 at 11:54 AM, Olaf Hering <olaf@aepfle.de> wrote:
> > Am 28.01.2016 um 17:46 schrieb Andrei Borzenkov:
> >> This still respects user settings GRUB_DISABLE_LINUX_UUID. Not sure what we
> >> should do in this case.
> >
> > grub cant possibly know what the device name of the target OS is. This
> > is especially true if it tries to guess what the OS on partitions other
> > than the current / might expect. So:
> 
> it never does it for partitions other than current.

/etc/grub.d/30_os-prober does it. The other OS is configured to mount
by label but grub forces an incorrect root= into cmdline. For short:
rework the patch to stop forcing root= into cmdline.

Olaf

Re: [PATCH] 10_linux: avoid multi-device root= kernel argument

[Andrei Borzenkov]

29.01.2016 19:23, Olaf Hering пишет:
> On Fri, Jan 29, Andrei Borzenkov wrote:
> 
>> On Fri, Jan 29, 2016 at 11:54 AM, Olaf Hering <olaf@aepfle.de> wrote:
>>> Am 28.01.2016 um 17:46 schrieb Andrei Borzenkov:
>>>> This still respects user settings GRUB_DISABLE_LINUX_UUID. Not sure what we
>>>> should do in this case.
>>>
>>> grub cant possibly know what the device name of the target OS is. This
>>> is especially true if it tries to guess what the OS on partitions other
>>> than the current / might expect. So:
>>
>> it never does it for partitions other than current.
> 
> /etc/grub.d/30_os-prober does it. The other OS is configured to mount

I fail to find string 30_os-prober in my patch. If there is problem with
30_os-prober, please start another discussion. And at least explain what
the problem is.

> by label but grub forces an incorrect root= into cmdline. For short:
> rework the patch to stop forcing root= into cmdline.
> 

Re: [PATCH] 10_linux: avoid multi-device root= kernel argument

[Juan Orti Alcaine]

2016-01-28 17:46 GMT+01:00 Andrei Borzenkov <arvidjaar@gmail.com>:
> If root filesystem is multidev btrfs, do not attempt to pass all devices as
> kernel root= argument. This results in splitting command line in GRUB due to
> embedded newline and even if we managed to quote it, kernel does not know how
> to interpret it anyway. Multidev btrfs requires user space device scanning,
> so passing single device would not work too.
>
> This still respects user settings GRUB_DISABLE_LINUX_UUID. Not sure what we
> should do in this case.
>
> Closes: 45709
>

This patch fixes my problem reported here:
https://bugzilla.redhat.com/show_bug.cgi?id=1294532
Now, the kernel root option is correctly generated as root=UUID=xxxx
instead of the broken multiline list of devices as before.

Thank you.

Re: [PATCH] 10_linux: avoid multi-device root= kernel argument

[Andrei Borzenkov]

01.02.2016 00:17, Juan Orti Alcaine пишет:
> 2016-01-28 17:46 GMT+01:00 Andrei Borzenkov <arvidjaar@gmail.com>:
>> If root filesystem is multidev btrfs, do not attempt to pass all devices as
>> kernel root= argument. This results in splitting command line in GRUB due to
>> embedded newline and even if we managed to quote it, kernel does not know how
>> to interpret it anyway. Multidev btrfs requires user space device scanning,
>> so passing single device would not work too.
>>
>> This still respects user settings GRUB_DISABLE_LINUX_UUID. Not sure what we
>> should do in this case.
>>
>> Closes: 45709
>>
> 
> This patch fixes my problem reported here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1294532
> Now, the kernel root option is correctly generated as root=UUID=xxxx
> instead of the broken multiline list of devices as before.
> 
> Thank you.
> 
Thank you and Michael for testing. Pushed.

[PATCH] 10_linux: avoid multi-device root= kernel argument

[Andrei Borzenkov]

If root filesystem is multidev btrfs, do not attempt to pass all devices as
kernel root= argument. This results in splitting command line in GRUB due to
embedded newline. Multidev btrfs requires user space device scanning,
so passing single device makes no sense as well.

It means we may end up without any root= argument if either UUIDs are
not available or user explicitly disabled them.

Closes: 45709

---
 util/grub.d/10_linux.in     | 7 ++++++-
 util/grub.d/20_linux_xen.in | 7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 859b608..a3a4624 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -43,10 +43,15 @@ case ${GRUB_DEVICE} in
   ;;
 esac
 
+# btrfs may reside on multiple devices. We cannot pass them as value of root= parameter
+# and mounting btrfs requires user space scanning, so leave root= empty in this case.
+# We hope that whatever initramfs is used knows how to mount root.
 if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
     || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
     || uses_abstraction "${GRUB_DEVICE}" lvm; then
-  LINUX_ROOT_DEVICE=${GRUB_DEVICE}
+  if test -e "${GRUB_DEVICE}" ; then
+    LINUX_ROOT_DEVICE=${GRUB_DEVICE}
+  fi
 else
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
 fi
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index f532fb9..761d289 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -43,10 +43,15 @@ case ${GRUB_DEVICE} in
   ;;
 esac
 
+# btrfs may reside on multiple devices. We cannot pass them as value of root= parameter
+# and mounting btrfs requires user space scanning, so leave root= empty in this case.
+# We hope that whatever initramfs is used knows how to mount root.
 if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
     || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
     || uses_abstraction "${GRUB_DEVICE}" lvm; then
-  LINUX_ROOT_DEVICE=${GRUB_DEVICE}
+  if test -e "${GRUB_DEVICE}" ; then
+    LINUX_ROOT_DEVICE=${GRUB_DEVICE}
+  fi
 else
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
 fi
-- 
tg: (ba83ed1..) u/btrfs-multidev (depends on: master)