[PATCH] diskboot: trivial correction on stale comments

2018-04-10 Thread Cao jin 
diskboot.img now is loaded at 0x8000 and is jumped to with 0:0x8000.

Signed-off-by: Cao jin 
---
 grub-core/boot/i386/pc/diskboot.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/grub-core/boot/i386/pc/diskboot.S 
b/grub-core/boot/i386/pc/diskboot.S
index 1ee4cf5b2..c1addc0df 100644
--- a/grub-core/boot/i386/pc/diskboot.S
+++ b/grub-core/boot/i386/pc/diskboot.S
@@ -37,8 +37,8 @@
 start:
 _start:
/*
-* _start is loaded at 0x2000 and is jumped to with
-* CS:IP 0:0x2000 in kernel.
+* _start is loaded at 0x8000 and is jumped to with
+* CS:IP 0:0x8000 in kernel.
 */
 
/*
-- 
2.14.3




___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: stale comment in diskboot.S ?

2018-04-10 Thread Cao jin 
Hi Daniel,

On 04/11/2018 04:25 AM, Daniel Kiper wrote:
> On Wed, Mar 28, 2018 at 04:09:00PM +0800, Cao jin wrote:
>> Hi,
>>
>> I was reading the code, and I want to confirm is following comment is stale?
>>
>>  /*
>>   * _start is loaded at 0x2000 and is jumped to with
>>   * CS:IP 0:0x2000 in kernel.
>>   */
>>
>> As I understand, it is loaded at 0x8000 and is jumped to with 0:0x8000.
> 
> It looks that you are right. Could you fix that comment?
> I am happy to take a patch for it.
> 

Sure, patch is on the way.

-- 
Sincerely,
Cao jin



___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V9 0/5] Add PARTUUID detection support

2018-04-10 Thread Nick Vinson 
On 04/10/2018 01:56 PM, Daniel Kiper wrote:
> On Sat, Apr 07, 2018 at 04:28:09PM -0700, Nicholas Vinson wrote:
>> Changes from Patch v8:
>> - Renamed GRUB_ENABLE_LINUX_PARTUUID to GRUB_DISABLE_LINUX_PARTUUID
>> - Updated the 10_linux logic so GRUB_ENABLE_LINUX_PARTUUID and
>> GRUB_ENABLE_LINUX_UUID would behave more independently.
>> - Documented interactions of GRUB_DISABLE_LINUX_UUID,
>> GRUB_DISABLE_LINUX_PARTUUID, and initramfs detection in commit
>> message.
>> - Added optional patch that sets GRUB_DISABLE_LINUX_PARTUUID to true
>> by default.
>> - Fixed merge conflicts between this patchset and upstream's master
>> branch.
> 
> Could you provide summary for current version not only for previous ones?

I can add a summary section before the change log.  I'll make sure to do
so with the next patchset.

Thanks,
Nicholas Vinson

> 
> Anyway, I think that most of the patches are now in good shape.
> I am looking forward for next version.
> 
> Thank you for doing the work.
> 
> Daniel
> 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V9 4/5] Update grub script template files

2018-04-10 Thread Nick Vinson 


On 04/10/2018 01:52 PM, Daniel Kiper wrote:
> On Sat, Apr 07, 2018 at 04:28:13PM -0700, Nicholas Vinson wrote:
>> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
>> partuuid target.  Update grub.texi documentation.  The following table
>> shows how GRUB_DISABLE_LINUX_UUID, GRUB_DISABLE_LINUX_PARTUUID, and
>> initramfs detection interact:
>>
>> Initramfs  GRUB_DISABLE_LINUX_PARTUUID  GRUB_DISABLE_LINUX_UUID  Linux Root
>> detected   Set  Set  ID Method
>>
>> False  FalseFalsepart UUID
>> False  FalseTrue part UUID
>> False  True Falsedev name
>> False  True True dev name
>> True   FalseFalsefs UUID
>> True   FalseTrue part UUID
>> True   True Falsefs UUID
>> True   True True dev name
> 
> What will happen if GRUB_DISABLE_LINUX_PARTUUID and/or GRUB_DISABLE_LINUX_UUID
> are not set? I think that you can avoid that by setting defaults. You do that
> for GRUB_DISABLE_LINUX_PARTUUID in next patch but GRUB_DISABLE_LINUX_UUID
> does not have any default.
> 

