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

["Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com>]

[-- 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 --]