Endian issue assembling arrays

This is a multi-part message in MIME format.
--------------010804060105060906020307
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Came across this while testing some updates to GRUB's RAID support.

Assembling a RAID1 array on a PPC box results in the following warning:

mdadm: device 1 in /dev/md/0 has wrong state in superblock, but
/dev/sdb2 seems ok
mdadm: /dev/md/0 has been started with 2


Doug


--------------010804060105060906020307
Content-Type: text/plain;
name="mdadm-assemble-roles-endian-fix.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="mdadm-assemble-roles-endian-fix.diff"

--- mdadm-3.1.2/super1.c 2010-03-09 18:26:44.000000000 -0500
+++ mdadm-3.1.2.dev/super1.c 2010-07-28 16:41:41.000000000 -0400
@@ -673,10 +673,10 @@
int d = info->disk.number;
int want;
if (info->disk.state == 6)
- want = __cpu_to_le32(info->disk.raid_disk);
+ want = info->disk.raid_disk;
else
want = 0xFFFF;
- if (sb->dev_roles[d] != want) {
+ if (__le16_to_cpu(sb->dev_roles[d]) != want) {
sb->dev_roles[d] = want;
rv = 1;
}

--------------010804060105060906020307--
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: Endian issue assembling arrays

On Wed, 28 Jul 2010 16:53:47 -0400
Doug Nazar wrote:

> Came across this while testing some updates to GRUB's RAID support.
>
> Assembling a RAID1 array on a PPC box results in the following warning:
>
> mdadm: device 1 in /dev/md/0 has wrong state in superblock, but
> /dev/sdb2 seems ok
> mdadm: /dev/md/0 has been started with 2
>
>
> Doug
>

Thanks for the report and the more recent reminder.

Your patch :

--- mdadm-3.1.2/super1.c 2010-03-09 18:26:44.000000000 -0500
+++ mdadm-3.1.2.dev/super1.c 2010-07-28 16:41:41.000000000 -0400
@@ -673,10 +673,10 @@
int d = info->disk.number;
int want;
if (info->disk.state == 6)
- want = __cpu_to_le32(info->disk.raid_disk);
+ want = info->disk.raid_disk;
else
want = 0xFFFF;
- if (sb->dev_roles[d] != want) {
+ if (__le16_to_cpu(sb->dev_roles[d]) != want) {
sb->dev_roles[d] = want;
rv = 1;
}


isn't quite right. The assignment needs a conversion too.

I have committed the following to
http://neil.brown.name/git/mdadm/

Thanks a lot,
NeilBrown

commit a2ce5a1af19e5dcfd59cad117c0e9fccabce7322
Author: NeilBrown
Date: Thu Sep 16 20:58:31 2010 +1000

Fix byte-order conversion in update_super1("assemble")

This code is wrong is several ways, and failed on big-endian machines.
Put in correct endian coversions: 'want' is cpu-order, dev_roles[] is little-endian,
16 bit.

Reported-by: Doug Nazar
Signed-off-by: NeilBrown

diff --git a/super1.c b/super1.c
index 01473d1..0eb0323 100644
--- a/super1.c
+++ b/super1.c
@@ -673,11 +673,11 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
int d = info->disk.number;
int want;
if (info->disk.state == 6)
- want = __cpu_to_le32(info->disk.raid_disk);
+ want = info->disk.raid_disk;
else
want = 0xFFFF;
- if (sb->dev_roles[d] != want) {
- sb->dev_roles[d] = want;
+ if (sb->dev_roles[d] != __cpu_to_le16(want)) {
+ sb->dev_roles[d] = __cpu_to_le16(want);
rv = 1;
}
}
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html