[PATCH 0/6] Reshape fixes (expansion)

The following series adds some fixes for reshape (expansion).

A while ago mdadm unit tests (suits 12 and 13) works correctly, now I've found problems.
I've learned that containers created for raid5 and raid0 has different number of blocks in mdstat.
I've put mdstat partial output to patch description: UT FIX: imsm container can have different blocks number
I'm not 100% sure that this is fix (for changed behavior), not workaround for container creation problem.
This fix is first of fixes (3) that enables reshape (12, 13) UT.
If you find this more as workaround, this patch is good point for discussion what change is correct.

Patches:
FIX: compare blocks on all data disks
FIX: Compare the same units
makes changes for blocks computing verification. After those 2 more patches UT (12, 13) works.
Please let me know what you are thinking about them. It is possible that this is not final patch also,
but I've tried to find out what this condition should do, and change code to archive it (IMHO).

Rest of patches addresses other issues, I can observe during expansion.

BR
Adam

---

Adam Kwolek (6):
FIX: md runs recovery instead reshape for growing single disk raid0 array
FIX: Container can be left frozen
FIX: compare blocks on all data disks
FIX: Compare the same units
UT FIX: imsm container can have different blocks number
imsm: FIX: Wrong output string format


Grow.c | 9 +++++++--
super-intel.c | 4 ++--
tests/env-imsm-template | 2 +-
3 files changed, 10 insertions(+), 5 deletions(-)

--
Signature
--
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

[PATCH 2/6] UT FIX: imsm container can have different blocks number

When imsm container is created it have different blocks number
in /proc/mdstat depending on containing array raid level (raid0/raid5).
raid5 case:
md127 : inactive sdd[3](S) sde[2](S) sdc[1](S) sdb[0](S)
2884 blocks super external:imsm

raid0 case:
md127 : inactive sdd[3](S) sde[2](S) sdc[1](S) sdb[0](S)
836 blocks super external:imsm

Due to this, it cannot be compared to one value for both cases.

Test imsm container existence before unit test run only.

Signed-off-by: Adam Kwolek
---

tests/env-imsm-template | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/env-imsm-template b/tests/env-imsm-template
index 74c43c6..7a2890c 100644
--- a/tests/env-imsm-template
+++ b/tests/env-imsm-template
@@ -2,7 +2,7 @@ imsm_check() {
udevadm settle
case $1 in
container )
- grep -s "$(((418 * $2) / 2)) blocks super external:imsm" /proc/mdstat > /dev/null || {
+ grep -s "blocks super external:imsm" /proc/mdstat > /dev/null || {
echo >&2 "**Fatal** Correctly formed container not found"; cat /proc/mdstat; exit 1; }
;;
member )

--
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

[PATCH 3/6] FIX: Compare the same units

In condition blocks and kbytes are compared.
The same units have to be compared, otherwise comparison
is meaningless.

Signed-off-by: Adam Kwolek
---

Grow.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index 8229b4d..6a9e81e 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1829,7 +1829,7 @@ started:
fprintf(stderr, Name ": Need to backup %luK of critical "
"section..\n", blocks/2);

