[PATCH 1/1] add --partuuid to probe

[PATCH 1/1] add --partuuid to probe

[Steve Kenton]

Support both EFI and NT Disk Signature for passing to kernel as root=PARTUUID=$val

Signed-off-by: Steve Kenton <skenton@ou.edu>
---
It's been six months so I thought I'd resend this so it does not get lost
in case I get hit by a meteor or something before the next release

 grub-core/commands/probe.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
index cf2793e..3afc8b8 100644
--- a/grub-core/commands/probe.c
+++ b/grub-core/commands/probe.c
@@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
     {"fs",		'f', 0, N_("Determine filesystem type."), 0, 0},
     {"fs-uuid",		'u', 0, N_("Determine filesystem UUID."), 0, 0},
     {"label",		'l', 0, N_("Determine filesystem label."), 0, 0},
+    {"partuuid",	'g', 0, N_("Determine partition GUID/UUID."), 0, 0}, /* GUID but Linux kernel calls it "PARTUUID" */
     {0, 0, 0, 0, 0, 0}
   };
 
@@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char **args)
       grub_device_close (dev);
       return GRUB_ERR_NONE;
     }
+  if (state[6].set)
+    {
+      char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
+      grub_uint8_t diskbuf[16];
+      if (dev->disk && dev->disk->partition)
+	{
+	  grub_partition_t p = dev->disk->partition;
+	  if (!grub_strcmp (p->partmap->name, "msdos"))
+	    {
+	      const int diskid_offset = 440; /* location in MBR */
+	      dev->disk->partition = p->parent;
+	      /* little-endian 4-byte NT disk signature */
+	      err = grub_disk_read (dev->disk, 0, diskid_offset, 4, diskbuf);
+	      dev->disk->partition = p;
+	      if (err)
+	        return grub_errno;
+	      partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
+					 diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
+					 p->number + 1); /* one based partition number */
+	    }
+	  else if (!grub_strcmp (p->partmap->name, "gpt"))
+	    {
+	      const int guid_offset = 16; /* location in entry */
+	      dev->disk->partition = p->parent;
+	      /* little-endian 16-byte EFI partition GUID */
+	      err = grub_disk_read (dev->disk, p->offset, p->index + guid_offset, 16, diskbuf);
+	      dev->disk->partition = p;
+	      if (err)
+	        return grub_errno;
+	      partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+					 diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
+					 diskbuf[5], diskbuf[4],
+					 diskbuf[7], diskbuf[6],
+					 diskbuf[8], diskbuf[9],
+					 diskbuf[10], diskbuf[11], diskbuf[12], diskbuf[13], diskbuf[14], diskbuf[15]);
+	    }
+	  else
+	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			       N_("partition map %s does not support partition UUIDs"),
+			       dev->disk->partition->partmap->name);
+	}
+      else
+	partuuid = grub_strdup (""); /* a freeable empty string */
+
+      if (state[0].set)
+	grub_env_set (state[0].arg, partuuid);
+      else
+	grub_printf ("%s", partuuid);
+      grub_free (partuuid);
+      grub_device_close (dev);
+      return GRUB_ERR_NONE;
+    }
   grub_device_close (dev);
   return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
 }
-- 
2.9.0.137.gcf4c2cf

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

14.02.2017 21:00, Steve Kenton пишет:
> Support both EFI and NT Disk Signature for passing to kernel as root=PARTUUID=$val
> 

Yes, I guess we need to add it finally. Unfortunately it is too late for
2.02, but it should go after in release. There were also patches for
grub-probe and we also need to support it in search to be on par with
filesystem UUID. May be if you could prepare consolidated patch set it
would be great.

I apologize that it tool so long. Thank you for not giving up!

> Signed-off-by: Steve Kenton <skenton@ou.edu>
> ---
> It's been six months so I thought I'd resend this so it does not get lost
> in case I get hit by a meteor or something before the next release
> 
>  grub-core/commands/probe.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index cf2793e..3afc8b8 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c

Please also add patch for manual.

> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
>      {"fs",		'f', 0, N_("Determine filesystem type."), 0, 0},
>      {"fs-uuid",		'u', 0, N_("Determine filesystem UUID."), 0, 0},
>      {"label",		'l', 0, N_("Determine filesystem label."), 0, 0},
> +    {"partuuid",	'g', 0, N_("Determine partition GUID/UUID."), 0, 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>      {0, 0, 0, 0, 0, 0}
>    };
>  
> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char **args)
>        grub_device_close (dev);
>        return GRUB_ERR_NONE;
>      }
> +  if (state[6].set)
> +    {
> +      char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
> +      grub_uint8_t diskbuf[16];
> +      if (dev->disk && dev->disk->partition)
> +	{
> +	  grub_partition_t p = dev->disk->partition;
> +	  if (!grub_strcmp (p->partmap->name, "msdos"))
> +	    {
> +	      const int diskid_offset = 440; /* location in MBR */
> +	      dev->disk->partition = p->parent;
> +	      /* little-endian 4-byte NT disk signature */
> +	      err = grub_disk_read (dev->disk, 0, diskid_offset, 4, diskbuf);
> +	      dev->disk->partition = p;
> +	      if (err)
> +	        return grub_errno;
> +	      partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
> +					 diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
> +					 p->number + 1); /* one based partition number */
> +	    }
> +	  else if (!grub_strcmp (p->partmap->name, "gpt"))
> +	    {
> +	      const int guid_offset = 16; /* location in entry */
> +	      dev->disk->partition = p->parent;
> +	      /* little-endian 16-byte EFI partition GUID */
> +	      err = grub_disk_read (dev->disk, p->offset, p->index + guid_offset, 16, diskbuf);
> +	      dev->disk->partition = p;
> +	      if (err)
> +	        return grub_errno;
> +	      partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> +					 diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
> +					 diskbuf[5], diskbuf[4],
> +					 diskbuf[7], diskbuf[6],
> +					 diskbuf[8], diskbuf[9],
> +					 diskbuf[10], diskbuf[11], diskbuf[12], diskbuf[13], diskbuf[14], diskbuf[15]);
> +	    }
> +	  else
> +	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> +			       N_("partition map %s does not support partition UUIDs"),
> +			       dev->disk->partition->partmap->name);
> +	}
> +      else
> +	partuuid = grub_strdup (""); /* a freeable empty string */
> +
> +      if (state[0].set)
> +	grub_env_set (state[0].arg, partuuid);
> +      else
> +	grub_printf ("%s", partuuid);
> +      grub_free (partuuid);
> +      grub_device_close (dev);
> +      return GRUB_ERR_NONE;
> +    }
>    grub_device_close (dev);
>    return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>  }
> 

Re: [PATCH 1/1] add --partuuid to probe

[Steve Kenton]

Nick,

What is the state of your patch set? I think mine is much smaller if you 
want to roll it into yours and resubmit. We were told all along to wait 
until after 2.02, which should to be very soon, so it looks like it's 
show time!

Steve Kenton


On 02/14/2017 07:12 PM, Andrei Borzenkov wrote:
> 14.02.2017 21:00, Steve Kenton пишет:
>> Support both EFI and NT Disk Signature for passing to kernel as root=PARTUUID=$val
>>
> Yes, I guess we need to add it finally. Unfortunately it is too late for
> 2.02, but it should go after in release. There were also patches for
> grub-probe and we also need to support it in search to be on par with
> filesystem UUID. May be if you could prepare consolidated patch set it
> would be great.
>
> I apologize that it tool so long. Thank you for not giving up!
>
>> Signed-off-by: Steve Kenton <skenton@ou.edu>
>> ---
>> It's been six months so I thought I'd resend this so it does not get lost
>> in case I get hit by a meteor or something before the next release
>>
>>   grub-core/commands/probe.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>
>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>> index cf2793e..3afc8b8 100644
>> --- a/grub-core/commands/probe.c
>> +++ b/grub-core/commands/probe.c
> Please also add patch for manual.
>
>> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
>>       {"fs",		'f', 0, N_("Determine filesystem type."), 0, 0},
>>       {"fs-uuid",		'u', 0, N_("Determine filesystem UUID."), 0, 0},
>>       {"label",		'l', 0, N_("Determine filesystem label."), 0, 0},
>> +    {"partuuid",	'g', 0, N_("Determine partition GUID/UUID."), 0, 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>>       {0, 0, 0, 0, 0, 0}
>>     };
>>   
>> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char **args)
>>         grub_device_close (dev);
>>         return GRUB_ERR_NONE;
>>       }
>> +  if (state[6].set)
>> +    {
>> +      char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
>> +      grub_uint8_t diskbuf[16];
>> +      if (dev->disk && dev->disk->partition)
>> +	{
>> +	  grub_partition_t p = dev->disk->partition;
>> +	  if (!grub_strcmp (p->partmap->name, "msdos"))
>> +	    {
>> +	      const int diskid_offset = 440; /* location in MBR */
>> +	      dev->disk->partition = p->parent;
>> +	      /* little-endian 4-byte NT disk signature */
>> +	      err = grub_disk_read (dev->disk, 0, diskid_offset, 4, diskbuf);
>> +	      dev->disk->partition = p;
>> +	      if (err)
>> +	        return grub_errno;
>> +	      partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
>> +					 diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
>> +					 p->number + 1); /* one based partition number */
>> +	    }
>> +	  else if (!grub_strcmp (p->partmap->name, "gpt"))
>> +	    {
>> +	      const int guid_offset = 16; /* location in entry */
>> +	      dev->disk->partition = p->parent;
>> +	      /* little-endian 16-byte EFI partition GUID */
>> +	      err = grub_disk_read (dev->disk, p->offset, p->index + guid_offset, 16, diskbuf);
>> +	      dev->disk->partition = p;
>> +	      if (err)
>> +	        return grub_errno;
>> +	      partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
>> +					 diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
>> +					 diskbuf[5], diskbuf[4],
>> +					 diskbuf[7], diskbuf[6],
>> +					 diskbuf[8], diskbuf[9],
>> +					 diskbuf[10], diskbuf[11], diskbuf[12], diskbuf[13], diskbuf[14], diskbuf[15]);
>> +	    }
>> +	  else
>> +	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
>> +			       N_("partition map %s does not support partition UUIDs"),
>> +			       dev->disk->partition->partmap->name);
>> +	}
>> +      else
>> +	partuuid = grub_strdup (""); /* a freeable empty string */
>> +
>> +      if (state[0].set)
>> +	grub_env_set (state[0].arg, partuuid);
>> +      else
>> +	grub_printf ("%s", partuuid);
>> +      grub_free (partuuid);
>> +      grub_device_close (dev);
>> +      return GRUB_ERR_NONE;
>> +    }
>>     grub_device_close (dev);
>>     return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>>   }
>>

Re: [PATCH 1/1] add --partuuid to probe

[Nick Vinson]

[-- Attachment #1.1: Type: text/plain, Size: 4947 bytes --]

I'll need to check it against the 2.02 sources, but even if it needs
updating, it shouldn't be too hard to do it.  I can add your changes to
the patchset.

-Nick

On 02/14/2017 11:39 AM, Steve Kenton wrote:
> Nick,
> 
> What is the state of your patch set? I think mine is much smaller if you
> want to roll it into yours and resubmit. We were told all along to wait
> until after 2.02, which should to be very soon, so it looks like it's
> show time!
> 
> Steve Kenton
> 
> 
> On 02/14/2017 07:12 PM, Andrei Borzenkov wrote:
>> 14.02.2017 21:00, Steve Kenton пишет:
>>> Support both EFI and NT Disk Signature for passing to kernel as
>>> root=PARTUUID=$val
>>>
>> Yes, I guess we need to add it finally. Unfortunately it is too late for
>> 2.02, but it should go after in release. There were also patches for
>> grub-probe and we also need to support it in search to be on par with
>> filesystem UUID. May be if you could prepare consolidated patch set it
>> would be great.
>>
>> I apologize that it tool so long. Thank you for not giving up!
>>
>>> Signed-off-by: Steve Kenton <skenton@ou.edu>
>>> ---
>>> It's been six months so I thought I'd resend this so it does not get
>>> lost
>>> in case I get hit by a meteor or something before the next release
>>>
>>>   grub-core/commands/probe.c | 53
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 53 insertions(+)
>>>
>>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>>> index cf2793e..3afc8b8 100644
>>> --- a/grub-core/commands/probe.c
>>> +++ b/grub-core/commands/probe.c
>> Please also add patch for manual.
>>
>>> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
>>>       {"fs",        'f', 0, N_("Determine filesystem type."), 0, 0},
>>>       {"fs-uuid",        'u', 0, N_("Determine filesystem UUID."), 0,
>>> 0},
>>>       {"label",        'l', 0, N_("Determine filesystem label."), 0, 0},
>>> +    {"partuuid",    'g', 0, N_("Determine partition GUID/UUID."), 0,
>>> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>>>       {0, 0, 0, 0, 0, 0}
>>>     };
>>>   @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt,
>>> int argc, char **args)
>>>         grub_device_close (dev);
>>>         return GRUB_ERR_NONE;
>>>       }
>>> +  if (state[6].set)
>>> +    {
>>> +      char *partuuid = NULL; /* NULL to silence a spurious GCC
>>> warning */
>>> +      grub_uint8_t diskbuf[16];
>>> +      if (dev->disk && dev->disk->partition)
>>> +    {
>>> +      grub_partition_t p = dev->disk->partition;
>>> +      if (!grub_strcmp (p->partmap->name, "msdos"))
>>> +        {
>>> +          const int diskid_offset = 440; /* location in MBR */
>>> +          dev->disk->partition = p->parent;
>>> +          /* little-endian 4-byte NT disk signature */
>>> +          err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
>>> diskbuf);
>>> +          dev->disk->partition = p;
>>> +          if (err)
>>> +            return grub_errno;
>>> +          partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
>>> +                     diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
>>> +                     p->number + 1); /* one based partition number */
>>> +        }
>>> +      else if (!grub_strcmp (p->partmap->name, "gpt"))
>>> +        {
>>> +          const int guid_offset = 16; /* location in entry */
>>> +          dev->disk->partition = p->parent;
>>> +          /* little-endian 16-byte EFI partition GUID */
>>> +          err = grub_disk_read (dev->disk, p->offset, p->index +
>>> guid_offset, 16, diskbuf);
>>> +          dev->disk->partition = p;
>>> +          if (err)
>>> +            return grub_errno;
>>> +          partuuid = grub_xasprintf
>>> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
>>> +                     diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
>>> +                     diskbuf[5], diskbuf[4],
>>> +                     diskbuf[7], diskbuf[6],
>>> +                     diskbuf[8], diskbuf[9],
>>> +                     diskbuf[10], diskbuf[11], diskbuf[12],
>>> diskbuf[13], diskbuf[14], diskbuf[15]);
>>> +        }
>>> +      else
>>> +        return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
>>> +                   N_("partition map %s does not support partition
>>> UUIDs"),
>>> +                   dev->disk->partition->partmap->name);
>>> +    }
>>> +      else
>>> +    partuuid = grub_strdup (""); /* a freeable empty string */
>>> +
>>> +      if (state[0].set)
>>> +    grub_env_set (state[0].arg, partuuid);
>>> +      else
>>> +    grub_printf ("%s", partuuid);
>>> +      grub_free (partuuid);
>>> +      grub_device_close (dev);
>>> +      return GRUB_ERR_NONE;
>>> +    }
>>>     grub_device_close (dev);
>>>     return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>>>   }
>>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/1] add --partuuid to probe

[Vladimir 'phcoder' Serbinenko]

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

On Tue, Feb 14, 2017, 19:01 Steve Kenton <skenton@ou.edu> wrote:

> Support both EFI and NT Disk Signature for passing to kernel as
> root=PARTUUID=$val
>
> Signed-off-by: Steve Kenton <skenton@ou.edu>
> ---
> It's been six months so I thought I'd resend this so it does not get lost
> in case I get hit by a meteor or something before the next release
>
>  grub-core/commands/probe.c | 53
> ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> index cf2793e..3afc8b8 100644
> --- a/grub-core/commands/probe.c
> +++ b/grub-core/commands/probe.c
> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
>      {"fs",             'f', 0, N_("Determine filesystem type."), 0, 0},
>      {"fs-uuid",                'u', 0, N_("Determine filesystem UUID."),
> 0, 0},
>      {"label",          'l', 0, N_("Determine filesystem label."), 0, 0},
> +    {"partuuid",       'g', 0, N_("Determine partition GUID/UUID."), 0,
> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>
I like how it generalizes.

>      {0, 0, 0, 0, 0, 0}
>    };
>
> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc,
> char **args)
>        grub_device_close (dev);
>        return GRUB_ERR_NONE;
>      }
> +  if (state[6].set)
> +    {
> +      char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
> +      grub_uint8_t diskbuf[16];
> +      if (dev->disk && dev->disk->partition)
> +       {
> +         grub_partition_t p = dev->disk->partition;
> +         if (!grub_strcmp (p->partmap->name, "msdos"))
>
Please use == 0 rather than !

> +           {
> +             const int diskid_offset = 440; /* location in MBR */
>
Please get this from a common header rather than hard coding. I think we
have it in msdos.h

> +             dev->disk->partition = p->parent;
> +             /* little-endian 4-byte NT disk signature */
> +             err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
> diskbuf);
> +             dev->disk->partition = p;
> +             if (err)
> +               return grub_errno;
> +             partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
> +                                        diskbuf[3], diskbuf[2],
> diskbuf[1], diskbuf[0],
> +                                        p->number + 1); /* one based
> partition number */
>
This is not NT-style. NT uses partition offset. Who uses this format? Are
you sure that partition numbers are synced with user? Even in presence of
Solaris and bsd partitions.

> +           }
> +         else if (!grub_strcmp (p->partmap->name, "gpt"))
>
Ditto.

> +           {
> +             const int guid_offset = 16; /* location in entry */
>
Ditto.

> +             dev->disk->partition = p->parent;
> +             /* little-endian 16-byte EFI partition GUID */
> +             err = grub_disk_read (dev->disk, p->offset, p->index +
> guid_offset, 16, diskbuf);
> +             dev->disk->partition = p;
> +             if (err)
> +               return grub_errno;
> +             partuuid = grub_xasprintf
> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> +                                        diskbuf[3], diskbuf[2],
> diskbuf[1], diskbuf[0],
> +                                        diskbuf[5], diskbuf[4],
> +                                        diskbuf[7], diskbuf[6],
> +                                        diskbuf[8], diskbuf[9],
> +                                        diskbuf[10], diskbuf[11],
> diskbuf[12], diskbuf[13], diskbuf[14], diskbuf[15]);
> +           }
> +         else
> +           return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> +                              N_("partition map %s does not support
> partition UUIDs"),
> +                              dev->disk->partition->partmap->name);
> +       }
> +      else
> +       partuuid = grub_strdup (""); /* a freeable empty string */
> +
> +      if (state[0].set)
> +       grub_env_set (state[0].arg, partuuid);
> +      else
> +       grub_printf ("%s", partuuid);
> +      grub_free (partuuid);
> +      grub_device_close (dev);
> +      return GRUB_ERR_NONE;
> +    }
>    grub_device_close (dev);
>    return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>  }
> --
> 2.9.0.137.gcf4c2cf
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 8486 bytes --]

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

15.02.2017 13:56, Vladimir 'phcoder' Serbinenko пишет:
> On Tue, Feb 14, 2017, 19:01 Steve Kenton <skenton@ou.edu> wrote:
> 
>> Support both EFI and NT Disk Signature for passing to kernel as
>> root=PARTUUID=$val
>>
>> Signed-off-by: Steve Kenton <skenton@ou.edu>
>> ---
>> It's been six months so I thought I'd resend this so it does not get lost
>> in case I get hit by a meteor or something before the next release
>>
>>  grub-core/commands/probe.c | 53
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>> index cf2793e..3afc8b8 100644
>> --- a/grub-core/commands/probe.c
>> +++ b/grub-core/commands/probe.c
>> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
>>      {"fs",             'f', 0, N_("Determine filesystem type."), 0, 0},
>>      {"fs-uuid",                'u', 0, N_("Determine filesystem UUID."),
>> 0, 0},
>>      {"label",          'l', 0, N_("Determine filesystem label."), 0, 0},
>> +    {"partuuid",       'g', 0, N_("Determine partition GUID/UUID."), 0,
>> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>>
> I like how it generalizes.
> 
>>      {0, 0, 0, 0, 0, 0}
>>    };
>>
>> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc,
>> char **args)
>>        grub_device_close (dev);
>>        return GRUB_ERR_NONE;
>>      }
>> +  if (state[6].set)
>> +    {
>> +      char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
>> +      grub_uint8_t diskbuf[16];
>> +      if (dev->disk && dev->disk->partition)
>> +       {
>> +         grub_partition_t p = dev->disk->partition;
>> +         if (!grub_strcmp (p->partmap->name, "msdos"))
>>
> Please use == 0 rather than !
> 
>> +           {
>> +             const int diskid_offset = 440; /* location in MBR */
>>
> Please get this from a common header rather than hard coding. I think we
> have it in msdos.h
> 
>> +             dev->disk->partition = p->parent;
>> +             /* little-endian 4-byte NT disk signature */
>> +             err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
>> diskbuf);
>> +             dev->disk->partition = p;
>> +             if (err)
>> +               return grub_errno;
>> +             partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
>> +                                        diskbuf[3], diskbuf[2],
>> diskbuf[1], diskbuf[0],
>> +                                        p->number + 1); /* one based
>> partition number */
>>
> This is not NT-style. NT uses partition offset. Who uses this format? Are

This is used by util-linux and Linux kernel.


 *      6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing the
 *         unique id of a partition if the partition table provides it.
 *         The UUID may be either an EFI/GPT UUID, or refer to an MSDOS
 *         partition using the format SSSSSSSS-PP, where SSSSSSSS is a zero-
 *         filled hex representation of the 32-bit "NT disk signature",
and PP
 *         is a zero-filled hex representation of the 1-based partition
number.

> you sure that partition numbers are synced with user? Even in presence of
> Solaris and bsd partitions.
> 

It is not clear what we should return for nested partition. I'm not sure
whether linux kernel scans nested partitions at all in which case we
probably should follow the suite and assign PARTUUID to top-level
partitions only.

>> +           }
>> +         else if (!grub_strcmp (p->partmap->name, "gpt"))
>>
> Ditto.
> 
>> +           {
>> +             const int guid_offset = 16; /* location in entry */
>>
> Ditto.
> 
>> +             dev->disk->partition = p->parent;
>> +             /* little-endian 16-byte EFI partition GUID */
>> +             err = grub_disk_read (dev->disk, p->offset, p->index +
>> guid_offset, 16, diskbuf);
>> +             dev->disk->partition = p;
>> +             if (err)
>> +               return grub_errno;
>> +             partuuid = grub_xasprintf
>> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
>> +                                        diskbuf[3], diskbuf[2],
>> diskbuf[1], diskbuf[0],
>> +                                        diskbuf[5], diskbuf[4],
>> +                                        diskbuf[7], diskbuf[6],
>> +                                        diskbuf[8], diskbuf[9],
>> +                                        diskbuf[10], diskbuf[11],
>> diskbuf[12], diskbuf[13], diskbuf[14], diskbuf[15]);
>> +           }
>> +         else
>> +           return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
>> +                              N_("partition map %s does not support
>> partition UUIDs"),
>> +                              dev->disk->partition->partmap->name);
>> +       }
>> +      else
>> +       partuuid = grub_strdup (""); /* a freeable empty string */
>> +
>> +      if (state[0].set)
>> +       grub_env_set (state[0].arg, partuuid);
>> +      else
>> +       grub_printf ("%s", partuuid);
>> +      grub_free (partuuid);
>> +      grub_device_close (dev);
>> +      return GRUB_ERR_NONE;
>> +    }
>>    grub_device_close (dev);
>>    return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
>>  }
>> --
>> 2.9.0.137.gcf4c2cf
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 

Re: [PATCH 1/1] add --partuuid to probe

[Vladimir 'phcoder' Serbinenko]

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

On Wed, Feb 15, 2017, 17:27 Andrei Borzenkov <arvidjaar@gmail.com> wrote:

> 15.02.2017 13:56, Vladimir 'phcoder' Serbinenko пишет:
> > On Tue, Feb 14, 2017, 19:01 Steve Kenton <skenton@ou.edu> wrote:
> >
> >> Support both EFI and NT Disk Signature for passing to kernel as
> >> root=PARTUUID=$val
> >>
> >> Signed-off-by: Steve Kenton <skenton@ou.edu>
> >> ---
> >> It's been six months so I thought I'd resend this so it does not get
> lost
> >> in case I get hit by a meteor or something before the next release
> >>
> >>  grub-core/commands/probe.c | 53
> >> ++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 53 insertions(+)
> >>
> >> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> >> index cf2793e..3afc8b8 100644
> >> --- a/grub-core/commands/probe.c
> >> +++ b/grub-core/commands/probe.c
> >> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
> >>      {"fs",             'f', 0, N_("Determine filesystem type."), 0, 0},
> >>      {"fs-uuid",                'u', 0, N_("Determine filesystem
> UUID."),
> >> 0, 0},
> >>      {"label",          'l', 0, N_("Determine filesystem label."), 0,
> 0},
> >> +    {"partuuid",       'g', 0, N_("Determine partition GUID/UUID."), 0,
> >> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
> >>
> > I like how it generalizes.
> >
> >>      {0, 0, 0, 0, 0, 0}
> >>    };
> >>
> >> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int
> argc,
> >> char **args)
> >>        grub_device_close (dev);
> >>        return GRUB_ERR_NONE;
> >>      }
> >> +  if (state[6].set)
> >> +    {
> >> +      char *partuuid = NULL; /* NULL to silence a spurious GCC warning
> */
> >> +      grub_uint8_t diskbuf[16];
> >> +      if (dev->disk && dev->disk->partition)
> >> +       {
> >> +         grub_partition_t p = dev->disk->partition;
> >> +         if (!grub_strcmp (p->partmap->name, "msdos"))
> >>
> > Please use == 0 rather than !
> >
> >> +           {
> >> +             const int diskid_offset = 440; /* location in MBR */
> >>
> > Please get this from a common header rather than hard coding. I think we
> > have it in msdos.h
> >
> >> +             dev->disk->partition = p->parent;
> >> +             /* little-endian 4-byte NT disk signature */
> >> +             err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
> >> diskbuf);
> >> +             dev->disk->partition = p;
> >> +             if (err)
> >> +               return grub_errno;
> >> +             partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
> >> +                                        diskbuf[3], diskbuf[2],
> >> diskbuf[1], diskbuf[0],
> >> +                                        p->number + 1); /* one based
> >> partition number */
> >>
> > This is not NT-style. NT uses partition offset. Who uses this format? Are
>
> This is used by util-linux and Linux kernel.
>
>
>  *      6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing the
>  *         unique id of a partition if the partition table provides it.
>  *         The UUID may be either an EFI/GPT UUID, or refer to an MSDOS
>  *         partition using the format SSSSSSSS-PP, where SSSSSSSS is a
> zero-
>  *         filled hex representation of the 32-bit "NT disk signature",
> and PP
>  *         is a zero-filled hex representation of the 1-based partition
> number.
>
> > you sure that partition numbers are synced with user? Even in presence of
> > Solaris and bsd partitions.
> >
>
> It is not clear what we should return for nested partition. I'm not sure
> whether linux kernel scans nested partitions at all in which case we
> probably should follow the suite and assign PARTUUID to top-level
> partitions only.
>
Linux scans nested partitions and it uses though numeration in dev/sdaX, in
some cases shifting numbering of normal partitions. In those cases grub and
Linux numeration get out of sync

>
> >> +           }
> >> +         else if (!grub_strcmp (p->partmap->name, "gpt"))
> >>
> > Ditto.
> >
> >> +           {
> >> +             const int guid_offset = 16; /* location in entry */
> >>
> > Ditto.
> >
> >> +             dev->disk->partition = p->parent;
> >> +             /* little-endian 16-byte EFI partition GUID */
> >> +             err = grub_disk_read (dev->disk, p->offset, p->index +
> >> guid_offset, 16, diskbuf);
> >> +             dev->disk->partition = p;
> >> +             if (err)
> >> +               return grub_errno;
> >> +             partuuid = grub_xasprintf
> >> ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> >> +                                        diskbuf[3], diskbuf[2],
> >> diskbuf[1], diskbuf[0],
> >> +                                        diskbuf[5], diskbuf[4],
> >> +                                        diskbuf[7], diskbuf[6],
> >> +                                        diskbuf[8], diskbuf[9],
> >> +                                        diskbuf[10], diskbuf[11],
> >> diskbuf[12], diskbuf[13], diskbuf[14], diskbuf[15]);
> >> +           }
> >> +         else
> >> +           return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> >> +                              N_("partition map %s does not support
> >> partition UUIDs"),
> >> +                              dev->disk->partition->partmap->name);
> >> +       }
> >> +      else
> >> +       partuuid = grub_strdup (""); /* a freeable empty string */
> >> +
> >> +      if (state[0].set)
> >> +       grub_env_set (state[0].arg, partuuid);
> >> +      else
> >> +       grub_printf ("%s", partuuid);
> >> +      grub_free (partuuid);
> >> +      grub_device_close (dev);
> >> +      return GRUB_ERR_NONE;
> >> +    }
> >>    grub_device_close (dev);
> >>    return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
> >>  }
> >> --
> >> 2.9.0.137.gcf4c2cf
> >>
> >>
> >> _______________________________________________
> >> Grub-devel mailing list
> >> Grub-devel@gnu.org
> >> https://lists.gnu.org/mailman/listinfo/grub-devel
> >>
> >
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 12287 bytes --]

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

