From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:59769)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1b5YiE-0000WM-U5
	for qemu-devel@nongnu.org; Wed, 25 May 2016 09:19:08 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1b5Yi9-0005eH-Qm
	for qemu-devel@nongnu.org; Wed, 25 May 2016 09:19:01 -0400
Date: Wed, 25 May 2016 15:18:42 +0200
From: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20160525131842.GG4815@noname.redhat.com>
References: <1464128732-12667-1-git-send-email-eblake@redhat.com>
	<1464128732-12667-5-git-send-email-eblake@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1464128732-12667-5-git-send-email-eblake@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 04/13] block: Switch bdrv_write_zeroes() to
 byte interface
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Jeff Cody <jcody@redhat.com>, Max Reitz <mreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <famz@redhat.com>, "Denis V. Lunev" <den@openvz.org>, Juan Quintela <quintela@redhat.com>, Amit Shah <amit.shah@redhat.com>

Am 25.05.2016 um 00:25 hat Eric Blake geschrieben:
> Rename to bdrv_pwrite_zeroes() to let the compiler ensure we
> cater to the updated semantics.  Do the same for
> bdrv_aio_write_zeroes() and bdrv_co_write_zeroes().  For now,
> we still require sector alignment in the callers, via assertions.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

> --- a/block/io.c
> +++ b/block/io.c
> @@ -603,18 +603,21 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,
>      return bdrv_rw_co(bs, sector_num, (uint8_t *)buf, nb_sectors, true, 0);
>  }
> 
> -int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num,
> -                      int nb_sectors, BdrvRequestFlags flags)
> +int bdrv_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
> +                       int count, BdrvRequestFlags flags)
>  {
> -    return bdrv_rw_co(bs, sector_num, NULL, nb_sectors, true,
> +    assert(offset % BDRV_SECTOR_SIZE == 0);
> +    assert(count % BDRV_SECTOR_SIZE == 0);
> +    return bdrv_rw_co(bs, offset >> BDRV_SECTOR_BITS, NULL,
> +                      count >> BDRV_SECTOR_BITS, true,
>                        BDRV_REQ_ZERO_WRITE | flags);
>  }

Should we go directly to bdrv_prwv_co() here so that we don't need to
assert BDRV_SECTOR_SIZE alignment in a byte-based function?

> -BlockAIOCB *bdrv_aio_write_zeroes(BlockDriverState *bs,
> -        int64_t sector_num, int nb_sectors, BdrvRequestFlags flags,
> +BlockAIOCB *bdrv_aio_pwrite_zeroes(BlockDriverState *bs,
> +        int64_t offset, int count, BdrvRequestFlags flags,
>          BlockCompletionFunc *cb, void *opaque)
>  {
> -    trace_bdrv_aio_write_zeroes(bs, sector_num, nb_sectors, flags, opaque);
> +    trace_bdrv_aio_pwrite_zeroes(bs, offset, count, flags, opaque);
> +    assert(offset % BDRV_SECTOR_SIZE == 0);
> +    assert(count % BDRV_SECTOR_SIZE == 0);
> 
> -    return bdrv_co_aio_rw_vector(bs, sector_num, NULL, nb_sectors,
> +    return bdrv_co_aio_rw_vector(bs, offset >> BDRV_SECTOR_BITS, NULL,
> +                                 count >> BDRV_SECTOR_BITS,
>                                   BDRV_REQ_ZERO_WRITE | flags,
>                                   cb, opaque, true);
>  }

Here the same would be nice, but we don't have a byte-based AIO
interface yet, so I'd agree with leaving the assertion here.

Kevin