> 2022年5月31日 18:27，Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx> 写道：
>
> Autolayout relies on drives order on super->disks list, but
> it is not quaranted by readdir() in sysfs_read(). As a result
> drive could be put in different slot in second volume.
>
> Make it consistent by reffering to first volume, if exists.
>
> Use enum imsm_status to unify error handling.
>
> Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx>
Acked-by: Coly Li <colyli@xxxxxxx>
Coly Li
> ---
> super-intel.c | 169 ++++++++++++++++++++++++++++++++------------------
> 1 file changed, 108 insertions(+), 61 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index f0196948..3c02d2f6 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -7493,11 +7493,27 @@ static int validate_geometry_imsm_volume(struct supertype *st, int level,
> return 1;
> }
>
> -static int imsm_get_free_size(struct supertype *st, int raiddisks,
> - unsigned long long size, int chunk,
> - unsigned long long *freesize)
> +/**
> + * imsm_get_free_size() - get the biggest, common free space from members.
> + * @super: &intel_super pointer, not NULL.
> + * @raiddisks: number of raid disks.
> + * @size: requested size, could be 0 (means max size).
> + * @chunk: requested chunk.
> + * @freesize: pointer for returned size value.
> + *
> + * Return: &IMSM_STATUS_OK or &IMSM_STATUS_ERROR.
> + *
> + * @freesize is set to meaningful value, this can be @size, or calculated
> + * max free size.
> + * super->create_offset value is modified and set appropriately in
> + * merge_extends() for further creation.
> + */
> +static imsm_status_t imsm_get_free_size(struct intel_super *super,
> + const int raiddisks,
> + unsigned long long size,
> + const int chunk,
> + unsigned long long *freesize)
> {
> - struct intel_super *super = st->sb;
> struct imsm_super *mpb = super->anchor;
> struct dl *dl;
> int i;
> @@ -7541,12 +7557,10 @@ static int imsm_get_free_size(struct supertype *st, int raiddisks,
> /* chunk is in K */
> minsize = chunk * 2;
>
> - if (cnt < raiddisks ||
> - (super->orom && used && used != raiddisks) ||
> - maxsize < minsize ||
> - maxsize == 0) {
> + if (cnt < raiddisks || (super->orom && used && used != raiddisks) ||
> + maxsize < minsize || maxsize == 0) {
> pr_err("not enough devices with space to create array.\n");
> - return 0; /* No enough free spaces large enough */
> + return IMSM_STATUS_ERROR;
> }
>
> if (size == 0) {
> @@ -7559,37 +7573,69 @@ static int imsm_get_free_size(struct supertype *st, int raiddisks,
> }
> if (mpb->num_raid_devs > 0 && size && size != maxsize)
> pr_err("attempting to create a second volume with size less then remaining space.\n");
> - cnt = 0;
> - for (dl = super->disks; dl; dl = dl->next)
> - if (dl->e)
> - dl->raiddisk = cnt++;
> -
> *freesize = size;
>
> dprintf("imsm: imsm_get_free_size() returns : %llu\n", size);
>
> - return 1;
> + return IMSM_STATUS_OK;
> }
>
> -static int reserve_space(struct supertype *st, int raiddisks,
> - unsigned long long size, int chunk,
> - unsigned long long *freesize)
> +/**
> + * autolayout_imsm() - automatically layout a new volume.
> + * @super: &intel_super pointer, not NULL.
> + * @raiddisks: number of raid disks.
> + * @size: requested size, could be 0 (means max size).
> + * @chunk: requested chunk.
> + * @freesize: pointer for returned size value.
> + *
> + * We are being asked to automatically layout a new volume based on the current
> + * contents of the container. If the parameters can be satisfied autolayout_imsm
> + * will record the disks, start offset, and will return size of the volume to
> + * be created. See imsm_get_free_size() for details.
> + * add_to_super() and getinfo_super() detect when autolayout is in progress.
> + * If first volume exists, slots are set consistently to it.
> + *
> + * Return: &IMSM_STATUS_OK on success, &IMSM_STATUS_ERROR otherwise.
> + *
> + * Disks are marked for creation via dl->raiddisk.
> + */
> +static imsm_status_t autolayout_imsm(struct intel_super *super,
> + const int raiddisks,
> + unsigned long long size, const int chunk,
> + unsigned long long *freesize)
> {
> - struct intel_super *super = st->sb;
> - struct dl *dl;
> - int cnt;
> - int rv = 0;
> + int curr_slot = 0;
> + struct dl *disk;
> + int vol_cnt = super->anchor->num_raid_devs;
> + imsm_status_t rv;
>
> - rv = imsm_get_free_size(st, raiddisks, size, chunk, freesize);
> - if (rv) {
> - cnt = 0;
> - for (dl = super->disks; dl; dl = dl->next)
> - if (dl->e)
> - dl->raiddisk = cnt++;
> - rv = 1;
> + rv = imsm_get_free_size(super, raiddisks, size, chunk, freesize);
> + if (rv != IMSM_STATUS_OK)
> + return IMSM_STATUS_ERROR;
> +
> + for (disk = super->disks; disk; disk = disk->next) {
> + if (!disk->e)
> + continue;
> +
> + if (curr_slot == raiddisks)
> + break;
> +
> + if (vol_cnt == 0) {
> + disk->raiddisk = curr_slot;
> + } else {
> + int _slot = get_disk_slot_in_dev(super, 0, disk->index);
> +
> + if (_slot == -1) {
> + pr_err("Disk %s is not used in first volume, aborting\n",
> + disk->devname);
> + return IMSM_STATUS_ERROR;
> + }
> + disk->raiddisk = _slot;
> + }
> + curr_slot++;
> }
>
> - return rv;
> + return IMSM_STATUS_OK;
> }
>
> static int validate_geometry_imsm(struct supertype *st, int level, int layout,
> @@ -7625,35 +7671,35 @@ static int validate_geometry_imsm(struct supertype *st, int level, int layout,
> }
>
> if (!dev) {
> - if (st->sb) {
> - struct intel_super *super = st->sb;
> - if (!validate_geometry_imsm_orom(st->sb, level, layout,
> - raiddisks, chunk, size,
> - verbose))
> + struct intel_super *super = st->sb;
> +
> + /*
> + * Autolayout mode, st->sb and freesize must be set.
> + */
> + if (!super || !freesize) {
> + pr_vrb("freesize and superblock must be set for autolayout, aborting\n");
> + return 1;
> + }
> +
> + if (!validate_geometry_imsm_orom(st->sb, level, layout,
> + raiddisks, chunk, size,
> + verbose))
> + return 0;
> +
> + if (super->orom) {
> + imsm_status_t rv;
> + int count = count_volumes(super->hba, super->orom->dpa,
> + verbose);
> + if (super->orom->vphba <= count) {
> + pr_vrb("platform does not support more than %d raid volumes.\n",
> + super->orom->vphba);
> return 0;
> - /* we are being asked to automatically layout a
> - * new volume based on the current contents of
> - * the container. If the the parameters can be
> - * satisfied reserve_space will record the disks,
> - * start offset, and size of the volume to be
> - * created. add_to_super and getinfo_super
> - * detect when autolayout is in progress.
> - */
> - /* assuming that freesize is always given when array is
> - created */
> - if (super->orom && freesize) {
> - int count;
> - count = count_volumes(super->hba,
> - super->orom->dpa, verbose);
> - if (super->orom->vphba <= count) {
> - pr_vrb("platform does not support more than %d raid volumes.\n",
> - super->orom->vphba);
> - return 0;
> - }
> }
> - if (freesize)
> - return reserve_space(st, raiddisks, size,
> - *chunk, freesize);
> +
> + rv = autolayout_imsm(super, raiddisks, size, *chunk,
> + freesize);
> + if (rv != IMSM_STATUS_OK)
> + return 0;
> }
> return 1;
> }
> @@ -11524,7 +11570,7 @@ enum imsm_reshape_type imsm_analyze_change(struct supertype *st,
> unsigned long long current_size;
> unsigned long long free_size;
> unsigned long long max_size;
> - int rv;
> + imsm_status_t rv;
>
> getinfo_super_imsm_volume(st, &info, NULL);
> if (geo->level != info.array.level && geo->level >= 0 &&
> @@ -11643,9 +11689,10 @@ enum imsm_reshape_type imsm_analyze_change(struct supertype *st,
> }
> /* check the maximum available size
> */
> - rv = imsm_get_free_size(st, dev->vol.map->num_members,
> - 0, chunk, &free_size);
> - if (rv == 0)
> + rv = imsm_get_free_size(super, dev->vol.map->num_members,
> + 0, chunk, &free_size);
> +
> + if (rv != IMSM_STATUS_OK)
> /* Cannot find maximum available space
> */
> max_size = 0;
> --
> 2.26.2
>
