Re: [GRUB PARTUUID PATCH V8 3/4] Add PARTUUID detection support to grub-probe
On Wed, Mar 28, 2018 at 08:32:53AM -0700, Nicholas Vinson wrote: > Add PARTUUID detection support grub-probe for MBR and GPT partition > schemes. > > Signed-off-by: Nicholas Vinson Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] F2FS support
On Thu, Mar 29, 2018 at 05:08:42PM +0100, Pete Batard wrote: > Hi Daniel, > > Thanks for reviewing the patch. > Here's a v2 that takes your comments into account. Thanks, LGTM +/- some nitpicks which I fix before committing. If there are no significant objections I will apply this patch in a week or so. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [GRUB PARTUUID PATCH V8 4/4] Update grub script template files
On Wed, Mar 28, 2018 at 08:32:54AM -0700, Nicholas Vinson wrote:
> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
> partuuid target. Update grub.texi documenation.
>
> Signed-off-by: Nicholas Vinson
> ---
> docs/grub.texi | 9 +
> util/grub-mkconfig.in | 3 +++
> util/grub.d/10_linux.in | 12 +---
> 3 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 65b4bbeda..019ce5d03 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -1424,6 +1424,15 @@ the Linux kernel, using a @samp{root=UUID=...} kernel
> parameter. This is
> usually more reliable, but in some cases it may not be appropriate. To
> disable the use of UUIDs, set this option to @samp{true}.
>
> +@item GRUB_ENABLE_LINUX_PARTUUID
> +Setting this option to @samp{true} changes the behavior of
> +@command{grub-mkconfig} so that it identifies the device containing the root
> +filesystem by the partition UUID via the @samp{root=PARTUUID=...} kernel
> +parameter. This is desirable for Linux systems where identifying the root
> +filesystem by its filesystem UUID or device name is inappropriate. However,
> +this option is only supported in Linux kernel versions 2.6.37 (3.10 for
> systems
> +using the MSDOS partition scheme) or newer.
> +
> @item GRUB_DISABLE_RECOVERY
> If this option is set to @samp{true}, disable the generation of recovery
> mode menu entries.
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index 35ef583b0..9a1f92bdf 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -134,6 +134,7 @@ fi
> # Device containing our userland. Typically used for root= parameter.
> GRUB_DEVICE="`${grub_probe} --target=device /`"
> GRUB_DEVICE_UUID="`${grub_probe} --device ${GRUB_DEVICE} --target=fs_uuid 2>
> /dev/null`" || true
> +GRUB_DEVICE_PARTUUID="`${grub_probe} --device ${GRUB_DEVICE}
> --target=partuuid 2> /dev/null`" || true
>
> # Device containing our /boot partition. Usually the same as GRUB_DEVICE.
> GRUB_DEVICE_BOOT="`${grub_probe} --target=device /boot`"
> @@ -188,6 +189,7 @@ if [ "x${GRUB_ACTUAL_DEFAULT}" = "xsaved" ] ; then
> GRUB_ACTUAL_DEFAULT="`"${grub
> # override them.
> export GRUB_DEVICE \
>GRUB_DEVICE_UUID \
> + GRUB_DEVICE_PARTUUID \
>GRUB_DEVICE_BOOT \
>GRUB_DEVICE_BOOT_UUID \
>GRUB_FS \
> @@ -223,6 +225,7 @@ export GRUB_DEFAULT \
>GRUB_TERMINAL_OUTPUT \
>GRUB_SERIAL_COMMAND \
>GRUB_DISABLE_LINUX_UUID \
> + GRUB_ENABLE_LINUX_PARTUUID \
>GRUB_DISABLE_RECOVERY \
>GRUB_VIDEO_BACKEND \
>GRUB_GFXMODE \
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index faedf74e1..53de33bea 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -45,12 +45,18 @@ 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}" \
> +if [ "x${GRUB_DEVICE_UUID}" = "x" ] && [ "x${GRUB_DEVICE_PARTUUID}" = "x" ] \
> +|| [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
Hmmm... Have you tested this patch with GRUB_DISABLE_LINUX_UUID=true and
GRUB_ENABLE_LINUX_PARTUUID=true? I do not think so. LINUX_ROOT_DEVICE will
be set to ${GRUB_DEVICE}. I have a feeling that this is not what you expect.
Additionally, what will happen if GRUB_DISABLE_LINUX_UUID=true and
GRUB_ENABLE_LINUX_PARTUUID=false? Well, this should work right now
but please test it after fixing above mentioned issue.
Should not we do s/GRUB_ENABLE_LINUX_PARTUUID/GRUB_DISABLE_LINUX_PARTUUID/?
Then the variable will have similar meaning to GRUB_DISABLE_LINUX_UUID.
Hence, all will be less confusing.
> +|| ( ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
> +&& ! test -e "/dev/disk/by-partuuid/${GRUB_DEVICE_PARTUUID}" ) \
> || ( test -e "${GRUB_DEVICE}" && uses_abstraction "${GRUB_DEVICE}" lvm
> ); then
>LINUX_ROOT_DEVICE=${GRUB_DEVICE}
> -else
> +elif [ "x${GRUB_ENABLE_LINUX_PARTUUID}" != "xtrue" ] \
> +|| [ "x${GRUB_DEVICE_PARTUUID}" = "x" ] \
> +|| ! test -e "/dev/disk/by-partuuid/${GRUB_DEVICE_PARTUUID}"; then
>LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
> +else
> + LINUX_ROOT_DEVICE=PARTUUID=${GRUB_DEVICE_PARTUUID}
> fi
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] F2FS support
On Sat, Mar 31, 2018 at 10:47:29PM +0200, Paul Menzel wrote: > Dear Pete, > > Am Donnerstag, den 29.03.2018, 17:08 +0100 schrieb Pete Batard: > > Thanks for reviewing the patch. > > Here's a v2 that takes your comments into account. > > Could you please make the commit message title a statement by adding a > verb? > > > Add F2FS support I will take that into account. > Could you additionally add an example to the commit message, how to > test the support with QEMU? That would help me, test your patch. I am not sure what do you mean by that. Anyway, IMO this request does not block patch itself. However, if you wish to provide an example I am happy to take it. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] F2FS support
On 2018.04.04 21:37, Daniel Kiper wrote: Thanks, LGTM +/- some nitpicks which I fix before committing. If there are no significant objections I will apply this patch in a week or so. Thanks Daniel. Much appreciated. Regards, /Pete ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [GRUB PARTUUID PATCH V8 4/4] Update grub script template files
On 04/04/2018 01:23 PM, Daniel Kiper wrote:
> On Wed, Mar 28, 2018 at 08:32:54AM -0700, Nicholas Vinson wrote:
>> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
>> partuuid target. Update grub.texi documenation.
>>
>> Signed-off-by: Nicholas Vinson
>> ---
>> docs/grub.texi | 9 +
>> util/grub-mkconfig.in | 3 +++
>> util/grub.d/10_linux.in | 12 +---
>> 3 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/grub.texi b/docs/grub.texi
>> index 65b4bbeda..019ce5d03 100644
>> --- a/docs/grub.texi
>> +++ b/docs/grub.texi
>> @@ -1424,6 +1424,15 @@ the Linux kernel, using a @samp{root=UUID=...} kernel
>> parameter. This is
>> usually more reliable, but in some cases it may not be appropriate. To
>> disable the use of UUIDs, set this option to @samp{true}.
>>
>> +@item GRUB_ENABLE_LINUX_PARTUUID
>> +Setting this option to @samp{true} changes the behavior of
>> +@command{grub-mkconfig} so that it identifies the device containing the root
>> +filesystem by the partition UUID via the @samp{root=PARTUUID=...} kernel
>> +parameter. This is desirable for Linux systems where identifying the root
>> +filesystem by its filesystem UUID or device name is inappropriate. However,
>> +this option is only supported in Linux kernel versions 2.6.37 (3.10 for
>> systems
>> +using the MSDOS partition scheme) or newer.
>> +
>> @item GRUB_DISABLE_RECOVERY
>> If this option is set to @samp{true}, disable the generation of recovery
>> mode menu entries.
>> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
>> index 35ef583b0..9a1f92bdf 100644
>> --- a/util/grub-mkconfig.in
>> +++ b/util/grub-mkconfig.in
>> @@ -134,6 +134,7 @@ fi
>> # Device containing our userland. Typically used for root= parameter.
>> GRUB_DEVICE="`${grub_probe} --target=device /`"
>> GRUB_DEVICE_UUID="`${grub_probe} --device ${GRUB_DEVICE} --target=fs_uuid
>> 2> /dev/null`" || true
>> +GRUB_DEVICE_PARTUUID="`${grub_probe} --device ${GRUB_DEVICE}
>> --target=partuuid 2> /dev/null`" || true
>>
>> # Device containing our /boot partition. Usually the same as GRUB_DEVICE.
>> GRUB_DEVICE_BOOT="`${grub_probe} --target=device /boot`"
>> @@ -188,6 +189,7 @@ if [ "x${GRUB_ACTUAL_DEFAULT}" = "xsaved" ] ; then
>> GRUB_ACTUAL_DEFAULT="`"${grub
>> # override them.
>> export GRUB_DEVICE \
>>GRUB_DEVICE_UUID \
>> + GRUB_DEVICE_PARTUUID \
>>GRUB_DEVICE_BOOT \
>>GRUB_DEVICE_BOOT_UUID \
>>GRUB_FS \
>> @@ -223,6 +225,7 @@ export GRUB_DEFAULT \
>>GRUB_TERMINAL_OUTPUT \
>>GRUB_SERIAL_COMMAND \
>>GRUB_DISABLE_LINUX_UUID \
>> + GRUB_ENABLE_LINUX_PARTUUID \
>>GRUB_DISABLE_RECOVERY \
>>GRUB_VIDEO_BACKEND \
>>GRUB_GFXMODE \
>> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
>> index faedf74e1..53de33bea 100644
>> --- a/util/grub.d/10_linux.in
>> +++ b/util/grub.d/10_linux.in
>> @@ -45,12 +45,18 @@ 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}" \
>> +if [ "x${GRUB_DEVICE_UUID}" = "x" ] && [ "x${GRUB_DEVICE_PARTUUID}" = "x" ]
>> \
>> +|| [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
>
> Hmmm... Have you tested this patch with GRUB_DISABLE_LINUX_UUID=true and
> GRUB_ENABLE_LINUX_PARTUUID=true? I do not think so. LINUX_ROOT_DEVICE will
> be set to ${GRUB_DEVICE}. I have a feeling that this is not what you expect.
The idea was for GRUB_DISABLE_LINUX_UUID to act as a controlling
variable with this setup.
I reviewed the description for the variable, and having it act that way
doesn't make sense. I will rewrite this part and make sure the two
variables act as independently as possible.
The one exception, however, would be when GRUB_DISABLE_LINUX_UUID is
unset and GRUB_ENABLE_LINUX_PARTUUID is set. In such a case, I'll have
the code favor the fs UUID over the partition UUID. I think this would
cause the least amount of surprise since the initramfs images I've seen
prefer the fs UUID.
>
> Additionally, what will happen if GRUB_DISABLE_LINUX_UUID=true and
> GRUB_ENABLE_LINUX_PARTUUID=false? Well, this should work right now
> but please test it after fixing above mentioned issue.
It should favor the Linux device name. I'll make sure that behavior is
preserved.
>
> Should not we do s/GRUB_ENABLE_LINUX_PARTUUID/GRUB_DISABLE_LINUX_PARTUUID/?
> Then the variable will have similar meaning to GRUB_DISABLE_LINUX_UUID.
> Hence, all will be less confusing.
I can return the name to GRUB_DISABLE_LINUX_PARTUUID. When I originally
used that name, there was concerns about maintaining compatibility with
older kernel versions. However, I think I can use the name
GRUB_DISABLE_LINUX_PARTUUID while maintaining that compatibility.
Thanks,
Nicholas Vinson
