Re: [PATCH] md: fix for wrong state in superblock issue in assemble process
From: NeilBrown <hidden>
Date: 2014-08-20 03:28:21
On Mon, 18 Aug 2014 15:04:43 +0530 Thiruvadi Rajaraman [off-list ref] wrote: Hi, you sent this email as HTML. That means you would have got an automatic message from the kernel.org mail server telling you that it wasn't accepted. Yet you didn't bother to try sending it again as plain text....
quoted
From b3261f3d73e87ebcefd3951b0f8667c54f531360 Mon Sep 17 00:00:00 2001From: Thiruvadi Rajaraman <redacted> Date: Wed, 23 Jul 2014 06:46:22 +0530 Subject: [PATCH] md: fix for wrong state in superblock issue This patch has created on top of linux stable kernel "v2.6.32".
I'm only really interested in patches based on current mainline kernel (3.16) or something reasonably recent.
The multipath md array assemble process by, # mdadm -A /dev/mdx /dev/sdx /dev/sdy ...
I'm not very keen of fixing bugs in md/multipath. I would really like to remove it completely, but people might be using it. If there a known bugs, then people might stop using it, and that would be good. dm/multipath is much better and is actually supported. Is there a reason you don't use that? See the multipath-tools package.
quoted hunk
which showing "wrong state in superblock" message because of dev_roles[d] of disk in md array is not updated with individual disk number while adding the removed disk to md array in assemble process. Issue solved by updating the dev_roles[raid disk] superblock detail with specific raid disk number during the array add process. --- drivers/md/md.c | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)diff --git a/drivers/md/md.c b/drivers/md/md.c index 4ce6e2f..7ba2a0d 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c@@ -1460,6 +1460,9 @@ static void super_1_sync(mddev_t *mddev, mdk_rdev_t*rdev) sb->dev_roles[i] = cpu_to_le16(0xffff); } + if (sb->level == cpu_to_le32(LEVEL_MULTIPATH)) + sb->dev_number = 0; +
So you are forcing the dev_number to always be zero on multipath. That makes sense. I'm surprised it isn't already zero... What code sets it to a non-zero number? It seems that the kernel doesn't set dev_number at all at present. I think it is best to leave it that way.
quoted hunk
sb->sb_csum = calc_sb_1_csum(sb); }@@ -1643,10 +1646,15 @@ static int bind_rdev_to_array(mdk_rdev_t * rdev,mddev_t * mddev) */ if (rdev->desc_nr < 0) { int choice = 0; - if (mddev->pers) choice = mddev->raid_disks; + if (mddev->pers && mddev->level != LEVEL_MULTIPATH) + choice = mddev->raid_disks; while (find_rdev_nr(mddev, choice)) choice++; rdev->desc_nr = choice; + if (mddev->level == LEVEL_MULTIPATH) { + rdev->raid_disk = rdev->desc_nr; + set_bit(In_sync, &rdev->flags); + }
This looks wrong. It isn't the job of bind_rdev_to_array to set ->raid_disk. That is by by ->hot_add_disk.
quoted hunk
} else { if (find_rdev_nr(mddev, rdev->desc_nr)) return -EBUSY;@@ -4845,6 +4853,9 @@ static int add_new_disk(mddev_t * mddev,mdu_disk_info_t *info) if (err) unbind_rdev_from_array(rdev); } + if (!err && mddev->level == LEVEL_MULTIPATH) { + err = mddev->pers->hot_add_disk(mddev, rdev); + } if (err) export_rdev(rdev); else
This possibly makes sense, but I would rather modify the earlier condition which called ->hot_add_disk of ->hot_remove_disk is missing. But the real question is: why do you want to use md/multipath? Can I convince you not to? Thanks, NeilBrown
Attachments
- signature.asc [application/pgp-signature] 828 bytes