[PATCH] md: register new md sysfs file 'uuid' read-only

[PATCH] md: register new md sysfs file 'uuid' read-only

[Sebastian Riemer]

Report the UUID of the MD array in the following format:
xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

This is useful if you don't want to wait for udev to
identify your MD array.

Signed-off-by: Sebastian Riemer <sebastian.riemer@profitbricks.com>
---
 drivers/md/md.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 308e87b..4e654d1 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3730,6 +3730,14 @@ static struct md_sysfs_entry md_raid_disks =
 __ATTR(raid_disks, S_IRUGO|S_IWUSR, raid_disks_show, raid_disks_store);
 
 static ssize_t
+uuid_show(struct mddev *mddev, char *page)
+{
+	return sprintf(page, "%pU\n", mddev->uuid);
+}
+static struct md_sysfs_entry md_uuid =
+__ATTR(uuid, S_IRUGO, uuid_show, NULL);
+
+static ssize_t
 chunk_size_show(struct mddev *mddev, char *page)
 {
 	if (mddev->reshape_position != MaxSector &&
@@ -4667,6 +4675,7 @@ static struct attribute *md_default_attrs[] = {
 	&md_level.attr,
 	&md_layout.attr,
 	&md_raid_disks.attr,
+	&md_uuid.attr,
 	&md_chunk_size.attr,
 	&md_size.attr,
 	&md_resync_start.attr,
-- 
1.7.1

Re: [PATCH] md: register new md sysfs file 'uuid' read-only

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]

On Tue,  2 Oct 2012 15:42:10 +0200 Sebastian Riemer
<sebastian.riemer@profitbricks.com> wrote:

> Report the UUID of the MD array in the following format:
> xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> 
> This is useful if you don't want to wait for udev to
> identify your MD array.

If you don't want to wait for udev, run "mdadm -D --export /dev/mdwhatever"
and extract the uuid from that.

And the UUID format you mention is different from the format used by mdadm,
which makes me like the patch even less.


What problem are you trying to solve here?

NeilBrown

> 
> Signed-off-by: Sebastian Riemer <sebastian.riemer@profitbricks.com>
> ---
>  drivers/md/md.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 308e87b..4e654d1 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3730,6 +3730,14 @@ static struct md_sysfs_entry md_raid_disks =
>  __ATTR(raid_disks, S_IRUGO|S_IWUSR, raid_disks_show, raid_disks_store);
>  
>  static ssize_t
> +uuid_show(struct mddev *mddev, char *page)
> +{
> +	return sprintf(page, "%pU\n", mddev->uuid);
> +}
> +static struct md_sysfs_entry md_uuid =
> +__ATTR(uuid, S_IRUGO, uuid_show, NULL);
> +
> +static ssize_t
>  chunk_size_show(struct mddev *mddev, char *page)
>  {
>  	if (mddev->reshape_position != MaxSector &&
> @@ -4667,6 +4675,7 @@ static struct attribute *md_default_attrs[] = {
>  	&md_level.attr,
>  	&md_layout.attr,
>  	&md_raid_disks.attr,
> +	&md_uuid.attr,
>  	&md_chunk_size.attr,
>  	&md_size.attr,
>  	&md_resync_start.attr,


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] md: register new md sysfs file 'uuid' read-only

[Sebastian Riemer]

Hi Neil,

I must have overseen your answer. We want to use as less mdadm as
possible - especially for simple checking stuff. The kernel knows best.
"mdadm -D" reads the data from disk. This means additional disk access.
It is also possible that this command triggers superblock updates.
Simple checking stuff shouldn't have to use disk access.

We are planning to use MD RAID a little bit different with e.g. 100 MD
arrays. Searching for a particular UUID in this bunch of MD devices can
be difficult without this patch. This is how the kernel represents
UUIDs. Conversion can be done in user space.

If you like it better in the xxxxxxxx:xxxxxxxx:xxxxxxxx:xxxxxxxx format,
I can resend the patch in that format.

It simplifies our checking magic not to rely on udev symlinks and to ask
the kernel for the cached UUID.

Btw.: We did an "Assemble-Resize-Stop, Assemble-Resize-Stop, ..."-Test
with "--assume-clean" and the second resize doesn't work. With
"Assemble-Resize-Detail-Stop" it works.

Cheers,
Sebastian


On 03.10.2012 00:30, NeilBrown wrote:
> On Tue, 2 Oct 2012 15:42:10 +0200 Sebastian Riemer
> <sebastian.riemer@profitbricks.com> wrote:
>
>> Report the UUID of the MD array in the following format:
>> xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
>>
>> This is useful if you don't want to wait for udev to
>> identify your MD array.
>
> If you don't want to wait for udev, run "mdadm -D --export
/dev/mdwhatever"
> and extract the uuid from that.
>
> And the UUID format you mention is different from the format used by
mdadm,
> which makes me like the patch even less.
>
>
> What problem are you trying to solve here?
>
> NeilBrown

Re: [PATCH] md: register new md sysfs file 'uuid' read-only

[Phil Turmel]

On 10/08/2012 12:32 PM, Sebastian Riemer wrote:> Hi Neil,
>
> I must have overseen your answer. We want to use as less mdadm as
> possible - especially for simple checking stuff. The kernel knows best.
> "mdadm -D" reads the data from disk. This means additional disk access.
> It is also possible that this command triggers superblock updates.
> Simple checking stuff shouldn't have to use disk access.
>
> We are planning to use MD RAID a little bit different with e.g. 100 MD
> arrays. Searching for a particular UUID in this bunch of MD devices can
> be difficult without this patch. This is how the kernel represents
> UUIDs. Conversion can be done in user space.
>
> If you like it better in the xxxxxxxx:xxxxxxxx:xxxxxxxx:xxxxxxxx format,
> I can resend the patch in that format.

If you are aiming for consistency, the dm driver(s) have already
established a no-punctuation format for the UUID itself (punctuation
separates UUID type from the UUID) within sysfs.

Regards,

Phil

Re: [PATCH] md: register new md sysfs file 'uuid' read-only

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2018 bytes --]

On Mon, 08 Oct 2012 18:32:16 +0200 Sebastian Riemer
<sebastian.riemer@profitbricks.com> wrote:

> Hi Neil,
> 
> I must have overseen your answer. We want to use as less mdadm as
> possible - especially for simple checking stuff. The kernel knows best.

Kernel sometimes knows.  Some arrays have metadata handled by user-space so
the kernel wouldn't be able to report anything.
Userspace always knows..

> "mdadm -D" reads the data from disk. This means additional disk access.
> It is also possible that this command triggers superblock updates.
> Simple checking stuff shouldn't have to use disk access.

In that case, how about using /run/mdadm/map ??
When mdadm assembles an array it needs to know the uuid, and it records this
in that file for future reference.  Will that do?


> 
> We are planning to use MD RAID a little bit different with e.g. 100 MD
> arrays. Searching for a particular UUID in this bunch of MD devices can
> be difficult without this patch. This is how the kernel represents
> UUIDs. Conversion can be done in user space.

I believe that format is for RFC4122 UUIDs.  MD's UUIDs are not created
according to those rules, so it isn't really appropriate to make it look like
they were.
However the formatting isn't the big issue - the UUID is something the kernel
knows by accident more than by necessity.  I'd rather it didn't.

> 
> If you like it better in the xxxxxxxx:xxxxxxxx:xxxxxxxx:xxxxxxxx format,
> I can resend the patch in that format.
> 
> It simplifies our checking magic not to rely on udev symlinks and to ask
> the kernel for the cached UUID.
> 
> Btw.: We did an "Assemble-Resize-Stop, Assemble-Resize-Stop, ..."-Test
> with "--assume-clean" and the second resize doesn't work. With
> "Assemble-Resize-Detail-Stop" it works.

More details?  What exactly do you mean by "Resize"?  What exactly do you
mean by "doesn't work"?  Can you make a simple script that reproduces this?
If so, I'd love to see it.

Thanks,
NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] md: register new md sysfs file 'uuid' read-only

[Sebastian Riemer]

On 09.10.2012 05:02, NeilBrown wrote:
> Kernel sometimes knows. Some arrays have metadata handled by user-space so
> the kernel wouldn't be able to report anything.
> Userspace always knows..

Thanks for the hint. I didn't think about that.

> In that case, how about using /run/mdadm/map ??
> When mdadm assembles an array it needs to know the uuid, and it
records this
> in that file for future reference. Will that do?

Thanks for the hint. Our 3.2.5-Version didn't build it for an existing
location. I made it do so but I don't like the UUID format in there.

I assembled UUID 12345678:12345678:12345678:12345678 and the file
contains the following:

md0 1.2 78563412:78563412:78563412:78563412 /dev/md0

This would be a nice file if I could grep for the UUID in the same
format as I've entered it upon creation/assembly.
So, I guess we can live with a custom patch for now and close the
discussion here.

>> Btw.: We did an "Assemble-Resize-Stop, Assemble-Resize-Stop, ..."-Test
>> with "--assume-clean" and the second resize doesn't work. With
>> "Assemble-Resize-Detail-Stop" it works.
>
> More details? What exactly do you mean by "Resize"? What exactly do you
> mean by "doesn't work"? Can you make a simple script that reproduces this?
> If so, I'd love to see it.

I'll open a new topic for this after analysing it in more detail today.
I've only seen the issue on the PC of my boss and some automation guys
reported it.

Cheers,
Sebastian