On Mon, 18 Aug 2014 15:04:43 +0530 Thiruvadi Rajaraman
<trajaraman@xxxxxxxxxx> 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....
> >From b3261f3d73e87ebcefd3951b0f8667c54f531360 Mon Sep 17 00:00:00 2001
> From: Thiruvadi Rajaraman <trajaraman@xxxxxxxxxx>
> 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.
>
> 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.
> 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.
> } 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
Attachment:
signature.asc
Description: PGP signature
