Prev: A question of perf NMI handler
Next: [RFC] Large weight differential leads to inefficient load balancing
From: Kay Sievers on 4 Aug 2010 06:20
On Wed, Aug 4, 2010 at 11:00, Karel Zak <kzak(a)redhat.com> wrote:
> On Tue, Aug 03, 2010 at 09:04:42PM -0500, Will Drewry wrote:
>> This change extends the partition_meta_info structure to
>> support EFI GPT-specific metadata and ensures that data
>> is copied in on partition scanning.
>
> Why do want to store GPT-specific data (efi_guid_t) to
> partition_meta_info? I think it would be better to use label and uuid
> in a generic format (e.g. string or u8 uuid[16]) -- then you don't
> have to use things like union, disklabel specific code to compare
> uuids, etc.  IMHO your current code is too complicated.

I don't mind having the raw data and the type accessible. It might be
useful for something we don't know about and it basically comes for
free.

But the only thing we are really interested in is the UUID, which,
like Tejun already suggested, we should probably store
format-independent, and have it always accessible. That way, we would
not need any type-specific parser, we just handle the normal DCE
format.

I don't think we should support any of the labels anyway in root= and
similar, because they never really worked reliably with duplicates,
and just ask for trouble.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Will Drewry on 4 Aug 2010 10:50
On Wed, Aug 4, 2010 at 5:14 AM, Kay Sievers <kay.sievers(a)vrfy.org> wrote:
> On Wed, Aug 4, 2010 at 11:00, Karel Zak <kzak(a)redhat.com> wrote:
>> On Tue, Aug 03, 2010 at 09:04:42PM -0500, Will Drewry wrote:
>>> This change extends the partition_meta_info structure to
>>> support EFI GPT-specific metadata and ensures that data
>>> is copied in on partition scanning.
>>
>> Why do want to store GPT-specific data (efi_guid_t) to
>> partition_meta_info? I think it would be better to use label and uuid
>> in a generic format (e.g. string or u8 uuid[16]) -- then you don't
>> have to use things like union, disklabel specific code to compare
>> uuids, etc. �IMHO your current code is too complicated.
>
> I don't mind having the raw data and the type accessible. It might be
> useful for something we don't know about and it basically comes for
> free.

I'll bump out the uuid at least, but it might be worth keeping an
extended meta info option. But it's a lot less work to ditch it, so
I'm happy enough either way.


> But the only thing we are really interested in is the UUID, which,
> like Tejun already suggested, we should probably store
> format-independent, and have it always accessible. That way, we would
> not need any type-specific parser, we just handle the normal DCE
> format.

I'll bump it out and make it the efi code generic-ify its uuid. Out
of curiousity, were you and Tejun thinking of keeping it as a 36 byte
string or as the 16 byte packed value. While less space efficient,
I'd prefer the u8[36] as it avoids dealing with versioning when
parsing the user-supplied string. Instead, each partition type can
properly unparse its uuids according to what they expect.

Seem reasonable?

> I don't think we should support any of the labels anyway in root= and
> similar, because they never really worked reliably with duplicates,
> and just ask for trouble.

Agreed. I don't think labels make sense, but we may later want to
support partition types (as I mention in my other mail).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kay Sievers on 4 Aug 2010 11:30
On Wed, Aug 4, 2010 at 16:44, Will Drewry <wad(a)chromium.org> wrote:
> On Wed, Aug 4, 2010 at 5:14 AM, Kay Sievers <kay.sievers(a)vrfy.org> wrote:

>> But the only thing we are really interested in is the UUID, which,
>> like Tejun already suggested, we should probably store
>> format-independent, and have it always accessible. That way, we would
>> not need any type-specific parser, we just handle the normal DCE
>> format.
>
> I'll bump it out and make it the efi code generic-ify its uuid.  Out
> of curiousity, were you and Tejun thinking of keeping it as a 36 byte
> string or as the 16 byte packed value.  While less space efficient,
> I'd prefer the u8[36] as it avoids dealing with versioning when
> parsing the user-supplied string.  Instead, each partition type can
> properly unparse its uuids according to what they expect.

I think we should use the packed version, which is case-insensitive,
or at least normalize it to a defined case.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Will Drewry on 4 Aug 2010 12:00
On Wed, Aug 4, 2010 at 10:28 AM, Kay Sievers <kay.sievers(a)vrfy.org> wrote:
> On Wed, Aug 4, 2010 at 16:44, Will Drewry <wad(a)chromium.org> wrote:
>> On Wed, Aug 4, 2010 at 5:14 AM, Kay Sievers <kay.sievers(a)vrfy.org> wrote:
>
>>> But the only thing we are really interested in is the UUID, which,
>>> like Tejun already suggested, we should probably store
>>> format-independent, and have it always accessible. That way, we would
>>> not need any type-specific parser, we just handle the normal DCE
>>> format.
>>
>> I'll bump it out and make it the efi code generic-ify its uuid. �Out
>> of curiousity, were you and Tejun thinking of keeping it as a 36 byte
>> string or as the 16 byte packed value. �While less space efficient,
>> I'd prefer the u8[36] as it avoids dealing with versioning when
>> parsing the user-supplied string. �Instead, each partition type can
>> properly unparse its uuids according to what they expect.
>
> I think we should use the packed version, which is case-insensitive,
> or at least normalize it to a defined case.

Yeah, I'm all for tolower()ing the entire value instead of packing :)
, but looking at the uuid code quickly, and it seems that the only
real difference in ordering practically is big-endian versus little
endian. Since sprintf supports converting both of those to strings,
it would make sense to just have a part_pack_uuid(char *uuid) which
will always do, let's say big endian, packing. Then it won't be too
much work for the partition format to export the value either directly
to the right format or just via sprintf then pack.

I'll add a common packer/unpacker, and see how it looks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: A question of perf NMI handler
Next: [RFC] Large weight differential leads to inefficient load balancing