EFI's GPT partitioning scheme expects that all partitions have a unique
identifiers.  After initial partition scanning, this information is
completely lost to the rest of the kernel.

efi_partition_by_guid exposes GPT parsing support in a limited fashion
to allow other portions of the kernel to map a partition from GUID to
map.

An alternate implementation (and more generic) would be to expose a function
(efi_partition_walk) that iterates over the partition table providing access to
each partitions information to a callback, much like device class iterators.

The motivation for this change is the ability to have device mapper targets
resolve backing devices by GUID instead of specifically by partition number.
This could also be used in the init boot path (root=GUID) quite simply by
modeling scanning code on printk_all_partitions(), or with another patchset
allowing dm="" in the boot path: http://lkml.org/lkml/2010/6/7/418

[ Additional context: http://codereview.chromium.org/3026039/show ]

Would a change like this or something that exposes the GPT information via a
walking function be something that would be of any interest, or is it explicitly
against the current design/access goals with respect to partition information?

Any and all feedback is truly appreciated.

Signed-off-by: Will Drewry <wad@chromium.org>
---
 fs/partitions/efi.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/efi.h |    5 ++++
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index 9efb2cf..4f642c5 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -633,3 +633,64 @@ int efi_partition(struct parsed_partitions *state)
 	printk("\n");
 	return 1;
 }
