From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59026) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8DPL-0002BD-0z for qemu-devel@nongnu.org; Wed, 01 Jun 2016 17:10:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8DPI-0008Dn-VC for qemu-devel@nongnu.org; Wed, 01 Jun 2016 17:10:30 -0400 From: Eric Blake Date: Wed, 1 Jun 2016 15:10:04 -0600 Message-Id: <1464815413-613-5-git-send-email-eblake@redhat.com> In-Reply-To: <1464815413-613-1-git-send-email-eblake@redhat.com> References: <1464815413-613-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v2 04/13] block: Switch bdrv_write_zeroes() to byte interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, qemu-block@nongnu.org, Max Reitz , Stefan Hajnoczi , Fam Zheng , "Denis V. Lunev" , Juan Quintela , Amit Shah 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(). Two of the three places map to the byte-based counterparts; but for now, since we have no byte-based aio write, we still require sector alignment in those callers, via assertions. Signed-off-by: Eric Blake --- include/block/block.h | 16 ++++++++-------- block/blkreplay.c | 4 +++- block/io.c | 45 ++++++++++++++++++++++++++++----------------- block/parallels.c | 4 +++- block/qcow2-cluster.c | 3 +-- block/qcow2.c | 9 ++++----- block/raw_bsd.c | 3 ++- migration/block.c | 5 +++-- tests/qemu-iotests/034 | 2 +- tests/qemu-iotests/154 | 2 +- trace-events | 4 ++-- 11 files changed, 56 insertions(+), 41 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 0c3e709..7e87451 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -33,7 +33,7 @@ typedef struct BlockDriverInfo { * True if the driver can optimize writing zeroes by unmapping * sectors. This is equivalent to the BLKDISCARDZEROES ioctl in Linux * with the difference that in qemu a discard is allowed to silently - * fail. Therefore we have to use bdrv_write_zeroes with the + * fail. Therefore we have to use bdrv_pwrite_zeroes with the * BDRV_REQ_MAY_UNMAP flag for an optimized zero write with unmapping. * After this call the driver has to guarantee that the contents read * back as zero. It is additionally required that the block device is @@ -227,11 +227,11 @@ int bdrv_read(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, int nb_sectors); int bdrv_write(BlockDriverState *bs, int64_t sector_num, const uint8_t *buf, int nb_sectors); -int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, BdrvRequestFlags flags); -BlockAIOCB *bdrv_aio_write_zeroes(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, BdrvRequestFlags flags, - BlockCompletionFunc *cb, void *opaque); +int bdrv_pwrite_zeroes(BlockDriverState *bs, int64_t offset, + int count, BdrvRequestFlags flags); +BlockAIOCB *bdrv_aio_pwrite_zeroes(BlockDriverState *bs, int64_t offset, + int count, BdrvRequestFlags flags, + BlockCompletionFunc *cb, void *opaque); int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags); int bdrv_pread(BlockDriverState *bs, int64_t offset, void *buf, int count); @@ -250,8 +250,8 @@ int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num, * function is not suitable for zeroing the entire image in a single request * because it may allocate memory for the entire region. */ -int coroutine_fn bdrv_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, BdrvRequestFlags flags); +int coroutine_fn bdrv_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, + int count, BdrvRequestFlags flags); BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *backing_file); int bdrv_get_backing_file_depth(BlockDriverState *bs); diff --git a/block/blkreplay.c b/block/blkreplay.c index 42f1813..1a721ad 100755 --- a/block/blkreplay.c +++ b/block/blkreplay.c @@ -107,7 +107,9 @@ static int coroutine_fn blkreplay_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags) { uint64_t reqid = request_id++; - int ret = bdrv_co_write_zeroes(bs->file->bs, sector_num, nb_sectors, flags); + int ret = bdrv_co_pwrite_zeroes(bs->file->bs, + sector_num << BDRV_SECTOR_BITS, + nb_sectors << BDRV_SECTOR_BITS, flags); block_request_create(reqid, bs, qemu_coroutine_self()); qemu_coroutine_yield(); diff --git a/block/io.c b/block/io.c index 3fe7576..5f021d5 100644 --- a/block/io.c +++ b/block/io.c @@ -620,18 +620,25 @@ 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, - BDRV_REQ_ZERO_WRITE | flags); + QEMUIOVector qiov; + struct iovec iov = { + .iov_base = NULL, + .iov_len = count, + }; + + qemu_iovec_init_external(&qiov, &iov, 1); + return bdrv_prwv_co(bs, offset, &qiov, true, + BDRV_REQ_ZERO_WRITE | flags); } /* - * Completely zero out a block device with the help of bdrv_write_zeroes. + * Completely zero out a block device with the help of bdrv_pwrite_zeroes. * The operation is sped up by checking the block status and only writing * zeroes to the device if they currently do not return zeroes. Optional - * flags are passed through to bdrv_write_zeroes (e.g. BDRV_REQ_MAY_UNMAP, + * flags are passed through to bdrv_pwrite_zeroes (e.g. BDRV_REQ_MAY_UNMAP, * BDRV_REQ_FUA). * * Returns < 0 on error, 0 on success. For error codes see bdrv_write(). @@ -662,7 +669,8 @@ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags) sector_num += n; continue; } - ret = bdrv_write_zeroes(bs, sector_num, n, flags); + ret = bdrv_pwrite_zeroes(bs, sector_num << BDRV_SECTOR_BITS, + n << BDRV_SECTOR_BITS, flags); if (ret < 0) { error_report("error writing zeroes at sector %" PRId64 ": %s", sector_num, strerror(-ret)); @@ -1518,18 +1526,18 @@ int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num, return bdrv_co_do_writev(bs, sector_num, nb_sectors, qiov, 0); } -int coroutine_fn bdrv_co_write_zeroes(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, - BdrvRequestFlags flags) +int coroutine_fn bdrv_co_pwrite_zeroes(BlockDriverState *bs, + int64_t offset, int count, + BdrvRequestFlags flags) { - trace_bdrv_co_write_zeroes(bs, sector_num, nb_sectors, flags); + trace_bdrv_co_pwrite_zeroes(bs, offset, count, flags); if (!(bs->open_flags & BDRV_O_UNMAP)) { flags &= ~BDRV_REQ_MAY_UNMAP; } - return bdrv_co_do_writev(bs, sector_num, nb_sectors, NULL, - BDRV_REQ_ZERO_WRITE | flags); + return bdrv_co_pwritev(bs, offset, count, NULL, + BDRV_REQ_ZERO_WRITE | flags); } typedef struct BdrvCoGetBlockStatusData { @@ -1881,13 +1889,16 @@ BlockAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num, cb, opaque, true); } -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); } diff --git a/block/parallels.c b/block/parallels.c index 99fc0f7..4621553 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -210,7 +210,9 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num, int ret; space += s->prealloc_size; if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) { - ret = bdrv_write_zeroes(bs->file->bs, s->data_end, space, 0); + ret = bdrv_pwrite_zeroes(bs->file->bs, + s->data_end << BDRV_SECTOR_BITS, + space << BDRV_SECTOR_BITS, 0); } else { ret = bdrv_truncate(bs->file->bs, (s->data_end + space) << BDRV_SECTOR_BITS); diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 892e0fb..d901d89 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1765,8 +1765,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, goto fail; } - ret = bdrv_write_zeroes(bs->file->bs, offset / BDRV_SECTOR_SIZE, - s->cluster_sectors, 0); + ret = bdrv_pwrite_zeroes(bs->file->bs, offset, s->cluster_size, 0); if (ret < 0) { if (!preallocated) { qcow2_free_clusters(bs, offset, s->cluster_size, diff --git a/block/qcow2.c b/block/qcow2.c index a6ea6cb..cc59efc 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2655,8 +2655,8 @@ static int make_completely_empty(BlockDriverState *bs) /* After this call, neither the in-memory nor the on-disk refcount * information accurately describe the actual references */ - ret = bdrv_write_zeroes(bs->file->bs, s->l1_table_offset / BDRV_SECTOR_SIZE, - l1_clusters * s->cluster_sectors, 0); + ret = bdrv_pwrite_zeroes(bs->file->bs, s->l1_table_offset, + l1_clusters * s->cluster_size, 0); if (ret < 0) { goto fail_broken_refcounts; } @@ -2669,9 +2669,8 @@ static int make_completely_empty(BlockDriverState *bs) * overwrite parts of the existing refcount and L1 table, which is not * an issue because the dirty flag is set, complete data loss is in fact * desired and partial data loss is consequently fine as well */ - ret = bdrv_write_zeroes(bs->file->bs, s->cluster_size / BDRV_SECTOR_SIZE, - (2 + l1_clusters) * s->cluster_size / - BDRV_SECTOR_SIZE, 0); + ret = bdrv_pwrite_zeroes(bs->file->bs, s->cluster_size, + (2 + l1_clusters) * s->cluster_size, 0); /* This call (even if it failed overall) may have overwritten on-disk * refcount structures; in that case, the in-memory refcount information * will probably differ from the on-disk information which makes the BDS diff --git a/block/raw_bsd.c b/block/raw_bsd.c index 3385ed4..d9adf90 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -131,7 +131,8 @@ static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags) { - return bdrv_co_write_zeroes(bs->file->bs, sector_num, nb_sectors, flags); + return bdrv_co_pwrite_zeroes(bs->file->bs, sector_num << BDRV_SECTOR_BITS, + nb_sectors << BDRV_SECTOR_BITS, flags); } static int coroutine_fn raw_co_discard(BlockDriverState *bs, diff --git a/migration/block.c b/migration/block.c index e0628d1..16cc1f8 100644 --- a/migration/block.c +++ b/migration/block.c @@ -883,8 +883,9 @@ static int block_load(QEMUFile *f, void *opaque, int version_id) } if (flags & BLK_MIG_FLAG_ZERO_BLOCK) { - ret = bdrv_write_zeroes(bs, addr, nr_sectors, - BDRV_REQ_MAY_UNMAP); + ret = bdrv_pwrite_zeroes(bs, addr << BDRV_SECTOR_BITS, + nr_sectors << BDRV_SECTOR_BITS, + BDRV_REQ_MAY_UNMAP); } else { buf = g_malloc(BLOCK_SIZE); qemu_get_buffer(f, buf, BLOCK_SIZE); diff --git a/tests/qemu-iotests/034 b/tests/qemu-iotests/034 index c711cfc..1b28bda 100755 --- a/tests/qemu-iotests/034 +++ b/tests/qemu-iotests/034 @@ -1,6 +1,6 @@ #!/bin/bash # -# Test bdrv_write_zeroes with backing files +# Test bdrv_pwrite_zeroes with backing files (see also 154) # # Copyright (C) 2012 Red Hat, Inc. # diff --git a/tests/qemu-iotests/154 b/tests/qemu-iotests/154 index 5905c55..7ca7219 100755 --- a/tests/qemu-iotests/154 +++ b/tests/qemu-iotests/154 @@ -1,6 +1,6 @@ #!/bin/bash # -# qcow2 specific bdrv_write_zeroes tests with backing files (complements 034) +# qcow2 specific bdrv_pwrite_zeroes tests with backing files (complements 034) # # Copyright (C) 2016 Red Hat, Inc. # diff --git a/trace-events b/trace-events index da930d5..6329c01 100644 --- a/trace-events +++ b/trace-events @@ -70,10 +70,10 @@ bdrv_aio_discard(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p" bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" -bdrv_aio_write_zeroes(void *bs, int64_t sector_num, int nb_sectors, int flags, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x opaque %p" +bdrv_aio_pwrite_zeroes(void *bs, int64_t offset, int count, int flags, void *opaque) "bs %p offset %"PRId64" count %d flags %#x opaque %p" bdrv_co_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d" bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d" -bdrv_co_write_zeroes(void *bs, int64_t sector_num, int nb_sector, int flags) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x" +bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags %#x" bdrv_co_do_copy_on_readv(void *bs, int64_t sector_num, int nb_sectors, int64_t cluster_sector_num, int cluster_nb_sectors) "bs %p sector_num %"PRId64" nb_sectors %d cluster_sector_num %"PRId64" cluster_nb_sectors %d" # block/stream.c -- 2.5.5