On Thu, 27 Jan 2011 10:28:28 +0100
Adam Kwolek <adam.kwolek@xxxxxxxxx> 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 <adam.kwolek@xxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
