Re: [PATCH] block/file-posix: fix update_zones_wp() caller

[Damien Le Moal <dlemoal@kernel.org>]

On 8/25/23 02:39, Sam Li wrote:
> When the zoned requests that may change wp fail, it needs to
> update only wps of the zones within the range of the requests
> for not disrupting the other in-flight requests. The wp is updated
> successfully after the request completes.
> 
> Fixed the callers with right offset and nr_zones.
> 
> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> ---
>  block/file-posix.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index b16e9c21a1..22559d6c2d 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2522,7 +2522,8 @@ out:
>          }
>      } else {
>          if (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) {
> -            update_zones_wp(bs, s->fd, 0, 1);
> +            update_zones_wp(bs, s->fd, offset,
> +                            ROUND_UP(bytes, bs->bl.zone_size));

Write and zone append operations are not allowed to cross zone boundaries. So I
the number of zones should always be 1. The above changes a number of zones to a
number of bytes, which seems wrong. The correct fix is I think:

		update_zones_wp(bs, s->fd, offset, 1);

>          }
>      }
>  
> @@ -3472,7 +3473,7 @@ static int coroutine_fn raw_co_zone_mgmt(BlockDriverState *bs, BlockZoneOp op,
>                          len >> BDRV_SECTOR_BITS);
>      ret = raw_thread_pool_submit(handle_aiocb_zone_mgmt, &acb);
>      if (ret != 0) {
> -        update_zones_wp(bs, s->fd, offset, i);
> +        update_zones_wp(bs, s->fd, offset, nrz);

Same here. Why would you need to update all zones wp ? This will affect zones
that do not have a write error and potentially change there correct in-memory wp
to a wrong value. I think this also should be:

	   update_zones_wp(bs, s->fd, offset, 1);

>          error_report("ioctl %s failed %d", op_name, ret);
>          return ret;
>      }

-- 
Damien Le Moal
Western Digital Research