If they're not set, then that's the same as them being set to 'False'.
I should have worded my table above a bit differently and used Yes/No
instead of True/False as that is really what it is trying to convey.
I.E. If there is no initramfs detected and GRUB_DISABLE_LINUX_PARTUUID
is not set and GRUB_DISABLE_LINUX_UUID is not set, set the kernel root
variable to "root=PARTUUID=...".

In the scripts, the only value explicitly checked for is 'true'.
Anything else (including unset) is considered false.

>> Signed-off-by: Nicholas Vinson 
>> ---
>>  docs/grub.texi  | 10 ++
>>  util/grub-mkconfig.in   |  3 +++
>>  util/grub.d/10_linux.in | 18 +++---
> 
> Could you update util/grub.d/20_linux_xen.in too?

Let me see what I can do.  I have never worked with xen before, so I do
not know much about it.

> 
>>  3 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/grub.texi b/docs/grub.texi
>> index 65b4bbeda..06f0afe45 100644
>> --- a/docs/grub.texi
>> +++ b/docs/grub.texi
>> @@ -1424,6 +1424,16 @@ 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_DISABLE_LINUX_PARTUUID
>> +If @command{grub-mkconfig} cannot identify the root filesystem via its
>> +universally-unique indentifier (UUID), @command{grub-mkconfig} will use the 
>> UUID
>> +of the partition containing the filesystem to identify the root filesystem 
>> to
>> +the Linux kernel via a @samp{root=PARTUUID=...} kernel parameter.  This is 
>> not
>> +as reliable as using the filesystem UUID, but is more reliable than using 
>> the
>> +Linux device names.  When enabled, this option requires the Linux kernel 
>> version
> 
> s/When enabled, /When enabled, GRUB_DISABLE_LINUX_PARTUUID set to false, /
> or something like that. Otherwise it is unclear.

I'll clean up the wording in the next release.

Thanks,
Nicholas Vinson

> 
> Daniel
> 

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Checksummed environments

2018-04-10 Thread Daniel Kiper 
On Fri, Apr 06, 2018 at 03:08:23PM +0200, Kristian Amlie wrote:
> On 06/04/18 14:35, Daniel Kiper wrote:
> > On Fri, Apr 06, 2018 at 11:25:22AM +0200, Kristian Amlie wrote:
> >> Hey, I work for Northern.tech, developing update software for embedded
> >> Linux devices.
> >>
> >> I have a question about GRUB's environment block: This block is not
> >
> > I am not sure what exactly you mean by "GRUB's environment block".
> > Could you send me some references to the code?
>
> Of course, sorry if the context wasn't clear. I'm talking about the
> environment block mentioned here:
> https://www.gnu.org/software/grub/manual/grub/grub.html#Environment-block,
> which is used to store information from one boot to the next, using
> save_env and load_env.
>
> >> checksummed, and hence I reckon it can become corrupt if power is lost
> >> in the middle of a write.
> >
> > What about the other blocks?
>
> In theory, there is only one block, because it is written in-place,
> directly on disk, without changing any other filesystem blocks. Is that
> what you meant by "other blocks"?
>
> >> This is an important safety criterion for us, so we've been thinking of
> >> developing environment block checksumming as an extension to the
> >> existing save_env and load_env commands. The most likely approach will
> >> be to grab X amount of bytes at the end of the block and use these for
> >> the checksum.
> >
> > Could you tell us more about that?
>
> The idea comes from U-Boot [1], which uses a dedicated block on a
> storage device to store data, and uses a CRC32 checksum to make sure it
> is consistent. The motivation is to be able to detect that the block is
> corrupt, rather than accepting a block of data that may have incorrect
> data in if a write was interrupted midway by a powerloss. You can read
> more about it in the link.
>
> [2] https://www.denx.de/wiki/DULG/UBootEnvVariables

