All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support
@ 2011-06-30 21:10 Will Drewry
  2011-06-30 21:10 ` [PATCH v3 2/2] Documentation: add pointer to name_to_dev_t for root= values Will Drewry
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Will Drewry @ 2011-06-30 21:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: kay.sievers, akpm, Will Drewry, Jens Axboe, Namhyung Kim,
	Trond Myklebust

Expand root=PARTUUID=UUID syntax to support selecting a root partition
by integer offset from a known, unique partition.  This approach
provides similar properties to specifying a device and partition number,
but using the UUID as the unique path prior to evaluating the offset.

For example,
  root=PARTUUID=99DE9194-FC15-4223-9192-FC243948F88B/PARTNROFF=1
selects the partition with UUID 99DE.. then select the next
partition.

This change is motivated by a particular usecase in Chromium OS where
the bootloader can easily determine what partition it is on (by UUID)
but doesn't perform general partition table walking.

That said, support for this model provides a direct mechanism for the
user to modify the root partition to boot without specifically needing
to extract each UUID or update the bootloader explicitly when the root
partition UUID is changed (if it is recreated to be larger, for
instance).  Pinning to a /boot-style partition UUID allows the arbitrary
root partition reconfiguration/modifications with slightly less
ambiguity than just [dev][partition] and less stringency than the
specific root partition UUID.

v3: - emit syntax error warning to KERN_ERR
    - fail on poorly formed syntax
    - use / instead of , as a separator
v2: - move from +/-%u to ,PARTNR=%d (as per Kay)

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

diff --git a/init/do_mounts.c b/init/do_mounts.c
index c0851a8..13cddea 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -85,12 +85,15 @@ no_match:
 
 /**
  * devt_from_partuuid - looks up the dev_t of a partition by its UUID
- * @uuid:	36 byte char array containing a hex ascii UUID
+ * @uuid:	min 36 byte char array containing a hex ascii UUID
  *
  * The function will return the first partition which contains a matching
  * UUID value in its partition_meta_info struct.  This does not search
  * by filesystem UUIDs.
  *
+ * If @uuid is followed by a "/PARTNROFF=%d", then the number will be
+ * extracted and used as an offset from the partition identified by the UUID.
+ *
  * Returns the matching dev_t on success or 0 on failure.
  */
 static dev_t devt_from_partuuid(char *uuid_str)
@@ -98,6 +101,22 @@ static dev_t devt_from_partuuid(char *uuid_str)
 	dev_t res = 0;
 	struct device *dev = NULL;
 	u8 uuid[16];
+	struct gendisk *disk;
+	struct hd_struct *part;
+	int offset = 0;
+
+	if (strlen(uuid_str) < 36)
+		goto done;
+
+	/* Check for optional partition number offset attributes. */
+	if (uuid_str[36]) {
+		/* Explicitly fail on poor PARTUUID syntax. */
+		if (sscanf(&uuid_str[36], "/PARTNROFF=%d", &offset) != 1) {
+			printk(KERN_ERR "VFS: PARTUUID= is invalid.\n"
+			 "Expected PARTUUID=<valid-uuid-id>[/PARTNROFF=%%d]\n");
+			goto done;
+		}
+	}
 
 	/* Pack the requested UUID in the expected format. */
 	part_pack_uuid(uuid_str, uuid);
@@ -107,8 +126,21 @@ static dev_t devt_from_partuuid(char *uuid_str)
 		goto done;
 
 	res = dev->devt;
-	put_device(dev);
 
+	/* Attempt to find the partition by offset. */
+	if (!offset)
+		goto no_offset;
+
+	res = 0;
+	disk = part_to_disk(dev_to_part(dev));
+	part = disk_get_part(disk, dev_to_part(dev)->partno + offset);
+	if (part) {
+		res = part_devt(part);
+		put_device(part_to_dev(part));
+	}
+
+no_offset:
+	put_device(dev);
 done:
 	return res;
 }
@@ -126,6 +158,8 @@ done:
  *	   used when disk name of partitioned disk ends on a digit.
  *	6) PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF representing the
  *	   unique id of a partition if the partition table provides it.
+ *	7) PARTUUID=<UUID>/PARTNROFF=<int> to select a partition in relation to
+ *	   a partition with a known unique id.
  *
  *	If name doesn't have fall into the categories above, we return (0,0).
  *	block_class is used to check if something is a disk name. If the disk
@@ -143,8 +177,6 @@ dev_t name_to_dev_t(char *name)
 #ifdef CONFIG_BLOCK
 	if (strncmp(name, "PARTUUID=", 9) == 0) {
 		name += 9;
-		if (strlen(name) != 36)
-			goto fail;
 		res = devt_from_partuuid(name);
 		if (!res)
 			goto fail;
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] Documentation: add pointer to name_to_dev_t for root= values
  2011-06-30 21:10 [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support Will Drewry
@ 2011-06-30 21:10 ` Will Drewry
  2011-07-05 20:53 ` [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support Andrew Morton
  2011-07-25 20:47 ` [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support H. Peter Anvin
  2 siblings, 0 replies; 6+ messages in thread
From: Will Drewry @ 2011-06-30 21:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: kay.sievers, akpm, Will Drewry, Randy Dunlap, linux-doc

Updates kernel-parameters.txt to point users to the authoritative
comment for name_to_dev_t.  In addition, updates other places where some
name_to_dev_t behavior was discussed.  All other references to root=
appear to be for explicit sample usage or just side comments when
discussing other kernel parameters.

Signed-off-by: Will Drewry <wad@chromium.org>
---
 Documentation/frv/booting.txt         |   13 ++++++++++---
 Documentation/kernel-parameters.txt   |    1 +
 Documentation/m68k/kernel-options.txt |   14 ++++++++++++++
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/frv/booting.txt b/Documentation/frv/booting.txt
index ace200b..37c4d84 100644
--- a/Documentation/frv/booting.txt
+++ b/Documentation/frv/booting.txt
@@ -106,13 +106,20 @@ separated by spaces:
       To use the first on-chip serial port at baud rate 115200, no parity, 8
       bits, and no flow control.
 
-  (*) root=/dev/<xxxx>
+  (*) root=<xxxx>
 
-      This specifies the device upon which the root filesystem resides. For
-      example:
+      This specifies the device upon which the root filesystem resides. It
+      may be specified by major and minor number, device path, or even
+      partition uuid, if supported.  For example:
 
 	/dev/nfs	NFS root filesystem
 	/dev/mtdblock3	Fourth RedBoot partition on the System Flash
+	PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF/PARTNROFF=1
+		first partition after the partition with the given UUID
+	253:0		Device with major 253 and minor 0
+
+      Authoritative information can be found in
+      "Documentation/kernel-parameters.txt".
 
   (*) rw
 
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index fd248a3..c8e1737 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2234,6 +2234,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	ro		[KNL] Mount root device read-only on boot
 
 	root=		[KNL] Root filesystem
+			See name_to_dev_t comment in init/do_mounts.c.
 
 	rootdelay=	[KNL] Delay (in seconds) to pause before attempting to
 			mount the root filesystem
diff --git a/Documentation/m68k/kernel-options.txt b/Documentation/m68k/kernel-options.txt
index c93bed6..97d45f2 100644
--- a/Documentation/m68k/kernel-options.txt
+++ b/Documentation/m68k/kernel-options.txt
@@ -129,6 +129,20 @@ decimal 11 is the major of SCSI CD-ROMs, and the minor 0 stands for
 the first of these. You can find out all valid major numbers by
 looking into include/linux/major.h.
 
+In addition to major and minor numbers, if the device containing your
+root partition uses a partition table format with unique partition
+identifiers, then you may use them.  For instance,
+"root=PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF".  It is also
+possible to reference another partition on the same device using a
+known partition UUID as the starting point.  For example,
+if partition 5 of the device has the UUID of
+00112233-4455-6677-8899-AABBCCDDEEFF then partition 3 may be found as
+follows:
+  PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF/PARTNROFF=-2
+
+Authoritative information can be found in
+"Documentation/kernel-parameters.txt".
+
 
 2.2) ro, rw
 -----------
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support
  2011-06-30 21:10 [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support Will Drewry
  2011-06-30 21:10 ` [PATCH v3 2/2] Documentation: add pointer to name_to_dev_t for root= values Will Drewry
@ 2011-07-05 20:53 ` Andrew Morton
  2011-07-06 18:01   ` Will Drewry
  2011-07-25 20:47 ` [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support H. Peter Anvin
  2 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2011-07-05 20:53 UTC (permalink / raw)
  To: Will Drewry
  Cc: linux-kernel, kay.sievers, Jens Axboe, Namhyung Kim, Trond Myklebust

On Thu, 30 Jun 2011 16:10:06 -0500
Will Drewry <wad@chromium.org> wrote:

> Expand root=PARTUUID=UUID syntax to support selecting a root partition
> by integer offset from a known, unique partition.  This approach
> provides similar properties to specifying a device and partition number,
> but using the UUID as the unique path prior to evaluating the offset.
> 
> For example,
>   root=PARTUUID=99DE9194-FC15-4223-9192-FC243948F88B/PARTNROFF=1
> selects the partition with UUID 99DE.. then select the next
> partition.
> 
> This change is motivated by a particular usecase in Chromium OS where
> the bootloader can easily determine what partition it is on (by UUID)
> but doesn't perform general partition table walking.
> 
> That said, support for this model provides a direct mechanism for the
> user to modify the root partition to boot without specifically needing
> to extract each UUID or update the bootloader explicitly when the root
> partition UUID is changed (if it is recreated to be larger, for
> instance).  Pinning to a /boot-style partition UUID allows the arbitrary
> root partition reconfiguration/modifications with slightly less
> ambiguity than just [dev][partition] and less stringency than the
> specific root partition UUID.
> 
> ...
>
>  static dev_t devt_from_partuuid(char *uuid_str)
> @@ -98,6 +101,22 @@ static dev_t devt_from_partuuid(char *uuid_str)
>  	dev_t res = 0;
>  	struct device *dev = NULL;
>  	u8 uuid[16];
> +	struct gendisk *disk;
> +	struct hd_struct *part;
> +	int offset = 0;
> +
> +	if (strlen(uuid_str) < 36)
> +		goto done;

I think this secretly changes behaviour?  Previously the code would have
accepted a less-than-36-byte UUID and would have done <something> with
it.  Now, it fails.

What was <something>, and what is the reason for this (undocumented!)
change?

> +	/* Check for optional partition number offset attributes. */
> +	if (uuid_str[36]) {
> +		/* Explicitly fail on poor PARTUUID syntax. */
> +		if (sscanf(&uuid_str[36], "/PARTNROFF=%d", &offset) != 1) {
> +			printk(KERN_ERR "VFS: PARTUUID= is invalid.\n"
> +			 "Expected PARTUUID=<valid-uuid-id>[/PARTNROFF=%%d]\n");

The check isn't complete - afacit input of the form PARTNROFF=42foo
will be treated as PARTNROFF=42?

>
> ...
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support
  2011-07-05 20:53 ` [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support Andrew Morton
@ 2011-07-06 18:01   ` Will Drewry
  2011-07-25 19:55     ` [PATCH] init: clean up devt_from_partuuid syntax checking and logging Will Drewry
  0 siblings, 1 reply; 6+ messages in thread
From: Will Drewry @ 2011-07-06 18:01 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, kay.sievers, Jens Axboe, Namhyung Kim, Trond Myklebust

On Tue, Jul 5, 2011 at 3:53 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 30 Jun 2011 16:10:06 -0500
> Will Drewry <wad@chromium.org> wrote:
>
>> Expand root=PARTUUID=UUID syntax to support selecting a root partition
>> by integer offset from a known, unique partition.  This approach
>> provides similar properties to specifying a device and partition number,
>> but using the UUID as the unique path prior to evaluating the offset.
>>
>> For example,
>>   root=PARTUUID=99DE9194-FC15-4223-9192-FC243948F88B/PARTNROFF=1
>> selects the partition with UUID 99DE.. then select the next
>> partition.
>>
>> This change is motivated by a particular usecase in Chromium OS where
>> the bootloader can easily determine what partition it is on (by UUID)
>> but doesn't perform general partition table walking.
>>
>> That said, support for this model provides a direct mechanism for the
>> user to modify the root partition to boot without specifically needing
>> to extract each UUID or update the bootloader explicitly when the root
>> partition UUID is changed (if it is recreated to be larger, for
>> instance).  Pinning to a /boot-style partition UUID allows the arbitrary
>> root partition reconfiguration/modifications with slightly less
>> ambiguity than just [dev][partition] and less stringency than the
>> specific root partition UUID.
>>
>> ...
>>
>>  static dev_t devt_from_partuuid(char *uuid_str)
>> @@ -98,6 +101,22 @@ static dev_t devt_from_partuuid(char *uuid_str)
>>       dev_t res = 0;
>>       struct device *dev = NULL;
>>       u8 uuid[16];
>> +     struct gendisk *disk;
>> +     struct hd_struct *part;
>> +     int offset = 0;
>> +
>> +     if (strlen(uuid_str) < 36)
>> +             goto done;
>
> I think this secretly changes behaviour?  Previously the code would have
> accepted a less-than-36-byte UUID and would have done <something> with
> it.  Now, it fails.
>
> What was <something>, and what is the reason for this (undocumented!)
> change?

Nice catch.  Initially and currently, the only caller to
devt_from_partuuid is name_to_dev_t.  name_to_dev_t() was forking on
PARTUUID and length == UUID length.  However, if someone had called
directly into devt_from_partuuid, no bounds checking would've occurred
and out of bounds reads may have resulted.

This just moves the check into devt_from_partuuid to unify the length
checking logic with the functional logic.  Now devt_from_partuuid is
safer (kinda) for other init-time callers and allows for detecting
additions.  (E.g., if Kay wants to add more / arguments.)

>> +     /* Check for optional partition number offset attributes. */
>> +     if (uuid_str[36]) {
>> +             /* Explicitly fail on poor PARTUUID syntax. */
>> +             if (sscanf(&uuid_str[36], "/PARTNROFF=%d", &offset) != 1) {
>> +                     printk(KERN_ERR "VFS: PARTUUID= is invalid.\n"
>> +                      "Expected PARTUUID=<valid-uuid-id>[/PARTNROFF=%%d]\n");
>
> The check isn't complete - afacit input of the form PARTNROFF=42foo
> will be treated as PARTNROFF=42?

Completely true.  I can post another version that either pulls a
trailing %c (which should fail) or uses %n.  I have somewhat limited
internet connectivity right now, but I will follow up with a final
clean up when I can (>~week)

Thanks for the close review!
will

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] init: clean up devt_from_partuuid syntax checking and logging
  2011-07-06 18:01   ` Will Drewry
@ 2011-07-25 19:55     ` Will Drewry
  0 siblings, 0 replies; 6+ messages in thread
From: Will Drewry @ 2011-07-25 19:55 UTC (permalink / raw)
  To: akpm
  Cc: kay.sievers, linux-kernel, Will Drewry, Jens Axboe, Namhyung Kim,
	Trond Myklebust

This patch makes two changes:
- check for trailing characters after parsing PARTNROFF=%d
- disable root_wait if a syntax error is seen

The former assures that bad input like
  root=PARTUUID=<validuuid>/PARTNROFF=5abc
properly fails by attempting to parse an extra character after the
integer.  If the integer is missing, sscanf will fail, but if it is
present, and there is a trailing non-nul character, then the extra
field will be parsed and the error case will be hit.

The latter assures that if rootwait has been specified, the error
message isn't flooded to the screen during rootwait's loop.  Instead of
adding printk ratelimiting, root_wait was disabled.  This stays true to
the rootwait goal of support asynchronous device arrival while still
providing users with helpful messages.  With ratelimiting or disabling
logging on rootwait, a range of edge cases turn up where the user would
not be informed of an error properly.

This patch is meant to be applied on top of:
  http://userweb.kernel.org/~akpm/mmotm/broken-out/init-add-root=partuuid=uuid-partnroff=%25d-support.patch

Please let me know if you'd prefer a new patch series with this
integrated into the above patch (or as 3rd in the series).

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

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 13cddea..bcbeca7 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -110,10 +110,16 @@ static dev_t devt_from_partuuid(char *uuid_str)
 
 	/* Check for optional partition number offset attributes. */
 	if (uuid_str[36]) {
+		char c = 0;
 		/* Explicitly fail on poor PARTUUID syntax. */
-		if (sscanf(&uuid_str[36], "/PARTNROFF=%d", &offset) != 1) {
+		if (sscanf(&uuid_str[36],
+			   "/PARTNROFF=%d%c", &offset, &c) != 1) {
 			printk(KERN_ERR "VFS: PARTUUID= is invalid.\n"
 			 "Expected PARTUUID=<valid-uuid-id>[/PARTNROFF=%%d]\n");
+			if (root_wait)
+				printk(KERN_ERR
+				     "Disabling rootwait; root= is invalid.\n");
+			root_wait = 0;
 			goto done;
 		}
 	}
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support
  2011-06-30 21:10 [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support Will Drewry
  2011-06-30 21:10 ` [PATCH v3 2/2] Documentation: add pointer to name_to_dev_t for root= values Will Drewry
  2011-07-05 20:53 ` [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support Andrew Morton
@ 2011-07-25 20:47 ` H. Peter Anvin
  2 siblings, 0 replies; 6+ messages in thread
From: H. Peter Anvin @ 2011-07-25 20:47 UTC (permalink / raw)
  To: Will Drewry
  Cc: linux-kernel, kay.sievers, akpm, Jens Axboe, Namhyung Kim,
	Trond Myklebust

On 06/30/2011 02:10 PM, Will Drewry wrote:
> Expand root=PARTUUID=UUID syntax to support selecting a root partition
> by integer offset from a known, unique partition.  This approach
> provides similar properties to specifying a device and partition number,
> but using the UUID as the unique path prior to evaluating the offset.
> 
> For example,
>   root=PARTUUID=99DE9194-FC15-4223-9192-FC243948F88B/PARTNROFF=1
> selects the partition with UUID 99DE.. then select the next
> partition.
> 
> This change is motivated by a particular usecase in Chromium OS where
> the bootloader can easily determine what partition it is on (by UUID)
> but doesn't perform general partition table walking.
> 
> That said, support for this model provides a direct mechanism for the
> user to modify the root partition to boot without specifically needing
> to extract each UUID or update the bootloader explicitly when the root
> partition UUID is changed (if it is recreated to be larger, for
> instance).  Pinning to a /boot-style partition UUID allows the arbitrary
> root partition reconfiguration/modifications with slightly less
> ambiguity than just [dev][partition] and less stringency than the
> specific root partition UUID.
> 

Obklibcsnark:

Does anyone still doubt that as long as this kind of code exists in the
kernel that it will not get continually added to?

	-hpa


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-07-25 20:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-30 21:10 [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support Will Drewry
2011-06-30 21:10 ` [PATCH v3 2/2] Documentation: add pointer to name_to_dev_t for root= values Will Drewry
2011-07-05 20:53 ` [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support Andrew Morton
2011-07-06 18:01   ` Will Drewry
2011-07-25 19:55     ` [PATCH] init: clean up devt_from_partuuid syntax checking and logging Will Drewry
2011-07-25 20:47 ` [PATCH v3 1/2] init: add root=PARTUUID=UUID/PARTNROFF=%d support H. Peter Anvin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.