+
+/**
+ * efi_partition_by_guid
+ * @bdev:	Whole block device to scan for a GPT.
+ * @guid:	Unique identifier for the partition to find.
+ *
+ * N.b., returns on the first match since it should be unique.
+ *
+ * Returns:
+ * -1 if an error ...
[ message continues ]

From: Will Drewry <wad@chromium.org>

Please put the openning brace on a new line.
--

[ message continues ]

Thanks! I'll post a new version with that fixed as well as the
alternative approach I mentioned.

cheers!
will
--

[ message continues ]

EFI's GPT partitioning scheme expects that all partitions have a unique
identifiers.  After initial partition scanning, this information is
completely lost to the rest of the kernel.

efi_partition_by_guid exposes GPT parsing support in a limited fashion
to allow other portions of the kernel to map a partition from GUID to
map.

This change is motivated the desire to have the ability to specify a
UUID argument to a device mapper target allowing it to select the device
by the UUID.

Change against Linus's tree: 9fe6206f400646a2322096b56c59891d530e8d51

Signed-off-by: Will Drewry <wad@chromium.org>

v2: pr_debug(KERN_WARNING -> pr_debug(. joe@perches.com
    moved down trailing {. davem@davemloft.net
---
 fs/partitions/efi.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/efi.h |    5 ++++
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index 9efb2cf..8669c4f 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -633,3 +633,64 @@ int efi_partition(struct parsed_partitions *state)
 	printk("\n");
 	return 1;
 }
+
+/**
+ * efi_partition_by_guid
+ * @bdev:	Whole block device to scan for a GPT.
+ * @guid:	Unique identifier for the partition to find.
+ *
+ * N.b., returns on the first match since it should be unique.
+ *
+ * Returns:
+ * -1 if an error occurred
+ *  0 if there was no match (or not GPT)
+ *  >=1 is the index of the partition found.
+ *
+ */
+int efi_partition_by_guid(struct block_device *bdev, efi_guid_t *guid) {
+	gpt_header *gpt = NULL;
+	gpt_entry *ptes = NULL;
+	u32 i;
+	struct parsed_partitions *state;
+	int part = 0;
+
+	if (!bdev || !guid)
+		return -1;
+
+	state = kzalloc(sizeof(struct parsed_partitions), GFP_KERNEL);
+	if (!state)
+		return -1;
+
+	state->limit = disk_max_parts(bdev->bd_disk);
+	pr_debug("efi_find_partition looking for gpt\n");
+
+	state->bdev = bdev;
+	if (!find_valid_gpt(state,  &gpt, &ptes) || !gpt || !ptes) ...
[ message continues ]

That opening brace should be on a line by itself:

int efi_partition_by_guid(struct block_device *bdev, efi_guid_t *guid)
{

and the kernel-doc function description should be like so:

/**
 * efi_partition_by_guid - find the first (only) matching guid on a block device




---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--

[ message continues ]

I'll be sure to use the proper format on the next patch round!

thanks!
--

[ message continues ]

efi_partition_walk provides a generic mechanism for iterating over a partition
table using a callback over the GPT entry data.  However, it avoids exposing
too many internals while still providing less specialized access than the
previous patch.

The same questions apply - any and all feedback is truly appreciated.

I'll post an updated version of the original with feedback from Joe Perches and
David Miller shortly.

Signed-off-by: Will Drewry <wad@chromium.org>
---
 fs/partitions/efi.c |  123 ++++++++++++++++++++++++++++++++++++++-------------
 include/linux/efi.h |    9 ++++
 2 files changed, 101 insertions(+), 31 deletions(-)

diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index 9efb2cf..cfb52ac 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -580,56 +580,117 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
 }
 
 /**
- * efi_partition(struct parsed_partitions *state)
- * @state
+ * efi_partition_walk
+ * @bdev:	Whole block device to scan for a GPT.
+ * @walk_arg:	Opaque data to pass to the walker
+ * @walk:	Function to walk valid GPT entries
  *
- * Description: called from check.c, if the disk contains GPT
- * partitions, sets up partition entries in the kernel.
+ * If walk() returns non-zero, then the walk will terminate early.
  *
- * If the first block on the disk is a legacy MBR,
- * it will get handled by msdos_partition().
- * If it's a Protective MBR, we'll handle it here.
+ * We ignore the GPT entry so the first number refers to the first
+ * data partition.
  *
- * We do not create a Linux partition for GPT, but
- * only for the actual data partitions.
  * Returns:
- * -1 if unable to read the partition table
- *  0 if this isn't our partition table
- *  1 if successful
+ * -1   if an error occurred or unable to read
+ *  0   if there is no gpt entry
+ *  1   if traversal occurred (even if terminated early)
  *
  */
-int efi_partition(struct parsed_partitions *state)
+int ...
[ message continues ]

(cc'ing Kay and quoting whole message for him)

Kay, you were talking about using GUID in GPT for finding out root
device and so on.  Does this fit your use case too?  If not it would
be nice to find out something which can be shared.


-- 
tejun
--

[ message continues ]

Yeah, we have something similar in mind since a while, to be able to
safely boot a box without an initramfs, and to be able to to specify
something like:
  root=PARTUUID=6547567-575-7567-567567-57
  root=PARTLABEL=foo
on the kernel commandline.

The current 'blkid' already reports stuff like, to have the same
information in userspace:
  $ blkid -p -oudev /dev/sde1
  ID_FS_LABEL=10GB
  ID_FS_LABEL_ENC=10GB
  ID_FS_UUID=5aafa1bb-70a7-4fe6-b93f-30658ec99fac
  ID_FS_UUID_ENC=5aafa1bb-70a7-4fe6-b93f-30658ec99fac
  ID_FS_VERSION=1.0
  ID_FS_TYPE=ext4
  ID_FS_USAGE=filesystem
  ID_PART_ENTRY_SCHEME=gpt
  ID_PART_ENTRY_UUID=1f765dcb-5214-bd47-b1c5-f2f18848335e
  ID_PART_ENTRY_TYPE=a2a0d0eb-e5b9-3344-87c0-68b6b72699c7
  ID_PART_ENTRY_NUMBER=1

I guess we want to store these identifiers directly into the partition
structure, independent of the partition format, so any code can
register a callback for a new block device, and can just check if
that's the device in question. Walking the block devices would just be
something usual provided by the driver core, instead of having some
specific EFI walk functions.

Kay
--

[ message continues ]

Cool. So I'd like this as well (at least the UUID part), and I'd like
this to be available for other consumers in the kernel, like
dm_get_device() or at least for mapped device targets to implement
support for themselves.  (I have a separate patch for
mimicking md= for device mapper devices which I should probably post

Yeah - when I use this function, I end up doing a walk over all the
block devices, checking if they are whole disk entries, then calling
the efi_partition_by_guid() function. (Or the walker which I posted
separately.)  It's not ideal but it has the smallest impact on the
existing code. (Not having disk_type available is irritating though.)

Would the type GUID and unique GUID be viable additions to a more
public struct?  If they were CONFIG_EFI_PARTITION guarded, then they
wouldn't waste memory for systems without GPT support, but it seems a
bit specific.  Also, I don't think it'd make sense to put it in the
partition struct as that represents the on-disk format for some tables
(from a quick scan over the code).  However, hd_struct looks the
sanest to me.

I'd be happy to pull together a potential change that exposes this
data once after disk (re)scan, but I'd hate to do so in a way that'd
be fundamentally unacceptable (but I don't want to end up down the

So I could see something like:

struct hd_struct {
...
#ifdef CONFIG_EFI_PARTITION
  efi_guid_t type_guid;
  efi_guid_t uuid;
  u16 label[72 / ...];
};

Alternatively, a slightly more generic option might be:

#ifdef CONFIG_PARTITION_INFO
  /* ASCII hex-formatted uuids inclusive of hyphens */
  u8 type_guid[MAX_HD_STRUCT_UUID_SIZE];
  u8 uuid[MAX_HD_STRUCT_UUID_SIZE];
  u16 label[MAX_HD_STRUCT_NAME + sizeof(u16)];
#endif


Any way, if any of this seems slightly palatable, let me know.  I'd
love to make this data accessible to the rest of the kernel.

thanks!
will
--

[ message continues ]

Maybe we go for a single pointer in the partition device, and allocate
a struct partition_meta_info, or something like this, if we have such
data to store. In that structure we can add all needed fields we need?
That would not really waste anything if it's not needed, or it can
possibly be free()d later, if nothing needs it anymore.

Kay
--

[ message continues ]

Sounds reasonable to me.  I'll see what I can cook up and post it back
to this thread.

cheers!
will
--

[ message continues ]

This change extends the partition_meta_info structure to
support EFI GPT-specific metadata and ensures that data
is copied in on partition scanning.

Adding this information would make it possible to identify a
partition by GUID using something like disk_part_iter_*(),
calls that make hd_struct accessible, or even class_find_device.

Signed-off-by: Will Drewry <wad@chromium.org>
---
 fs/partitions/efi.c   |   16 ++++++++++++++++
 include/linux/genhd.h |   12 ++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index 9efb2cf..2880b33 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -614,6 +614,7 @@ int efi_partition(struct parsed_partitions *state)
 	pr_debug("GUID Partition Table is valid!  Yea!\n");
 
 	for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1; i++) {
+		struct partition_meta_info *info;
 		u64 start = le64_to_cpu(ptes[i].starting_lba);
 		u64 size = le64_to_cpu(ptes[i].ending_lba) -
 			   le64_to_cpu(ptes[i].starting_lba) + 1ULL;
@@ -627,6 +628,21 @@ int efi_partition(struct parsed_partitions *state)
 		if (!efi_guidcmp(ptes[i].partition_type_guid,
 				 PARTITION_LINUX_RAID_GUID))
 			state->parts[i + 1].flags = ADDPART_FLAG_RAID;
+
+		info = alloc_part_info(NULL);
+		if (!info) {
+			printk(KERN_WARNING
+			       "unable to allocate memory for part->info\n");
+			continue;
+		}
+		state->parts[i + 1].info = info;
+		info->format = PARTITION_META_INFO_FORMAT_EFI;
+		memcpy(info->efi.uuid.b, ptes[i].unique_partition_guid.b,
+			sizeof(info->efi.uuid.b));
+		memcpy(info->efi.type.b, ptes[i].partition_type_guid.b,
+			sizeof(info->efi.type.b));
+		memcpy(info->efi.label, ptes[i].partition_name,
+			sizeof(info->efi.label));
 	}
 	kfree(ptes);
 	kfree(gpt);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 7b6644a..e0a742f 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -91,11 +91,23 @@ struct disk_stats {
 ...
[ message continues ]

Wow, you are fast. :) Sounds and looks good to me.

I guess we should assign the meta structure only after all values are
filled in? Otherwise we could get partial reads from a possible user?

Did you already test to put a lookup-call to the in-kernel mounter, if
we use some special partition table uuid identifier for root=? That
would be nice to see, if all that works as expected, and we can get to
the data we collect.

Kay
--

[ message continues ]

In add_partition(), I don't think the partition itself is shared until
rcu_assign_pointer is called, and I don't think that parsed_partitions
is safe to be shared inside check.c, but perhaps that's a dangerous
assumption

I can certainly move the pointer assignment to occur after the data is

Not yet, but I'd like to have it working there (and in my
device-mapper target) as soon as possible.  Hopefully, I'll have that
done pretty soon and I'll repost the series inclusive of an init:
change.

Any preferences on the variable? I'll start with your example of
PARTUUID=, but that follows the initramfs model (UUID=) and not the
existing magic root devices (/dev/nfs, /dev/ram).
/dev/by-part-uuid/XXX... doesn't seem super-friendly though :)

cheers -
will
--

[ message continues ]

Yeah, this stuff does not really fit in to the path notation stuff,
unless we use the udev-style link names, which some people like to
avoid, and prefer some more abstract thing here.

We have UUID=, LABEL= for mount and fstab, I think PARTUUID=,
PARTLABEL= matches this.

We can always change that, if something better comes up before this
gets official. :)

Kay
--

[ message continues ]

This change extends the partition_meta_info structure to
support EFI GPT-specific metadata and ensures that data
is copied in on partition scanning.

Signed-off-by: Will Drewry <wad@chromium.org>

v2: move info assignment after memcpy()s
    adds _MAX to the enum after EFI
---
 fs/partitions/efi.c   |   16 ++++++++++++++++
 include/linux/genhd.h |   14 ++++++++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index 9efb2cf..fac527d 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -614,6 +614,7 @@ int efi_partition(struct parsed_partitions *state)
 	pr_debug("GUID Partition Table is valid!  Yea!\n");
 
 	for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1; i++) {
+		struct partition_meta_info *info;
 		u64 start = le64_to_cpu(ptes[i].starting_lba);
 		u64 size = le64_to_cpu(ptes[i].ending_lba) -
 			   le64_to_cpu(ptes[i].starting_lba) + 1ULL;
@@ -627,6 +628,21 @@ int efi_partition(struct parsed_partitions *state)
 		if (!efi_guidcmp(ptes[i].partition_type_guid,
 				 PARTITION_LINUX_RAID_GUID))
 			state->parts[i + 1].flags = ADDPART_FLAG_RAID;
+
+		info = alloc_part_info(NULL);
+		if (!info) {
+			printk(KERN_WARNING
+			       "unable to allocate memory for part->info\n");
+			continue;
+		}
+		info->format = PARTITION_META_INFO_FORMAT_EFI;
+		memcpy(info->efi.uuid.b, ptes[i].unique_partition_guid.b,
+			sizeof(info->efi.uuid.b));
+		memcpy(info->efi.type.b, ptes[i].partition_type_guid.b,
+			sizeof(info->efi.type.b));
+		memcpy(info->efi.label, ptes[i].partition_name,
+			sizeof(info->efi.label));
+		state->parts[i + 1].info = info;
 	}
 	kfree(ptes);
 	kfree(gpt);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 7b6644a..beb98e3 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -91,11 +91,25 @@ struct disk_stats {
 enum partition_meta_info_format_t {
 	/* Partition info format */
 	PARTITION_META_INFO_FORMAT_NONE = ...
[ message continues ]

Hello,


It would be nice if uuid can be made a common field outside of the
union so that generic code which only cares about UUID can simply read
it off.

Thanks.	

-- 
tejun
--

[ message continues ]

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


the partition name is in UTF8LE, is it correct to use it in raw

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com
--

[ message continues ]

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

[ message continues ]

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'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.


Agreed. I don't think labels make sense, but we may later want to
support partition types (as I mention in my other mail).
--

[ message continues ]

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

Kay
--

[ message continues ]

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.
--

[ message continues ]

This change extends the partition_meta_info structure to
support EFI GPT-specific metadata and ensures that data
is copied in on partition scanning.

Signed-off-by: Will Drewry <wad@chromium.org>

v2: move info assignment after memcpy()s
    adds _MAX to the enum after EFI
v3: exports a generic uuid/volname format
---
 fs/partitions/efi.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index 9efb2cf..427f2fb 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -94,6 +94,7 @@
  *
  ************************************************************/
 #include <linux/crc32.h>
+#include <linux/ctype.h>
 #include <linux/math64.h>
 #include <linux/slab.h>
 #include "check.h"
@@ -604,6 +605,7 @@ int efi_partition(struct parsed_partitions *state)
 	gpt_entry *ptes = NULL;
 	u32 i;
 	unsigned ssz = bdev_logical_block_size(state->bdev) / 512;
+	u8 unparsed_guid[37];
 
 	if (!find_valid_gpt(state, &gpt, &ptes) || !gpt || !ptes) {
 		kfree(gpt);
@@ -614,6 +616,9 @@ int efi_partition(struct parsed_partitions *state)
 	pr_debug("GUID Partition Table is valid!  Yea!\n");
 
 	for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1; i++) {
+		struct partition_meta_info *info;
+		unsigned label_count = 0;
+		unsigned label_max;
 		u64 start = le64_to_cpu(ptes[i].starting_lba);
 		u64 size = le64_to_cpu(ptes[i].ending_lba) -
 			   le64_to_cpu(ptes[i].starting_lba) + 1ULL;
@@ -627,6 +632,26 @@ int efi_partition(struct parsed_partitions *state)
 		if (!efi_guidcmp(ptes[i].partition_type_guid,
 				 PARTITION_LINUX_RAID_GUID))
 			state->parts[i + 1].flags = ADDPART_FLAG_RAID;
+
+		info = &state->parts[i + 1].info;
+		/* Instead of doing a manual swap to big endian, reuse the
+		 * common ASCII hex format as the interim.
+		 */
+		efi_guid_unparse(&ptes[i].unique_partition_guid, unparsed_guid);
+		part_pack_uuid(unparsed_guid, info->uuid);
+
+		/* Naively ...
[ message continues ]

This is the third patch in a series which adds support for
storing partition metadata, optionally, off of the hd_struct.

One major use for that data is being able to resolve partition
by other identities than just the index on a block device.  Device
enumeration varies by platform and there's a benefit to being able
to use something like EFI GPT's GUIDs to determine the correct
block device and partition to mount as the root.

This change adds that support to root= by adding support for
the following syntax:

  root=PARTUUID=hex-uuid

Signed-off-by: Will Drewry <wad@chromium.org>

v2: added to patch series
v3: adds uuids to printk_all_partitions
    simplifies matching to a uniform uuid packed format (big endian)
---
 block/genhd.c    |    9 +++++-
 init/do_mounts.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index c8da120..5c9c503 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -642,6 +642,7 @@ void __init printk_all_partitions(void)
 		struct hd_struct *part;
 		char name_buf[BDEVNAME_SIZE];
 		char devt_buf[BDEVT_SIZE];
+		u8 uuid[PARTITION_META_INFO_UUIDLTH * 2 + 1];
 
 		/*
 		 * Don't show empty devices or things that have been
@@ -660,10 +661,14 @@ void __init printk_all_partitions(void)
 		while ((part = disk_part_iter_next(&piter))) {
 			bool is_part0 = part == &disk->part0;
 
-			printk("%s%s %10llu %s", is_part0 ? "" : "  ",
+			uuid[0] = 0;
+			if (part->info)
+				part_unpack_uuid(part->info->uuid, uuid);
+
+			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
 			       bdevt_str(part_devt(part), devt_buf),
 			       (unsigned long long)part->nr_sects >> 1,
-			       disk_name(disk, part->partno, name_buf));
+			       disk_name(disk, part->partno, name_buf), uuid);
 			if (is_part0) {
 				if (disk->driverfs_dev != NULL &&
 				    disk->driverfs_dev->driver != NULL)
diff --git a/init/do_mounts.c b/init/do_mounts.c
index ...
[ message continues ]

Hello,


Hmmm... I'm a bit worried about this.  This might break userspace
tools.  I think it would be better to export it via sysfs.

Thanks.

-- 
tejun
--

[ message continues ]

Cool - I'm happy to drop that part from the patch.  I was more
following the spirit of the comment around making it easy for a user
to figure out what went wrong (e.g., wrong uuid) if they don't have a
root, but if there are tools parsing that output, I'd rather not break
them!

Would sysfs make sense as part of this patch series or would it be
fair game for a follow-on?  I'm inclined to wait since the UUIDs can't
be changed (blkpg support isn't wired up) except with rescans, and a
userspace tool can just walk the partition table to get the uuids.

Thanks!
--

[ message continues ]

Hello,


Oh, forget about what I said.  For some reason I thought the above
code was for /proc/partitions.  :-)

Thanks.

-- 
tejun
--

[ message continues ]

Cool - even better!

Thanks!
will
--

[ message continues ]

Just an update:

There is still no better idea than using this notation. We should
distinguish between filesystem and partiton UUIDs, and overloading
/dev with magic strings that will never exist there doesn't sound like
a good idea.

I checked with Karel, and fstab supports UUID=, initramfs tools
support root=UUID=, so we are probably going to have PARTUUID support
in fstab and initramfs too when we get there.

Kay
--

[ message continues ]

I'm reposting this patch series as v4 since there have been no additional
comments, and I cleaned up one extra bit of unneeded code (in 3/3). The patches
are against Linus's tree: 2bfc96a127bc1cc94d26bfaa40159966064f9c8c
(2.6.36-rc3).

Would this patchset be suitable for inclusion in an mm branch?


This changes adds a partition_meta_info struct which itself contains a
union of structures that provide partition table specific metadata.

This change leaves the union empty. The subsequent patch includes an
implementation for CONFIG_EFI_PARTITION-based metadata.

Signed-off-by: Will Drewry <wad@chromium.org>

---
v2: move assignment of p->info after copy
v3: make partition_meta_info part of parsed_partitions (not a ptr)
    get rid of extended partition-format specific information
    add generic pack/unpack uuid helpers
v4: include genhd.h in check.h

 block/genhd.c         |    1 +
 block/ioctl.c         |    2 +-
 fs/partitions/check.c |   23 ++++++++++++++++++--
 fs/partitions/check.h |    3 ++
 include/linux/genhd.h |   53 +++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 59a2db6..c8da120 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1004,6 +1004,7 @@ static void disk_release(struct device *dev)
 	kfree(disk->random);
 	disk_replace_part_tbl(disk, NULL);
 	free_part_stats(&disk->part0);
+	free_part_info(&disk->part0);
 	kfree(disk);
 }
 struct class block_class = {
diff --git a/block/ioctl.c b/block/ioctl.c
index d8052f0..2c15fe0 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -62,7 +62,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 
 			/* all seems OK */
 			part = add_partition(disk, partno, start, length,
-					     ADDPART_FLAG_NONE);
+					     ADDPART_FLAG_NONE, NULL);
 			mutex_unlock(&bdev->bd_mutex);
 			return IS_ERR(part) ? PTR_ERR(part) : 0;
 		case BLKPG_DEL_PARTITION:
diff --git ...
[ message continues ]

I've included them in the for-2.6.37/core branch.

-- 
Jens Axboe

--

[ message continues ]

This change extends the partition_meta_info structure to
support EFI GPT-specific metadata and ensures that data
is copied in on partition scanning.

Signed-off-by: Will Drewry <wad@chromium.org>

---
v2: move info assignment after memcpy()s
    adds _MAX to the enum after EFI
v3: exports a generic uuid/volname format
v4: no changes

 fs/partitions/efi.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index dbb44d4..ac0ccb5 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -94,6 +94,7 @@
  *
  ************************************************************/
 #include <linux/crc32.h>
+#include <linux/ctype.h>
 #include <linux/math64.h>
 #include <linux/slab.h>
 #include "check.h"
@@ -604,6 +605,7 @@ int efi_partition(struct parsed_partitions *state)
 	gpt_entry *ptes = NULL;
 	u32 i;
 	unsigned ssz = bdev_logical_block_size(state->bdev) / 512;
+	u8 unparsed_guid[37];
 
 	if (!find_valid_gpt(state, &gpt, &ptes) || !gpt || !ptes) {
 		kfree(gpt);
@@ -614,6 +616,9 @@ int efi_partition(struct parsed_partitions *state)
 	pr_debug("GUID Partition Table is valid!  Yea!\n");
 
 	for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1; i++) {
+		struct partition_meta_info *info;
+		unsigned label_count = 0;
+		unsigned label_max;
 		u64 start = le64_to_cpu(ptes[i].starting_lba);
 		u64 size = le64_to_cpu(ptes[i].ending_lba) -
 			   le64_to_cpu(ptes[i].starting_lba) + 1ULL;
@@ -627,6 +632,26 @@ int efi_partition(struct parsed_partitions *state)
 		if (!efi_guidcmp(ptes[i].partition_type_guid,
 				 PARTITION_LINUX_RAID_GUID))
 			state->parts[i + 1].flags = ADDPART_FLAG_RAID;
+
+		info = &state->parts[i + 1].info;
+		/* Instead of doing a manual swap to big endian, reuse the
+		 * common ASCII hex format as the interim.
+		 */
+		efi_guid_unparse(&ptes[i].unique_partition_guid, unparsed_guid);
+		part_pack_uuid(unparsed_guid, ...
[ message continues ]

This is the third patch in a series which adds support for
storing partition metadata, optionally, off of the hd_struct.

One major use for that data is being able to resolve partition
by other identities than just the index on a block device.  Device
enumeration varies by platform and there's a benefit to being able
to use something like EFI GPT's GUIDs to determine the correct
block device and partition to mount as the root.

This change adds that support to root= by adding support for
the following syntax:

  root=PARTUUID=hex-uuid

Signed-off-by: Will Drewry <wad@chromium.org>

---
v2: added to patch series
v3: adds uuids to printk_all_partitions
    simplifies matching to a uniform uuid packed format (big endian)
v4: no changes

 block/genhd.c    |    9 +++++-
 init/do_mounts.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index c8da120..5c9c503 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -642,6 +642,7 @@ void __init printk_all_partitions(void)
 		struct hd_struct *part;
 		char name_buf[BDEVNAME_SIZE];
 		char devt_buf[BDEVT_SIZE];
+		u8 uuid[PARTITION_META_INFO_UUIDLTH * 2 + 1];
 
 		/*
 		 * Don't show empty devices or things that have been
@@ -660,10 +661,14 @@ void __init printk_all_partitions(void)
 		while ((part = disk_part_iter_next(&piter))) {
 			bool is_part0 = part == &disk->part0;
 
-			printk("%s%s %10llu %s", is_part0 ? "" : "  ",
+			uuid[0] = 0;
+			if (part->info)
+				part_unpack_uuid(part->info->uuid, uuid);
+
+			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
 			       bdevt_str(part_devt(part), devt_buf),
 			       (unsigned long long)part->nr_sects >> 1,
-			       disk_name(disk, part->partno, name_buf));
+			       disk_name(disk, part->partno, name_buf), uuid);
 			if (is_part0) {
 				if (disk->driverfs_dev != NULL &&
 				    disk->driverfs_dev->driver != NULL)
diff --git a/init/do_mounts.c ...
[ message continues ]

This changes adds a partition_meta_info struct which itself contains a
union of structures that provide partition table specific metadata.

This change leaves the union empty. The subsequent patch includes an
implementation for CONFIG_EFI_PARTITION-based metadata.

Signed-off-by: Will Drewry <wad@chromium.org>

v2: move assignment of p->info after copy
v3: make partition_meta_info part of parsed_partitions (not a ptr)
    get rid of extended partition-format specific information
      (this could be easily be added to the struct at any point later)
    add generic pack/unpack uuid helpers
---
 block/genhd.c         |    1 +
 block/ioctl.c         |    2 +-
 fs/partitions/check.c |   23 ++++++++++++++++++--
 fs/partitions/check.h |    3 ++
 include/linux/genhd.h |   53 +++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 59a2db6..c8da120 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1004,6 +1004,7 @@ static void disk_release(struct device *dev)
 	kfree(disk->random);
 	disk_replace_part_tbl(disk, NULL);
 	free_part_stats(&disk->part0);
+	free_part_info(&disk->part0);
 	kfree(disk);
 }
 struct class block_class = {
diff --git a/block/ioctl.c b/block/ioctl.c
index e8eb679..f8e4bfe 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -62,7 +62,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 
 			/* all seems OK */
 			part = add_partition(disk, partno, start, length,
-					     ADDPART_FLAG_NONE);
+					     ADDPART_FLAG_NONE, NULL);
 			mutex_unlock(&bdev->bd_mutex);
 			return IS_ERR(part) ? PTR_ERR(part) : 0;
 		case BLKPG_DEL_PARTITION:
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 5dcd4b0..ec9c8b6 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -338,6 +338,7 @@ static void part_release(struct device *dev)
 {
 	struct hd_struct *p = dev_to_part(dev);
 ...
[ message continues ]

For GPT, at least, the partition type isn't directly encoded anywhere
else and it can be vendor specific.  For instance, if a vendor wants
to make an auto-configuring, they could reserve a
VENDOR-ROOT-PRI0-PART-TYPE-GUID then easily create a mount path for

Since the type was partition specific, I left the data untouched. It
sounds like it'd be more useful to expose the data in a generic way
--

[ message continues ]

This is the third patch in a series which adds support for
storing partition metadata, optionally, off of the hd_struct.

One major use for that data is being able to resolve partition
by other identities than just the index on a block device.  Device
enumeration varies by platform and there's a benefit to being able
to use something like EFI GPT's GUIDs to determine the correct
block device and partition to mount as the root.

This change adds that support to root= by adding support for
the following syntax:

  root=PARTUUID=hex-uuid

I can't say this is the best way to do it, but it should be reasonably
clean to extend. There are a number of alternate approaches, and I'd love to
hear what is preferred.

Signed-off-by: Will Drewry <wad@chromium.org>
---
 init/do_mounts.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 02e3ca4..7b22abf 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -58,6 +58,78 @@ static int __init readwrite(char *str)
 __setup("ro", readonly);
 __setup("rw", readwrite);
 
+/**
+ * match_dev_by_uuid - callback for finding a partition using its uuid
+ * @dev:	device passed in by the caller
+ * @data:	opaque pointer to a 36 byte char array with a UUID
+ *
+ * Returns 1 if the device matches, and 0 otherwise.
+ */
+static int __init match_dev_by_uuid(struct device *dev, void *data)
+{
+	u8 *uuid = data;
+	char candidate[37];
+	struct hd_struct *part = dev_to_part(dev);
+
+	if (!part->info)
+		goto no_match;
+
+	/* Each format may parse UUIDs differently.  To that end,
+	 * each format will have to parse either the given uuid or
+	 * the candidate partition.  It'd be more efficient to parse
+	 * prior to the walk, but we'd need to store all the possible
+	 * parsed guids.
+	 */
+	switch (part->info->format) {
+#ifdef CONFIG_EFI_PARTITION
+	case PARTITION_META_INFO_FORMAT_EFI:
+		snprintf(candidate, ...
[ message continues ]

This changes adds a partition_meta_info struct which itself contains a
union of structures that provide partition table specific metadata.

This change leaves the union empty. The subsequent patch includes an
implementation for CONFIG_EFI_PARTITION-based metadata.

Signed-off-by: Will Drewry <wad@chromium.org>

v2: move assignment of p->info after copy
---
 block/genhd.c         |    1 +
 block/ioctl.c         |    2 +-
 fs/partitions/check.c |   22 +++++++++++++++++++---
 fs/partitions/check.h |    2 ++
 include/linux/genhd.h |   32 ++++++++++++++++++++++++++++++--
 5 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 59a2db6..c8da120 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1004,6 +1004,7 @@ static void disk_release(struct device *dev)
 	kfree(disk->random);
 	disk_replace_part_tbl(disk, NULL);
 	free_part_stats(&disk->part0);
+	free_part_info(&disk->part0);
 	kfree(disk);
 }
 struct class block_class = {
diff --git a/block/ioctl.c b/block/ioctl.c
index e8eb679..f8e4bfe 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -62,7 +62,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 
 			/* all seems OK */
 			part = add_partition(disk, partno, start, length,
-					     ADDPART_FLAG_NONE);
+					     ADDPART_FLAG_NONE, NULL);
 			mutex_unlock(&bdev->bd_mutex);
 			return IS_ERR(part) ? PTR_ERR(part) : 0;
 		case BLKPG_DEL_PARTITION:
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 5dcd4b0..b616da8 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -196,6 +196,8 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 		printk(" unknown partition table\n");
 	else if (warn_no_part)
 		printk(" unable to read partition table\n");
+	for (i = 0; i < DISK_MAX_PARTS; ++i)
+		kfree(state->parts[i].info);
 	kfree(state);
 	return ERR_PTR(res);
 }
@@ -338,6 +340,7 @@ static void part_release(struct device *dev)
 {
 	struct hd_struct ...
[ message continues ]

Hello,



But you can just embed the structure here.  It's a temp data structure
to make things easier for individual partition scan code.  There's no
need to save bytes.

Thanks.

-- 
tejun
--

[ message continues ]

Great - that'll simplify the code more.

Thanks!
--

[ message continues ]

>>>>> "Kay" == Kay Sievers <kay.sievers@vrfy.org> writes:



Kay> Yeah, this stuff does not really fit in to the path notation
Kay> stuff, unless we use the udev-style link names, which some people
Kay> like to avoid, and prefer some more abstract thing here.

Kay> We have UUID=, LABEL= for mount and fstab, I think PARTUUID=,
Kay> PARTLABEL= matches this.

Ugh, why do we care whether a UUID refers to a disk or a partition?
It should be unique no matter what, so just do /dev/uuid/XXXX and call
it done.

John
--

[ message continues ]

Since these are supposed to be _unique_ it seems that any reference to
a uuid should be fair game.  And if a root=/dev/uuid/XXX makes it to
an initramfs environment, then that'll be able to easily check if the
UUID refers to a filesystem uuid or not.

I can roll that into the next version unless there's specific complaints.
--

[ message continues ]

Because it is not a filesystem UUID, and that matters to distinguish.

I don't think so. People should stop encoding non-existing stuff in
the /dev namespace. :)

Thanks,
Kay
--

[ message continues ]

This changes adds a partition_meta_info struct which itself contains a
union of structures that provide partition table specific metadata.

This change leaves the union empty. The subsequent patch includes an
implementation for CONFIG_EFI_PARTITION-based metadata.

Signed-off-by: Will Drewry <wad@chromium.org>
---
 block/genhd.c         |    1 +
 block/ioctl.c         |    2 +-
 fs/partitions/check.c |   21 ++++++++++++++++++---
 fs/partitions/check.h |    2 ++
 include/linux/genhd.h |   32 ++++++++++++++++++++++++++++++--
 5 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 59a2db6..c8da120 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1004,6 +1004,7 @@ static void disk_release(struct device *dev)
 	kfree(disk->random);
 	disk_replace_part_tbl(disk, NULL);
 	free_part_stats(&disk->part0);
+	free_part_info(&disk->part0);
 	kfree(disk);
 }
 struct class block_class = {
diff --git a/block/ioctl.c b/block/ioctl.c
index e8eb679..f8e4bfe 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -62,7 +62,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 
 			/* all seems OK */
 			part = add_partition(disk, partno, start, length,
-					     ADDPART_FLAG_NONE);
+					     ADDPART_FLAG_NONE, NULL);
 			mutex_unlock(&bdev->bd_mutex);
 			return IS_ERR(part) ? PTR_ERR(part) : 0;
 		case BLKPG_DEL_PARTITION:
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 5dcd4b0..635725a 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -196,6 +196,8 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 		printk(" unknown partition table\n");
 	else if (warn_no_part)
 		printk(" unable to read partition table\n");
+	for (i = 0; i < DISK_MAX_PARTS; ++i)
+		kfree(state->parts[i].info);
 	kfree(state);
 	return ERR_PTR(res);
 }
@@ -338,6 +340,7 @@ static void part_release(struct device *dev)
 {
 	struct hd_struct *p = dev_to_part(dev);
 ...
[ message continues ]