Skip to contentSkip to conversation

Endian issue assembling arrays

PATCH Doug Nazar 2 participants 2 messages Jul 28, 2010 – Sep 16, 2010 MD RAID

lore
Share & tools
lore

Doug Nazar

author
#0 lore raw

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

super1.c +2 -2
@@ -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;
 		}
#1 lore raw
Quoted 11 lines · On Wed, 28 Jul 2010 16:53:47 -0400 Doug Nazar <[email protected]> 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 :

super1.c +2 -2
@@ -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 <[email protected]>
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 <[email protected]>
Signed-off-by NeilBrown <[email protected]>
super1.c +3 -3
index 01473d1..0eb0323 100644
@@ -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;
 		}
 	}