[PATCH] FIX: delta_disk can have UnSet value

[PATCH] FIX: delta_disk can have UnSet value

am 27.01.2011 10:28:28 von adam.kwolek

Delta_disk can be set to UnSet value.
This can a cause to pass wrong parameter to reshape_super().

Signed-off-by: Adam Kwolek
---

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

diff --git a/Grow.c b/Grow.c
index daf310b..bc015c6 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1575,7 +1575,9 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,

if (reshape_super(st, info.component_size, info.new_level,
info.new_layout, info.new_chunk,
- info.array.raid_disks + info.delta_disks,
+ info.delta_disks == UnSet ?
+ info.array.raid_disks :
+ info.array.raid_disks + info.delta_disks,
backup_file, devname, quiet)) {
rv = 1;
goto release;

--
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] FIX: delta_disk can have UnSet value

am 28.01.2011 02:18:20 von NeilBrown

On Thu, 27 Jan 2011 10:28:28 +0100
Adam Kwolek wrote:

> Delta_disk can be set to UnSet value.
> This can a cause to pass wrong parameter to reshape_super().
>
> Signed-off-by: Adam Kwolek
> ---
>
> Grow.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index daf310b..bc015c6 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1575,7 +1575,9 @@ int Grow_reshape(char *devname, int fd, int
> quiet, char *backup_file,
> if (reshape_super(st, info.component_size,
> info.new_level, info.new_layout, info.new_chunk,
> - info.array.raid_disks +
> info.delta_disks,
> + info.delta_disks == UnSet ?
> + info.array.raid_disks :
> + info.array.raid_disks +
> info.delta_disks, backup_file, devname, quiet)) {
> rv = 1;
> goto release;

This is clearly a problem, but I think it is the wrong fix.
If delta_disks is unset, then the new raid_disks number could vary
depending on other values.
E.g. if you has a 4-disk raid5 and ask to convert it to RAID6 without
specifying the number of devices, then it should assume a 5-disk RAID6.
Your change would make it look like a 4-disk RAID6 was requested.

So I think we need to change reshape_super to take a 'delta_disks'
parameter rather than a 'raid_disks' parameter.
Either that, or figure out the preferred default in advance.
analyse_change does that I think, but it is called later, so we cannot
really use what it produces.

I'm not sure what the best answer is just now - hopefully I'll be able
to look properly on Monday.

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