Re: [PATCH 1/1] block: improve alignment detection and fix 271 test

["Denis V. Lunev" <den@virtuozzo.com>]

On 9/7/23 23:53, Denis V. Lunev wrote:
> Unfortunately 271 IO test is broken if started in non-cached mode.
>
> Commits
>      commit a6b257a08e3d72219f03e461a52152672fec0612
>      Author: Nir Soffer <nirsof@gmail.com>
>      Date:   Tue Aug 13 21:21:03 2019 +0300
>      file-posix: Handle undetectable alignment
> and
>      commit 9c60a5d1978e6dcf85c0e01b50e6f7f54ca09104
>      Author: Kevin Wolf <kwolf@redhat.com>
>      Date:   Thu Jul 16 16:26:00 2020 +0200
>      block: Require aligned image size to avoid assertion failure
> have interesting side effect if used togather.
>
> If the image size is not multiple of 4k and that image falls under
> original constraints of Nil's patch, the image can not be opened
> due to the check in the bdrv_check_perm().
>
> The patch tries to satisfy the requirements of bdrv_check_perm()
> inside raw_probe_alignment(). This is at my opinion better that just
> disallowing to run that test in non-cached mode. The operation is legal
> by itself.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Nir Soffer <nirsof@gmail.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Alberto Garcia <berto@igalia.com>
> ---
>   block/file-posix.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index b16e9c21a1..988cfdc76c 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -447,8 +447,21 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>           for (i = 0; i < ARRAY_SIZE(alignments); i++) {
>               align = alignments[i];
>               if (raw_is_io_aligned(fd, buf, align)) {
> -                /* Fallback to safe value. */
> -                bs->bl.request_alignment = (align != 1) ? align : max_align;
> +                if (align != 1) {
> +                    bs->bl.request_alignment = align;
> +                    break;
> +                }
> +                /*
> +                 * Fallback to safe value. max_align is perfect, but the size of the device must be multiple of
> +                 * the virtual length of the device. In the other case we will get a error in
> +                 * bdrv_node_refresh_perm().
> +                 */
> +                for (align = max_align; align > 1; align /= 2) {
> +                    if ((bs->total_sectors * BDRV_SECTOR_SIZE) % align == 0) {
> +                        break;
> +                    }
> +                }
> +                bs->bl.request_alignment = align;
>                   break;
>               }
>           }
ping