On Fri, 26 Nov 2010 09:08:01 +0100 Adam Kwolek <adam.kwolek@xxxxxxxxx> wrote:
> Assumption for spares searching was that after picking new device, it has to be added to array before next search.
> This causes returning different disk on each call.
>
> When spares list is created during Online Capacity Expansion, first devices list is collected and then all devices are added to md.
> Picked device from spares pool has to be checked against picked devices so far. If not, the same disk will be returned all the time.
> Already picked devices are stored in the list and this list is used for new devices verification also.
>
> Signed-off-by: Adam Kwolek <adam.kwolek@xxxxxxxxx>
I have applied this without the change in imsm_grow_array, because I haven't
yet accepted the patch which adds that function.
So when we do add imsm_grow_array, imsm_add_spare will be read for it.
Also the two conversions from dprintf to fprintf didn't apply to my current
tree so I dropped them. I mention it only because this is a perfect example
of when I might be appropriate to include those extra changes in this patch,
but it would have been nice to see it mentioned in the description:
Two dprintf calls change to fprintf because .....
Thanks,
NeilBrown
> ---
>
> super-intel.c | 24 +++++++++++++++++-------
> 1 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index 3f75550..e4ba875 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -5006,7 +5006,8 @@ static struct dl *imsm_readd(struct intel_super *super, int idx, struct active_a
> }
>
> static struct dl *imsm_add_spare(struct intel_super *super, int slot,
> - struct active_array *a, int activate_new)
> + struct active_array *a, int activate_new,
> + struct mdinfo *additional_test_list)
> {
> struct imsm_dev *dev = get_imsm_dev(super, a->info.container_member);
> int idx = get_imsm_disk_idx(dev, slot);
> @@ -5032,6 +5033,16 @@ static struct dl *imsm_add_spare(struct intel_super *super, int slot,
> }
> if (d)
> continue;
> + while (additional_test_list) {
> + if (additional_test_list->disk.major == dl->major &&
> + additional_test_list->disk.minor == dl->minor) {
> + dprintf("%x:%x already in additional test list\n", dl->major, dl->minor);
> + break;
> + }
> + additional_test_list = additional_test_list->next;
> + }
> + if (additional_test_list)
> + continue;
>
> /* skip in use or failed drives */
> if (is_failed(&dl->disk) || idx == dl->index ||
> @@ -5165,9 +5176,9 @@ static struct mdinfo *imsm_activate_spare(struct active_array *a,
> */
> dl = imsm_readd(super, i, a);
> if (!dl)
> - dl = imsm_add_spare(super, i, a, 0);
> + dl = imsm_add_spare(super, i, a, 0, NULL);
> if (!dl)
> - dl = imsm_add_spare(super, i, a, 1);
> + dl = imsm_add_spare(super, i, a, 1, NULL);
> if (!dl)
> continue;
>
> @@ -6422,11 +6433,11 @@ struct mdinfo *get_spares_imsm(int devnum)
> sprintf(buf, "/dev/md/%s", info->name);
> ret_val = sysfs_get_unused_spares(cont_id, fd);
> if (ret_val == NULL) {
> - dprintf("imsm: ERROR: Cannot get spare devices.\n");
> + fprintf(stderr, Name": imsm: ERROR: Cannot get spare devices.\n");
> goto abort;
> }
> if (ret_val->array.spare_disks == 0) {
> - dprintf("imsm: ERROR: No available spares.\n");
> + fprintf(stderr, Name": imsm: ERROR: No available spares.\n");
> free(ret_val);
> ret_val = NULL;
> goto abort;
> @@ -7013,10 +7024,9 @@ struct mdinfo *imsm_grow_array(struct active_array *a)
> for (i = prev_raid_disks; i < new_raid_disks; i++) {
> /* OK, this device can be added. Try to add.
> */
> - dl = imsm_add_spare(super, i, a, 0);
> + dl = imsm_add_spare(super, i, a, 0, rv);
> if (!dl)
> continue;
> -
> if (dl->index < 0)
> dl->index = i;
> /* found a usable disk with enough space */
--
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