I am OK with the idea. However, I think that the feature should have
a kind of switch to turn it off/on. At first sight it looks that new
environment variable is sufficient for it.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V9 0/5] Add PARTUUID detection support

2018-04-10 Thread Daniel Kiper 
On Sat, Apr 07, 2018 at 04:28:09PM -0700, Nicholas Vinson wrote:
> Changes from Patch v8:
> - Renamed GRUB_ENABLE_LINUX_PARTUUID to GRUB_DISABLE_LINUX_PARTUUID
> - Updated the 10_linux logic so GRUB_ENABLE_LINUX_PARTUUID and
> GRUB_ENABLE_LINUX_UUID would behave more independently.
> - Documented interactions of GRUB_DISABLE_LINUX_UUID,
> GRUB_DISABLE_LINUX_PARTUUID, and initramfs detection in commit
> message.
> - Added optional patch that sets GRUB_DISABLE_LINUX_PARTUUID to true
> by default.
> - Fixed merge conflicts between this patchset and upstream's master
> branch.

Could you provide summary for current version not only for previous ones?

Anyway, I think that most of the patches are now in good shape.
I am looking forward for next version.

Thank you for doing the work.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [GRUB PARTUUID PATCH V9 4/5] Update grub script template files

2018-04-10 Thread Daniel Kiper 
On Sat, Apr 07, 2018 at 04:28:13PM -0700, Nicholas Vinson wrote:
> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
> partuuid target.  Update grub.texi documentation.  The following table
> shows how GRUB_DISABLE_LINUX_UUID, GRUB_DISABLE_LINUX_PARTUUID, and
> initramfs detection interact:
>
> Initramfs  GRUB_DISABLE_LINUX_PARTUUID  GRUB_DISABLE_LINUX_UUID  Linux Root
> detected   Set  Set  ID Method
>
> False  FalseFalsepart UUID
> False  FalseTrue part UUID
> False  True Falsedev name
> False  True True dev name
> True   FalseFalsefs UUID
> True   FalseTrue part UUID
> True   True Falsefs UUID
> True   True True dev name

What will happen if GRUB_DISABLE_LINUX_PARTUUID and/or GRUB_DISABLE_LINUX_UUID
are not set? I think that you can avoid that by setting defaults. You do that
for GRUB_DISABLE_LINUX_PARTUUID in next patch but GRUB_DISABLE_LINUX_UUID
does not have any default.

> Signed-off-by: Nicholas Vinson 
> ---
>  docs/grub.texi  | 10 ++
>  util/grub-mkconfig.in   |  3 +++
>  util/grub.d/10_linux.in | 18 +++---

Could you update util/grub.d/20_linux_xen.in too?

>  3 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 65b4bbeda..06f0afe45 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -1424,6 +1424,16 @@ 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_DISABLE_LINUX_PARTUUID
> +If @command{grub-mkconfig} cannot identify the root filesystem via its
> +universally-unique indentifier (UUID), @command{grub-mkconfig} will use the 
> UUID
> +of the partition containing the filesystem to identify the root filesystem to
> +the Linux kernel via a @samp{root=PARTUUID=...} kernel parameter.  This is 
> not
> +as reliable as using the filesystem UUID, but is more reliable than using the
> +Linux device names.  When enabled, this option requires the Linux kernel 
> version

s/When enabled, /When enabled, GRUB_DISABLE_LINUX_PARTUUID set to false, /
or something like that. Otherwise it is unclear.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: stale comment in diskboot.S ?

2018-04-10 Thread Daniel Kiper 
On Wed, Mar 28, 2018 at 04:09:00PM +0800, Cao jin wrote:
> Hi,
>
> I was reading the code, and I want to confirm is following comment is stale?
>
>   /*
>* _start is loaded at 0x2000 and is jumped to with
>* CS:IP 0:0x2000 in kernel.
>*/
>
> As I understand, it is loaded at 0x8000 and is jumped to with 0:0x8000.

It looks that you are right. Could you fix that comment?
I am happy to take a patch for it.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel