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

[Andrei Borzenkov <arvidjaar@gmail.com>]

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.