- if (blocks >= sra->component_size/2) {
+ if (blocks >= sra->component_size) {
fprintf(stderr, Name ": %s: Something wrong"
" - reshape aborted\n",
devname);

--
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

[PATCH 4/6] FIX: compare blocks on all data disks

blocks are counted for all data disks.
component_size value is given for one array disk only.
This means we should multiple component_size by data disks number
to get similar (compare able) values.

Signed-off-by: Adam Kwolek
---

Grow.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index 6a9e81e..09d7439 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1829,7 +1829,8 @@ started:
fprintf(stderr, Name ": Need to backup %luK of critical "
"section..\n", blocks/2);

- if (blocks >= sra->component_size) {
+ disks = min(reshape.before.data_disks, reshape.after.data_disks);
+ if (blocks >= sra->component_size * disks) {
fprintf(stderr, Name ": %s: Something wrong"
" - reshape aborted\n",
devname);

--
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

[PATCH 5/6] FIX: Container can be left frozen

When container operation fails before child process starts,
array can be left frozen because container_reshape() doesn't make
unfreeze() operation in all error cases, as it is responsible for.

add unfreeze() operation for error case scenarios in reshape_container()

Signed-off-by: Adam Kwolek
---

Grow.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index 09d7439..e77ab80 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2123,8 +2123,10 @@ int reshape_container(char *container, int cfd, char *devname,
if (reshape_super(st, -1, info->new_level,
info->new_layout, info->new_chunk,
info->array.raid_disks + info->delta_disks,
- backup_file, devname, quiet))
+ backup_file, devname, quiet)) {
+ unfreeze(st);
return 1;
+ }

sync_metadata(st);

@@ -2135,6 +2137,7 @@ int reshape_container(char *container, int cfd, char *devname,
switch (fork()) {
case -1: /* error */
perror("Cannot fork to complete reshape\n");
+ unfreeze(st);
return 1;
default: /* parent */
printf(Name ": multi-array reshape continues in background\n");

--
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

[PATCH 6/6] FIX: md runs recovery instead reshape for growing single

Problem occurs when we want to expand single disk raid0 array.
This is done via degraded 2 disks raid4 array. When new spare disk
for reshape is added to array, md immediately initiates recovery before
mdadm can configure and start reshape. This is due fact that 2 disk
raid4/5 array is special md case.
Mdmon does nothing here because container is blocked.
This is caused because after takeover array is not in frozen state in md.

Put array in to frozen state after takeover to allow mdadm to finish
configuration before reshape is executed in md.

Signed-off-by: Adam Kwolek
---

Grow.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Grow.c b/Grow.c
index e77ab80..88e03ef 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1677,6 +1677,7 @@ static int reshape_array(char *container, int fd, char *devname,
fprintf(stderr, Name ": level of %s changed to %s\n",
devname, c);
orig_level = info->array.level;
+ sysfs_freeze_array(info);

if (reshape.level > 0 && st->ss->external) {
/* make sure mdmon is aware of the new level */

--
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: [PATCH 0/6] Reshape fixes (expansion)

On Tue, 08 Feb 2011 14:30:28 +0100 Adam Kwolek wrote:

> The following series adds some fixes for reshape (expansion).
>
> A while ago mdadm unit tests (suits 12 and 13) works correctly, now I've found problems.
> I've learned that containers created for raid5 and raid0 has different number of blocks in mdstat.
> I've put mdstat partial output to patch description: UT FIX: imsm container can have different blocks number
> I'm not 100% sure that this is fix (for changed behavior), not workaround for container creation problem.
> This fix is first of fixes (3) that enables reshape (12, 13) UT.
> If you find this more as workaround, this patch is good point for discussion what change is correct.
>
> Patches:
> FIX: compare blocks on all data disks
> FIX: Compare the same units
> makes changes for blocks computing verification. After those 2 more patches UT (12, 13) works.
> Please let me know what you are thinking about them. It is possible that this is not final patch also,
> but I've tried to find out what this condition should do, and change code to archive it (IMHO).
>
> Rest of patches addresses other issues, I can observe during expansion.
>
> BR
> Adam
>
> ---
>
> Adam Kwolek (6):
> FIX: md runs recovery instead reshape for growing single disk raid0 array
Applied.

> FIX: Container can be left frozen
Applied.

> FIX: compare blocks on all data disks
> FIX: Compare the same units
No, I don't like these.
The point here is to compare the amount of data to be backed up with the
size of one of the spare devices, because in some cases the data will be
backed up to a spare device.
If the size of the data to be backed up is bigger than half the size of a
spare device, then it doesn't really fit and something is wrong.
Normally the amount of data to be backed up is much much smaller than the
size of any device.

Maybe we just need to make the test use larger devices or smaller chunk size,
or similar.


> UT FIX: imsm container can have different blocks number
Applied (the block numbers here are completely meaningless, so ignoring them
is good).
> imsm: FIX: Wrong output string format
Applied.

All pushed out to devel-3.2

Thanks.

NeilBrown

--
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: [PATCH 0/6] Reshape fixes (expansion)

> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Wednesday, February 09, 2011 4:11 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 0/6] Reshape fixes (expansion)
>
> On Tue, 08 Feb 2011 14:30:28 +0100 Adam Kwolek
> wrote:
>
> > The following series adds some fixes for reshape (expansion).
> >
> > A while ago mdadm unit tests (suits 12 and 13) works correctly, now
> I've found problems.
> > I've learned that containers created for raid5 and raid0 has
> different number of blocks in mdstat.
> > I've put mdstat partial output to patch description: UT FIX: imsm
> container can have different blocks number
> > I'm not 100% sure that this is fix (for changed behavior), not
> workaround for container creation problem.
> > This fix is first of fixes (3) that enables reshape (12, 13) UT.
> > If you find this more as workaround, this patch is good point for
> discussion what change is correct.
> >
> > Patches:
> > FIX: compare blocks on all data disks
> > FIX: Compare the same units
> > makes changes for blocks computing verification. After those 2 more
> patches UT (12, 13) works.
> > Please let me know what you are thinking about them. It is possible
> that this is not final patch also,
> > but I've tried to find out what this condition should do, and change
> code to archive it (IMHO).
> >
> > Rest of patches addresses other issues, I can observe during
> expansion.
> >
> > BR
> > Adam
> >
> > ---
> >
> > Adam Kwolek (6):
> > FIX: md runs recovery instead reshape for growing single disk
> raid0 array
> Applied.
>
> > FIX: Container can be left frozen
> Applied.
>
> > FIX: compare blocks on all data disks
> > FIX: Compare the same units
> No, I don't like these.
> The point here is to compare the amount of data to be backed up with
> the
> size of one of the spare devices, because in some cases the data will
> be
> backed up to a spare device.
> If the size of the data to be backed up is bigger than half the size of
> a
> spare device, then it doesn't really fit and something is wrong.
> Normally the amount of data to be backed up is much much smaller than
> the
> size of any device.
>
> Maybe we just need to make the test use larger devices or smaller chunk
> size,
> or similar.

Thank you for explanation. I'll prepare changes to UT.

BR
Adam

>
>
> > UT FIX: imsm container can have different blocks number
> Applied (the block numbers here are completely meaningless, so ignoring
> them
> is good).
> > imsm: FIX: Wrong output string format
> Applied.
>
> All pushed out to devel-3.2
>
> Thanks.
>
> NeilBrown

--
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