15.02.2017 20:25, Vladimir 'phcoder' Serbinenko пишет:
> On Wed, Feb 15, 2017, 17:27 Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
>> 15.02.2017 13:56, Vladimir 'phcoder' Serbinenko пишет:
>>> On Tue, Feb 14, 2017, 19:01 Steve Kenton <skenton@ou.edu> wrote:
>>>
>>>> Support both EFI and NT Disk Signature for passing to kernel as
>>>> root=PARTUUID=$val
>>>>
>>>> Signed-off-by: Steve Kenton <skenton@ou.edu>
>>>> ---
>>>> It's been six months so I thought I'd resend this so it does not get
>> lost
>>>> in case I get hit by a meteor or something before the next release
>>>>
>>>>  grub-core/commands/probe.c | 53
>>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 53 insertions(+)
>>>>
>>>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
>>>> index cf2793e..3afc8b8 100644
>>>> --- a/grub-core/commands/probe.c
>>>> +++ b/grub-core/commands/probe.c
>>>> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
>>>>      {"fs",             'f', 0, N_("Determine filesystem type."), 0, 0},
>>>>      {"fs-uuid",                'u', 0, N_("Determine filesystem
>> UUID."),
>>>> 0, 0},
>>>>      {"label",          'l', 0, N_("Determine filesystem label."), 0,
>> 0},
>>>> +    {"partuuid",       'g', 0, N_("Determine partition GUID/UUID."), 0,
>>>> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
>>>>
>>> I like how it generalizes.
>>>
>>>>      {0, 0, 0, 0, 0, 0}
>>>>    };
>>>>
>>>> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int
>> argc,
>>>> char **args)
>>>>        grub_device_close (dev);
>>>>        return GRUB_ERR_NONE;
>>>>      }
>>>> +  if (state[6].set)
>>>> +    {
>>>> +      char *partuuid = NULL; /* NULL to silence a spurious GCC warning
>> */
>>>> +      grub_uint8_t diskbuf[16];
>>>> +      if (dev->disk && dev->disk->partition)
>>>> +       {
>>>> +         grub_partition_t p = dev->disk->partition;
>>>> +         if (!grub_strcmp (p->partmap->name, "msdos"))
>>>>
>>> Please use == 0 rather than !
>>>
>>>> +           {
>>>> +             const int diskid_offset = 440; /* location in MBR */
>>>>
>>> Please get this from a common header rather than hard coding. I think we
>>> have it in msdos.h
>>>
>>>> +             dev->disk->partition = p->parent;
>>>> +             /* little-endian 4-byte NT disk signature */
>>>> +             err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
>>>> diskbuf);
>>>> +             dev->disk->partition = p;
>>>> +             if (err)
>>>> +               return grub_errno;
>>>> +             partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
>>>> +                                        diskbuf[3], diskbuf[2],
>>>> diskbuf[1], diskbuf[0],
>>>> +                                        p->number + 1); /* one based
>>>> partition number */
>>>>
>>> This is not NT-style. NT uses partition offset. Who uses this format? Are
>>
>> This is used by util-linux and Linux kernel.
>>
>>
>>  *      6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing the
>>  *         unique id of a partition if the partition table provides it.
>>  *         The UUID may be either an EFI/GPT UUID, or refer to an MSDOS
>>  *         partition using the format SSSSSSSS-PP, where SSSSSSSS is a
>> zero-
>>  *         filled hex representation of the 32-bit "NT disk signature",
>> and PP
>>  *         is a zero-filled hex representation of the 1-based partition
>> number.
>>
>>> you sure that partition numbers are synced with user? Even in presence of
>>> Solaris and bsd partitions.
>>>
>>
>> It is not clear what we should return for nested partition. I'm not sure
>> whether linux kernel scans nested partitions at all in which case we
>> probably should follow the suite and assign PARTUUID to top-level
>> partitions only.
>>
> Linux scans nested partitions and it uses though numeration in dev/sdaX, in
> some cases shifting numbering of normal partitions. In those cases grub and
> Linux numeration get out of sync
> 

Can you provide example? I tried to create nested partition table, but
Linux will not display it (actually attempt to "blockdev --rereadpt
/dev/vda5" fails with "Invalid argument").

        if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
                return -EINVAL;

Where bdev->bd_contains points to containing device for partition and to
itself for the whole disk.

As util-linux does not scan partition table itself, it does show these
nested partitions either.

Re: [PATCH 1/1] add --partuuid to probe

[Vladimir 'phcoder' Serbinenko]

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

On Sat, Feb 18, 2017, 22:13 Andrei Borzenkov <arvidjaar@gmail.com> wrote:

> 15.02.2017 20:25, Vladimir 'phcoder' Serbinenko пишет:
> > On Wed, Feb 15, 2017, 17:27 Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> >
> >> 15.02.2017 13:56, Vladimir 'phcoder' Serbinenko пишет:
> >>> On Tue, Feb 14, 2017, 19:01 Steve Kenton <skenton@ou.edu> wrote:
> >>>
> >>>> Support both EFI and NT Disk Signature for passing to kernel as
> >>>> root=PARTUUID=$val
> >>>>
> >>>> Signed-off-by: Steve Kenton <skenton@ou.edu>
> >>>> ---
> >>>> It's been six months so I thought I'd resend this so it does not get
> >> lost
> >>>> in case I get hit by a meteor or something before the next release
> >>>>
> >>>>  grub-core/commands/probe.c | 53
> >>>> ++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  1 file changed, 53 insertions(+)
> >>>>
> >>>> diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
> >>>> index cf2793e..3afc8b8 100644
> >>>> --- a/grub-core/commands/probe.c
> >>>> +++ b/grub-core/commands/probe.c
> >>>> @@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
> >>>>      {"fs",             'f', 0, N_("Determine filesystem type."), 0,
> 0},
> >>>>      {"fs-uuid",                'u', 0, N_("Determine filesystem
> >> UUID."),
> >>>> 0, 0},
> >>>>      {"label",          'l', 0, N_("Determine filesystem label."), 0,
> >> 0},
> >>>> +    {"partuuid",       'g', 0, N_("Determine partition GUID/UUID."),
> 0,
> >>>> 0}, /* GUID but Linux kernel calls it "PARTUUID" */
> >>>>
> >>> I like how it generalizes.
> >>>
> >>>>      {0, 0, 0, 0, 0, 0}
> >>>>    };
> >>>>
> >>>> @@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int
> >> argc,
> >>>> char **args)
> >>>>        grub_device_close (dev);
> >>>>        return GRUB_ERR_NONE;
> >>>>      }
> >>>> +  if (state[6].set)
> >>>> +    {
> >>>> +      char *partuuid = NULL; /* NULL to silence a spurious GCC
> warning
> >> */
> >>>> +      grub_uint8_t diskbuf[16];
> >>>> +      if (dev->disk && dev->disk->partition)
> >>>> +       {
> >>>> +         grub_partition_t p = dev->disk->partition;
> >>>> +         if (!grub_strcmp (p->partmap->name, "msdos"))
> >>>>
> >>> Please use == 0 rather than !
> >>>
> >>>> +           {
> >>>> +             const int diskid_offset = 440; /* location in MBR */
> >>>>
> >>> Please get this from a common header rather than hard coding. I think
> we
> >>> have it in msdos.h
> >>>
> >>>> +             dev->disk->partition = p->parent;
> >>>> +             /* little-endian 4-byte NT disk signature */
> >>>> +             err = grub_disk_read (dev->disk, 0, diskid_offset, 4,
> >>>> diskbuf);
> >>>> +             dev->disk->partition = p;
> >>>> +             if (err)
> >>>> +               return grub_errno;
> >>>> +             partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
> >>>> +                                        diskbuf[3], diskbuf[2],
> >>>> diskbuf[1], diskbuf[0],
> >>>> +                                        p->number + 1); /* one based
> >>>> partition number */
> >>>>
> >>> This is not NT-style. NT uses partition offset. Who uses this format?
> Are
> >>
> >> This is used by util-linux and Linux kernel.
> >>
> >>
> >>  *      6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing
> the
> >>  *         unique id of a partition if the partition table provides it.
> >>  *         The UUID may be either an EFI/GPT UUID, or refer to an MSDOS
> >>  *         partition using the format SSSSSSSS-PP, where SSSSSSSS is a
> >> zero-
> >>  *         filled hex representation of the 32-bit "NT disk signature",
> >> and PP
> >>  *         is a zero-filled hex representation of the 1-based partition
> >> number.
> >>
> >>> you sure that partition numbers are synced with user? Even in presence
> of
> >>> Solaris and bsd partitions.
> >>>
> >>
> >> It is not clear what we should return for nested partition. I'm not sure
> >> whether linux kernel scans nested partitions at all in which case we
> >> probably should follow the suite and assign PARTUUID to top-level
> >> partitions only.
> >>
> > Linux scans nested partitions and it uses though numeration in dev/sdaX,
> in
> > some cases shifting numbering of normal partitions. In those cases grub
> and
> > Linux numeration get out of sync
> >
>
> Can you provide example?

Bsd and Solaris partitions. I remember we had problem with numbering of
those.

> I tried to create nested partition table, but
> Linux will not display it (actually attempt to "blockdev --rereadpt
> /dev/vda5" fails with "Invalid argument").
>
>         if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
>                 return -EINVAL;
>
> Where bdev->bd_contains points to containing device for partition and to
> itself for the whole disk.
>
> As util-linux does not scan partition table itself, it does show these
> nested partitions either.
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 9816 bytes --]

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

27.02.2017 03:37, Vladimir 'phcoder' Serbinenko пишет:
...
>>>>> This is not NT-style. NT uses partition offset. Who uses this format?
>> Are
>>>>
>>>> This is used by util-linux and Linux kernel.
>>>>
>>>>
>>>>  *      6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing
>> the
>>>>  *         unique id of a partition if the partition table provides it.
>>>>  *         The UUID may be either an EFI/GPT UUID, or refer to an MSDOS
>>>>  *         partition using the format SSSSSSSS-PP, where SSSSSSSS is a
>>>> zero-
>>>>  *         filled hex representation of the 32-bit "NT disk signature",
>>>> and PP
>>>>  *         is a zero-filled hex representation of the 1-based partition
>>>> number.
>>>>
>>>>> you sure that partition numbers are synced with user? Even in presence
>> of
>>>>> Solaris and bsd partitions.
>>>>>
>>>>
>>>> It is not clear what we should return for nested partition. I'm not sure
>>>> whether linux kernel scans nested partitions at all in which case we
>>>> probably should follow the suite and assign PARTUUID to top-level
>>>> partitions only.
>>>>
>>> Linux scans nested partitions and it uses though numeration in dev/sdaX,
>> in
>>> some cases shifting numbering of normal partitions. In those cases grub
>> and
>>> Linux numeration get out of sync
>>>
>>
>> Can you provide example?
> 
> Bsd and Solaris partitions. I remember we had problem with numbering of
> those.
> 

Linux ignores nested BSD partitions (just tested). There are no special
files created. Of course someone needs to test what happens under
*BSD/Solaris in this case.

I never liked idea of artificial partition GUIDs for MBR, but as long as
only Linux is using them and we are consistent with its usage - so be it.

>> I tried to create nested partition table, but
>> Linux will not display it (actually attempt to "blockdev --rereadpt
>> /dev/vda5" fails with "Invalid argument").
>>
>>         if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
>>                 return -EINVAL;
>>
>> Where bdev->bd_contains points to containing device for partition and to
>> itself for the whole disk.
>>
>> As util-linux does not scan partition table itself, it does show these
>> nested partitions either.
>>

Re: [PATCH 1/1] add --partuuid to probe

[Vladimir 'phcoder' Serbinenko]

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

On Mon, Feb 27, 2017, 09:55 Andrei Borzenkov <arvidjaar@gmail.com> wrote:

> 27.02.2017 03:37, Vladimir 'phcoder' Serbinenko пишет:
> ...
> >>>>> This is not NT-style. NT uses partition offset. Who uses this format?
> >> Are
> >>>>
> >>>> This is used by util-linux and Linux kernel.
> >>>>
> >>>>
> >>>>  *      6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing
> >> the
> >>>>  *         unique id of a partition if the partition table provides
> it.
> >>>>  *         The UUID may be either an EFI/GPT UUID, or refer to an
> MSDOS
> >>>>  *         partition using the format SSSSSSSS-PP, where SSSSSSSS is a
> >>>> zero-
> >>>>  *         filled hex representation of the 32-bit "NT disk
> signature",
> >>>> and PP
> >>>>  *         is a zero-filled hex representation of the 1-based
> partition
> >>>> number.
> >>>>
> >>>>> you sure that partition numbers are synced with user? Even in
> presence
> >> of
> >>>>> Solaris and bsd partitions.
> >>>>>
> >>>>
> >>>> It is not clear what we should return for nested partition. I'm not
> sure
> >>>> whether linux kernel scans nested partitions at all in which case we
> >>>> probably should follow the suite and assign PARTUUID to top-level
> >>>> partitions only.
> >>>>
> >>> Linux scans nested partitions and it uses though numeration in
> dev/sdaX,
> >> in
> >>> some cases shifting numbering of normal partitions. In those cases grub
> >> and
> >>> Linux numeration get out of sync
> >>>
> >>
> >> Can you provide example?
> >
> > Bsd and Solaris partitions. I remember we had problem with numbering of
> > those.
> >
>
> Linux ignores nested BSD partitions (just tested). There are no special
> files created. Of course someone needs to test what happens under
> *BSD/Solaris in this case.
>
Kpartx or normal sdX? Is bsd-partition support enabled in kernel build?

>
> I never liked idea of artificial partition GUIDs for MBR, but as long as
> only Linux is using them and we are consistent with its usage - so be it.
>
> >> I tried to create nested partition table, but
> >> Linux will not display it (actually attempt to "blockdev --rereadpt
> >> /dev/vda5" fails with "Invalid argument").
> >>
> >>         if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
> >>                 return -EINVAL;
> >>
> >> Where bdev->bd_contains points to containing device for partition and to
> >> itself for the whole disk.
> >>
> >> As util-linux does not scan partition table itself, it does show these
> >> nested partitions either.
> >>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 5223 bytes --]

Re: [PATCH 1/1] add --partuuid to probe

[Andrei Borzenkov]

27.02.2017 21:20, Vladimir 'phcoder' Serbinenko пишет:
> On Mon, Feb 27, 2017, 09:55 Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
>> 27.02.2017 03:37, Vladimir 'phcoder' Serbinenko пишет:
>> ...
>>>>>>> This is not NT-style. NT uses partition offset. Who uses this format?
>>>> Are
>>>>>>
>>>>>> This is used by util-linux and Linux kernel.
>>>>>>
>>>>>>
>>>>>>  *      6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing
>>>> the
>>>>>>  *         unique id of a partition if the partition table provides
>> it.
>>>>>>  *         The UUID may be either an EFI/GPT UUID, or refer to an
>> MSDOS
>>>>>>  *         partition using the format SSSSSSSS-PP, where SSSSSSSS is a
>>>>>> zero-
>>>>>>  *         filled hex representation of the 32-bit "NT disk
>> signature",
>>>>>> and PP
>>>>>>  *         is a zero-filled hex representation of the 1-based
>> partition
>>>>>> number.
>>>>>>
>>>>>>> you sure that partition numbers are synced with user? Even in
>> presence
>>>> of
>>>>>>> Solaris and bsd partitions.
>>>>>>>
>>>>>>
>>>>>> It is not clear what we should return for nested partition. I'm not
>> sure
>>>>>> whether linux kernel scans nested partitions at all in which case we
>>>>>> probably should follow the suite and assign PARTUUID to top-level
>>>>>> partitions only.
>>>>>>
>>>>> Linux scans nested partitions and it uses though numeration in
>> dev/sdaX,
>>>> in
>>>>> some cases shifting numbering of normal partitions. In those cases grub
>>>> and
>>>>> Linux numeration get out of sync
>>>>>
>>>>
>>>> Can you provide example?
>>>
>>> Bsd and Solaris partitions. I remember we had problem with numbering of
>>> those.
>>>
>>
>> Linux ignores nested BSD partitions (just tested). There are no special
>> files created. Of course someone needs to test what happens under
>> *BSD/Solaris in this case.
>>
> Kpartx or normal sdX? Is bsd-partition support enabled in kernel build?
> 

OK I see, kernel skips BSD partition marked as "unused".

So it appears that kernel always puts special nested partitions after
normal logical MSDOS partitions, so it will not skew MSDOS partition
numbers.

[    1.529752]  vda: vda1 vda2 vda3 < vda5 vda6 >
                vda2: <openbsd: vda7 >


        /*
         * Look for partitions in two passes:
         * First find the primary and DOS-type extended partitions.
         * On the second pass look inside *BSD, Unixware and Solaris
partitions.
         */

For such partition (vda7) PARTUUID is empty.

P.S. I wonder whether we correctly map such partition ... no, we do not.

10:~ # cat /tmp/foo
(hd0) /dev/vda
10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda2
hd0,msdos2
10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda5
hd0,msdos5
10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda7
hd0,msdos2


>>
>> I never liked idea of artificial partition GUIDs for MBR, but as long as
>> only Linux is using them and we are consistent with its usage - so be it.
>>
>>>> I tried to create nested partition table, but
>>>> Linux will not display it (actually attempt to "blockdev --rereadpt
>>>> /dev/vda5" fails with "Invalid argument").
>>>>
>>>>         if (!disk_part_scan_enabled(disk) || bdev != bdev->bd_contains)
>>>>                 return -EINVAL;
>>>>
>>>> Where bdev->bd_contains points to containing device for partition and to
>>>> itself for the whole disk.
>>>>
>>>> As util-linux does not scan partition table itself, it does show these
>>>> nested partitions either.

Re: [PATCH 1/1] add --partuuid to probe

[Vladimir 'phcoder' Serbinenko]

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

On Mon, Feb 27, 2017, 20:11 Andrei Borzenkov <arvidjaar@gmail.com> wrote:

> 27.02.2017 21:20, Vladimir 'phcoder' Serbinenko пишет:
> > On Mon, Feb 27, 2017, 09:55 Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> >
> >> 27.02.2017 03:37, Vladimir 'phcoder' Serbinenko пишет:
> >> ...
> >>>>>>> This is not NT-style. NT uses partition offset. Who uses this
> format?
> >>>> Are
> >>>>>>
> >>>>>> This is used by util-linux and Linux kernel.
> >>>>>>
> >>>>>>
> >>>>>>  *      6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF
> representing
> >>>> the
> >>>>>>  *         unique id of a partition if the partition table provides
> >> it.
> >>>>>>  *         The UUID may be either an EFI/GPT UUID, or refer to an
> >> MSDOS
> >>>>>>  *         partition using the format SSSSSSSS-PP, where SSSSSSSS
> is a
> >>>>>> zero-
> >>>>>>  *         filled hex representation of the 32-bit "NT disk
> >> signature",
> >>>>>> and PP
> >>>>>>  *         is a zero-filled hex representation of the 1-based
> >> partition
> >>>>>> number.
> >>>>>>
> >>>>>>> you sure that partition numbers are synced with user? Even in
> >> presence
> >>>> of
> >>>>>>> Solaris and bsd partitions.
> >>>>>>>
> >>>>>>
> >>>>>> It is not clear what we should return for nested partition. I'm not
> >> sure
> >>>>>> whether linux kernel scans nested partitions at all in which case we
> >>>>>> probably should follow the suite and assign PARTUUID to top-level
> >>>>>> partitions only.
> >>>>>>
> >>>>> Linux scans nested partitions and it uses though numeration in
> >> dev/sdaX,
> >>>> in
> >>>>> some cases shifting numbering of normal partitions. In those cases
> grub
> >>>> and
> >>>>> Linux numeration get out of sync
> >>>>>
> >>>>
> >>>> Can you provide example?
> >>>
> >>> Bsd and Solaris partitions. I remember we had problem with numbering of
> >>> those.
> >>>
> >>
> >> Linux ignores nested BSD partitions (just tested). There are no special
> >> files created. Of course someone needs to test what happens under
> >> *BSD/Solaris in this case.
> >>
> > Kpartx or normal sdX? Is bsd-partition support enabled in kernel build?
> >
>
> OK I see, kernel skips BSD partition marked as "unused".
>
> So it appears that kernel always puts special nested partitions after
> normal logical MSDOS partitions, so it will not skew MSDOS partition
> numbers.
>
> [    1.529752]  vda: vda1 vda2 vda3 < vda5 vda6 >
>                 vda2: <openbsd: vda7 >
>
>
>         /*
>          * Look for partitions in two passes:
>          * First find the primary and DOS-type extended partitions.
>          * On the second pass look inside *BSD, Unixware and Solaris
> partitions.
>          */
>
> For such partition (vda7) PARTUUID is empty.
>
> P.S. I wonder whether we correctly map such partition ... no, we do not.
>
> 10:~ # cat /tmp/foo
> (hd0) /dev/vda
> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda2
> hd0,msdos2
> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda5
> hd0,msdos5
> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda7
> hd0,msdos2
>
WAI. In case when subpartition starts at the same sector as partition
itself.

>
>
> >>
> >> I never liked idea of artificial partition GUIDs for MBR, but as long as
> >> only Linux is using them and we are consistent with its usage - so be
> it.
> >>
> >>>> I tried to create nested partition table, but
> >>>> Linux will not display it (actually attempt to "blockdev --rereadpt
> >>>> /dev/vda5" fails with "Invalid argument").
> >>>>
> >>>>         if (!disk_part_scan_enabled(disk) || bdev !=
> bdev->bd_contains)
> >>>>                 return -EINVAL;
> >>>>
> >>>> Where bdev->bd_contains points to containing device for partition and
> to
> >>>> itself for the whole disk.
> >>>>
> >>>> As util-linux does not scan partition table itself, it does show these
> >>>> nested partitions either.
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 8077 bytes --]

grub-probe for nested BSD partition on Linux (was: [PATCH 1/1] add --partuuid to probe)

[Andrei Borzenkov]

28.02.2017 17:08, Vladimir 'phcoder' Serbinenko пишет:
>>
>> OK I see, kernel skips BSD partition marked as "unused".
>>
>> So it appears that kernel always puts special nested partitions after
>> normal logical MSDOS partitions, so it will not skew MSDOS partition
>> numbers.
>>
>> [    1.529752]  vda: vda1 vda2 vda3 < vda5 vda6 >
>>                 vda2: <openbsd: vda7 >
>>
>>
>>         /*
>>          * Look for partitions in two passes:
>>          * First find the primary and DOS-type extended partitions.
>>          * On the second pass look inside *BSD, Unixware and Solaris
>> partitions.
>>          */
>>
>> For such partition (vda7) PARTUUID is empty.
>>
>> P.S. I wonder whether we correctly map such partition ... no, we do not.
>>
>> 10:~ # cat /tmp/foo
>> (hd0) /dev/vda
>> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda2
>> hd0,msdos2
>> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda5
>> hd0,msdos5
>> 10:~ # grub2-probe -m /tmp/foo -t compatibility_hint -d /dev/vda7
>> hd0,msdos2
>>
> WAI. In case when subpartition starts at the same sector as partition
> itself.
> 

Sorry? vda7 is 256M, how can you suddenly pretend it is 2G?

10:~ # fdisk -l /dev/vda
Disk /dev/vda: 5 GiB, 5368709120 bytes, 10485760 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x882b18da

Device     Boot   Start      End Sectors  Size Id Type
/dev/vda1          2048  2099199 2097152    1G 83 Linux
/dev/vda2       2099200  6293503 4194304    2G a6 OpenBSD
/dev/vda3       6293504 10485759 4192256    2G  5 Extended
/dev/vda5       6295552  6819839  524288  256M 83 Linux
/dev/vda6       6821888  7346175  524288  256M 83 Linux
10:~ # fdisk -l /dev/vda2
Disk /dev/vda2: 2 GiB, 2147483648 bytes, 4194304 sectors
Geometry: 16 heads, 63 sectors/track, 10402 cylinders
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: bsd

Slice   Start      End  Sectors  Size Type     Fsize Bsize Cpg
a     2099200  2623488   524289  256M boot         0     0   0
b     2099200  2623488   524289  256M unused       0     0   0
c     2099200  6293503  4194304    2G unused       0     0   0
d           0 10485215 10485216    5G unused       0     0   0

Partition table entries are not in disk order.

Re: grub-probe for nested BSD partition on Linux (was: [PATCH 1/1] add --partuuid to probe)

[Lennart Sorensen]

On Tue, Feb 28, 2017 at 08:13:53PM +0300, Andrei Borzenkov wrote:
> Sorry? vda7 is 256M, how can you suddenly pretend it is 2G?
> 
> 10:~ # fdisk -l /dev/vda
> Disk /dev/vda: 5 GiB, 5368709120 bytes, 10485760 sectors
> Units: sectors of 1 * 512 = 512 bytes
> Sector size (logical/physical): 512 bytes / 512 bytes
> I/O size (minimum/optimal): 512 bytes / 512 bytes
> Disklabel type: dos
> Disk identifier: 0x882b18da
> 
> Device     Boot   Start      End Sectors  Size Id Type
> /dev/vda1          2048  2099199 2097152    1G 83 Linux
> /dev/vda2       2099200  6293503 4194304    2G a6 OpenBSD
> /dev/vda3       6293504 10485759 4192256    2G  5 Extended
> /dev/vda5       6295552  6819839  524288  256M 83 Linux
> /dev/vda6       6821888  7346175  524288  256M 83 Linux
> 10:~ # fdisk -l /dev/vda2
> Disk /dev/vda2: 2 GiB, 2147483648 bytes, 4194304 sectors
> Geometry: 16 heads, 63 sectors/track, 10402 cylinders
> Units: sectors of 1 * 512 = 512 bytes
> Sector size (logical/physical): 512 bytes / 512 bytes
> I/O size (minimum/optimal): 512 bytes / 512 bytes
> Disklabel type: bsd
> 
> Slice   Start      End  Sectors  Size Type     Fsize Bsize Cpg
> a     2099200  2623488   524289  256M boot         0     0   0
> b     2099200  2623488   524289  256M unused       0     0   0
> c     2099200  6293503  4194304    2G unused       0     0   0
> d           0 10485215 10485216    5G unused       0     0   0
> 
> Partition table entries are not in disk order.

Well that does look mostly sane for an ancient BSD version.

Slice a is the rootfs for BSD

Slice b should traditionally be swap, but appears to be unused here,
and for some reason uses the same part of the disk as slice a, which
does look odd.  Maybe since the type is unknown, it doesn't matter.

Slice c is the whole partition containing the slices (vda2)

Slice d is the whole disk (vda), which apparently hasn't been used in
BSD for a couple of decades now, if wikipedia can be trusted.

So if linux only bothers to count the partition with a type that isn't
unused, then it probably makes sense.  That appears to be what the
partition parsing code does.

The start being the same as the start of vda2 does seem odd given it
appears that you are supposed to leave space at the start (1 track
recommended, or 64 sectors), and it appears the disklabel is at sector
1 (0 being the first), so it doesn't seem like having your filesystem
start there would be safe unless the filesystem has reserved space at
the start for such things.

-- 
Len Sorensen

Re: grub-probe for nested BSD partition on Linux

[Andrei Borzenkov]

28.02.2017 21:31, Lennart Sorensen пишет:
> On Tue, Feb 28, 2017 at 08:13:53PM +0300, Andrei Borzenkov wrote:
>> Sorry? vda7 is 256M, how can you suddenly pretend it is 2G?
>>
>> 10:~ # fdisk -l /dev/vda
>> Disk /dev/vda: 5 GiB, 5368709120 bytes, 10485760 sectors
>> Units: sectors of 1 * 512 = 512 bytes
>> Sector size (logical/physical): 512 bytes / 512 bytes
>> I/O size (minimum/optimal): 512 bytes / 512 bytes
>> Disklabel type: dos
>> Disk identifier: 0x882b18da
>>
>> Device     Boot   Start      End Sectors  Size Id Type
>> /dev/vda1          2048  2099199 2097152    1G 83 Linux
>> /dev/vda2       2099200  6293503 4194304    2G a6 OpenBSD
>> /dev/vda3       6293504 10485759 4192256    2G  5 Extended
>> /dev/vda5       6295552  6819839  524288  256M 83 Linux
>> /dev/vda6       6821888  7346175  524288  256M 83 Linux
>> 10:~ # fdisk -l /dev/vda2
>> Disk /dev/vda2: 2 GiB, 2147483648 bytes, 4194304 sectors
>> Geometry: 16 heads, 63 sectors/track, 10402 cylinders
>> Units: sectors of 1 * 512 = 512 bytes
>> Sector size (logical/physical): 512 bytes / 512 bytes
>> I/O size (minimum/optimal): 512 bytes / 512 bytes
>> Disklabel type: bsd
>>
>> Slice   Start      End  Sectors  Size Type     Fsize Bsize Cpg
>> a     2099200  2623488   524289  256M boot         0     0   0
>> b     2099200  2623488   524289  256M unused       0     0   0
>> c     2099200  6293503  4194304    2G unused       0     0   0
>> d           0 10485215 10485216    5G unused       0     0   0
>>
>> Partition table entries are not in disk order.
> 
> Well that does look mostly sane for an ancient BSD version.
> 

In case I was not clear.

BSD partition a is represented in Linux as /dev/vda7. grub-probe -t
compatibility_hints -d /dev/vda7 returns hd0,msdos2 which is obviously
wrong, because it refers to 2GB area on disk while partition /dev/vda7
is just 256M. I would expect either hd0,msdos2,openbsd1 or nothing
depending on whether such nested partition is supported.

> Slice a is the rootfs for BSD
> 
> Slice b should traditionally be swap, but appears to be unused here,
> and for some reason uses the same part of the disk as slice a, which
> does look odd.  Maybe since the type is unknown, it doesn't matter.
> 
> Slice c is the whole partition containing the slices (vda2)
> 
> Slice d is the whole disk (vda), which apparently hasn't been used in
> BSD for a couple of decades now, if wikipedia can be trusted.
> 
> So if linux only bothers to count the partition with a type that isn't
> unused, then it probably makes sense.  That appears to be what the
> partition parsing code does.
> 
> The start being the same as the start of vda2 does seem odd given it
> appears that you are supposed to leave space at the start (1 track
> recommended, or 64 sectors), and it appears the disklabel is at sector
> 1 (0 being the first), so it doesn't seem like having your filesystem
> start there would be safe unless the filesystem has reserved space at
> the start for such things.
> 

I just pressed 'b' in Linux fdisk and only change two partition sizes. I
have no idea how fdisk choses partition start. Could be a bug in fdisk.

Re: grub-probe for nested BSD partition on Linux

[Lennart Sorensen]

On Tue, Feb 28, 2017 at 09:50:27PM +0300, Andrei Borzenkov wrote:
> 28.02.2017 21:31, Lennart Sorensen пишет:
> > On Tue, Feb 28, 2017 at 08:13:53PM +0300, Andrei Borzenkov wrote:
> >> Sorry? vda7 is 256M, how can you suddenly pretend it is 2G?
> >>
> >> 10:~ # fdisk -l /dev/vda
> >> Disk /dev/vda: 5 GiB, 5368709120 bytes, 10485760 sectors
> >> Units: sectors of 1 * 512 = 512 bytes
> >> Sector size (logical/physical): 512 bytes / 512 bytes
> >> I/O size (minimum/optimal): 512 bytes / 512 bytes
> >> Disklabel type: dos
> >> Disk identifier: 0x882b18da
> >>
> >> Device     Boot   Start      End Sectors  Size Id Type
> >> /dev/vda1          2048  2099199 2097152    1G 83 Linux
> >> /dev/vda2       2099200  6293503 4194304    2G a6 OpenBSD
> >> /dev/vda3       6293504 10485759 4192256    2G  5 Extended
> >> /dev/vda5       6295552  6819839  524288  256M 83 Linux
> >> /dev/vda6       6821888  7346175  524288  256M 83 Linux
> >> 10:~ # fdisk -l /dev/vda2
> >> Disk /dev/vda2: 2 GiB, 2147483648 bytes, 4194304 sectors
> >> Geometry: 16 heads, 63 sectors/track, 10402 cylinders
> >> Units: sectors of 1 * 512 = 512 bytes
> >> Sector size (logical/physical): 512 bytes / 512 bytes
> >> I/O size (minimum/optimal): 512 bytes / 512 bytes
> >> Disklabel type: bsd
> >>
> >> Slice   Start      End  Sectors  Size Type     Fsize Bsize Cpg
> >> a     2099200  2623488   524289  256M boot         0     0   0
> >> b     2099200  2623488   524289  256M unused       0     0   0
> >> c     2099200  6293503  4194304    2G unused       0     0   0
> >> d           0 10485215 10485216    5G unused       0     0   0
> >>
> >> Partition table entries are not in disk order.
> > 
> > Well that does look mostly sane for an ancient BSD version.
> > 
> 
> In case I was not clear.
> 
> BSD partition a is represented in Linux as /dev/vda7. grub-probe -t
> compatibility_hints -d /dev/vda7 returns hd0,msdos2 which is obviously
> wrong, because it refers to 2GB area on disk while partition /dev/vda7
> is just 256M. I would expect either hd0,msdos2,openbsd1 or nothing
> depending on whether such nested partition is supported.

Oh now I see what you mean.  Hmm, that is an interesting problem.

It seems FFS and UFS reserve either 1 or 8KB at the start for bootblock,
which I suppose would save the disklabel from being overwritten if using
one of those filesystems.  The documentation does seem to say to always
leave a gap before the first slice/partition in all cases.

> > Slice a is the rootfs for BSD
> > 
> > Slice b should traditionally be swap, but appears to be unused here,
> > and for some reason uses the same part of the disk as slice a, which
> > does look odd.  Maybe since the type is unknown, it doesn't matter.
> > 
> > Slice c is the whole partition containing the slices (vda2)
> > 
> > Slice d is the whole disk (vda), which apparently hasn't been used in
> > BSD for a couple of decades now, if wikipedia can be trusted.
> > 
> > So if linux only bothers to count the partition with a type that isn't
> > unused, then it probably makes sense.  That appears to be what the
> > partition parsing code does.
> > 
> > The start being the same as the start of vda2 does seem odd given it
> > appears that you are supposed to leave space at the start (1 track
> > recommended, or 64 sectors), and it appears the disklabel is at sector
> > 1 (0 being the first), so it doesn't seem like having your filesystem
> > start there would be safe unless the filesystem has reserved space at
> > the start for such things.
> > 
> 
> I just pressed 'b' in Linux fdisk and only change two partition sizes. I
> have no idea how fdisk choses partition start. Could be a bug in fdisk.

I think fdisk's default is probably wrong in this case.

Certainly if you create a slice and then decide to mkfs.ext2 on it,
your disklabel is gone if you don't make the start offset a bit higher.

-- 
Len Sorensen

Re: grub-probe for nested BSD partition on Linux

[Andrei Borzenkov]

01.03.2017 01:05, Lennart Sorensen пишет:
> On Tue, Feb 28, 2017 at 09:50:27PM +0300, Andrei Borzenkov wrote:
>> 28.02.2017 21:31, Lennart Sorensen пишет:
>>> On Tue, Feb 28, 2017 at 08:13:53PM +0300, Andrei Borzenkov wrote:
>>>> Sorry? vda7 is 256M, how can you suddenly pretend it is 2G?
>>>>
>>>> 10:~ # fdisk -l /dev/vda
>>>> Disk /dev/vda: 5 GiB, 5368709120 bytes, 10485760 sectors
>>>> Units: sectors of 1 * 512 = 512 bytes
>>>> Sector size (logical/physical): 512 bytes / 512 bytes
>>>> I/O size (minimum/optimal): 512 bytes / 512 bytes
>>>> Disklabel type: dos
>>>> Disk identifier: 0x882b18da
>>>>
>>>> Device     Boot   Start      End Sectors  Size Id Type
>>>> /dev/vda1          2048  2099199 2097152    1G 83 Linux
>>>> /dev/vda2       2099200  6293503 4194304    2G a6 OpenBSD
>>>> /dev/vda3       6293504 10485759 4192256    2G  5 Extended
>>>> /dev/vda5       6295552  6819839  524288  256M 83 Linux
>>>> /dev/vda6       6821888  7346175  524288  256M 83 Linux
>>>> 10:~ # fdisk -l /dev/vda2
>>>> Disk /dev/vda2: 2 GiB, 2147483648 bytes, 4194304 sectors
>>>> Geometry: 16 heads, 63 sectors/track, 10402 cylinders
>>>> Units: sectors of 1 * 512 = 512 bytes
>>>> Sector size (logical/physical): 512 bytes / 512 bytes
>>>> I/O size (minimum/optimal): 512 bytes / 512 bytes
>>>> Disklabel type: bsd
>>>>
>>>> Slice   Start      End  Sectors  Size Type     Fsize Bsize Cpg
>>>> a     2099200  2623488   524289  256M boot         0     0   0
>>>> b     2099200  2623488   524289  256M unused       0     0   0
>>>> c     2099200  6293503  4194304    2G unused       0     0   0
>>>> d           0 10485215 10485216    5G unused       0     0   0
>>>>
>>>> Partition table entries are not in disk order.
>>>
>>> Well that does look mostly sane for an ancient BSD version.
>>>
>>
>> In case I was not clear.
>>
>> BSD partition a is represented in Linux as /dev/vda7. grub-probe -t
>> compatibility_hints -d /dev/vda7 returns hd0,msdos2 which is obviously
>> wrong, because it refers to 2GB area on disk while partition /dev/vda7
>> is just 256M. I would expect either hd0,msdos2,openbsd1 or nothing
>> depending on whether such nested partition is supported.
> 
> Oh now I see what you mean.  Hmm, that is an interesting problem.
> 
> It seems FFS and UFS reserve either 1 or 8KB at the start for bootblock,
> which I suppose would save the disklabel from being overwritten if using
> one of those filesystems. 

Solaris usually does not have issues with slice containing label as the
first block. Native tools (UFS, ZFS, SVM) and non-native (Veritas) all
preserve it. So I never considered it something unusual.

[PATCH 1/1] add --partuuid to probe

[Steve Kenton]

Supports both EFI and NT Disk Signature for passing to kernel as root=PARTUUID=$val

Signed-off-by: Steve Kenton <skenton@ou.edu>
---
This passed light testing here, giving correct results for both types compared to blkid

 grub-core/commands/probe.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/grub-core/commands/probe.c b/grub-core/commands/probe.c
index cf2793e..0473d39 100644
--- a/grub-core/commands/probe.c
+++ b/grub-core/commands/probe.c
@@ -45,6 +45,7 @@ static const struct grub_arg_option options[] =
     {"fs",		'f', 0, N_("Determine filesystem type."), 0, 0},
     {"fs-uuid",		'u', 0, N_("Determine filesystem UUID."), 0, 0},
     {"label",		'l', 0, N_("Determine filesystem label."), 0, 0},
+    {"partuuid",	'g', 0, N_("Determine partition UUID."), 0, 0}, /* 'g' for guid since 'u' was taken */
     {0, 0, 0, 0, 0, 0}
   };
 
@@ -154,6 +155,58 @@ grub_cmd_probe (grub_extcmd_context_t ctxt, int argc, char **args)
       grub_device_close (dev);
       return GRUB_ERR_NONE;
     }
+  if (state[6].set)
+    {
+      char *partuuid = NULL; /* NULL to silence a spurious GCC warning */
+      grub_uint8_t diskbuf[16];
+      if (dev->disk && dev->disk->partition)
+	{
+	  grub_partition_t p = dev->disk->partition;
+	  if (!grub_strcmp (p->partmap->name, "msdos"))
+	    {
+	      const int diskid_offset = 440; /* location in MBR */
+	      dev->disk->partition = p->parent;
+	      /* little-endian 4-byte NT disk signature */
+	      err = grub_disk_read (dev->disk, 0, diskid_offset, 4, diskbuf);
+	      dev->disk->partition = p;
+	      if (err)
+	        return grub_errno;
+	      partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x",
+					 diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
+					 p->number + 1); /* one based partition number */
+	    }
+	  else if (!grub_strcmp (p->partmap->name, "gpt"))
+	    {
+	      const int guid_offset = 16; /* location in entry */
+	      dev->disk->partition = p->parent;
+	      /* little-endian 16-byte EFI partition GUID */
+	      err = grub_disk_read (dev->disk, p->offset, p->index + guid_offset, 16, diskbuf);
+	      dev->disk->partition = p;
+	      if (err)
+	        return grub_errno;
+	      partuuid = grub_xasprintf ("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+					 diskbuf[3], diskbuf[2], diskbuf[1], diskbuf[0],
+					 diskbuf[5], diskbuf[4],
+					 diskbuf[7], diskbuf[6],
+					 diskbuf[8], diskbuf[9],
+					 diskbuf[10], diskbuf[11], diskbuf[12], diskbuf[13], diskbuf[14], diskbuf[15]);
+	    }
+	  else
+	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			       N_("partition map %s does not support partition UUIDs"),
+			       dev->disk->partition->partmap->name);
+	}
+      else
+	partuuid = grub_strdup (""); /* a freeable empty string */
+
+      if (state[0].set)
+	grub_env_set (state[0].arg, partuuid);
+      else
+	grub_printf ("%s", partuuid);
+      grub_free (partuuid);
+      grub_device_close (dev);
+      return GRUB_ERR_NONE;
+    }
   grub_device_close (dev);
   return grub_error (GRUB_ERR_BAD_ARGUMENT, "unrecognised target");
 }
-- 
2.9.0.137.gcf4c2cf