The following changes since commit fc3dbb90f2eb069801bfb4cfe9cbc83cf9c5f4a9: Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' into staging (2019-02-21 13:09:33 +0000) are available in the Git repository at: git://github.com/stefanha/qemu.git tags/block-pull-request for you to fetch changes up to 9a9f4b74fa547b68edb38fa414999836770a4735: tests/virtio-blk: add test for DISCARD command (2019-02-22 09:42:17 +0000) ---------------------------------------------------------------- Pull request ---------------------------------------------------------------- Stefano Garzarella (10): virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() virtio-blk: add host_features field in VirtIOBlock virtio-blk: add "discard" and "write-zeroes" properties virtio-net: make VirtIOFeature usable for other virtio devices virtio-blk: set config size depending on the features enabled virtio-blk: add DISCARD and WRITE_ZEROES features tests/virtio-blk: change assert on data_size in virtio_blk_request() tests/virtio-blk: add virtio_blk_fix_dwz_hdr() function tests/virtio-blk: add test for WRITE_ZEROES command tests/virtio-blk: add test for DISCARD command Vladimir Sementsov-Ogievskiy (17): block: enhance QEMUIOVector structure block/io: use qemu_iovec_init_buf block/block-backend: use QEMU_IOVEC_INIT_BUF block/backup: use qemu_iovec_init_buf block/commit: use QEMU_IOVEC_INIT_BUF block/stream: use QEMU_IOVEC_INIT_BUF block/parallels: use QEMU_IOVEC_INIT_BUF block/qcow: use qemu_iovec_init_buf block/qcow2: use qemu_iovec_init_buf block/qed: use qemu_iovec_init_buf block/vmdk: use qemu_iovec_init_buf qemu-img: use qemu_iovec_init_buf migration/block: use qemu_iovec_init_buf tests/test-bdrv-drain: use QEMU_IOVEC_INIT_BUF hw/ide: drop iov field from IDEState hw/ide: drop iov field from IDEBufferedRequest hw/ide: drop iov field from IDEDMA include/hw/ide/internal.h | 3 - include/hw/virtio/virtio-blk.h | 6 +- include/hw/virtio/virtio.h | 15 ++ include/qemu/iov.h | 64 ++++++++- block/backup.c | 5 +- block/block-backend.c | 13 +- block/commit.c | 7 +- block/io.c | 89 +++--------- block/parallels.c | 13 +- block/qcow.c | 21 +-- block/qcow2.c | 12 +- block/qed-table.c | 16 +-- block/qed.c | 31 ++--- block/stream.c | 7 +- block/vmdk.c | 7 +- hw/block/virtio-blk.c | 245 ++++++++++++++++++++++++++++++--- hw/core/machine.c | 2 + hw/ide/atapi.c | 14 +- hw/ide/core.c | 19 ++- hw/net/virtio-net.c | 31 +---- hw/virtio/virtio.c | 15 ++ migration/block.c | 10 +- qemu-img.c | 10 +- tests/test-bdrv-drain.c | 29 +--- tests/virtio-blk-test.c | 127 ++++++++++++++++- 25 files changed, 525 insertions(+), 286 deletions(-) -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Add a possibility of embedded iovec, for cases when we need only one local iov. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 20190218140926.333779-2-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-2-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/qemu/iov.h | 64 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/include/qemu/iov.h b/include/qemu/iov.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/iov.h +++ b/include/qemu/iov.h @@ -XXX,XX +XXX,XX @@ size_t iov_discard_back(struct iovec *iov, unsigned int *iov_cnt, typedef struct QEMUIOVector { struct iovec *iov; int niov; - int nalloc; - size_t size; + + /* + * For external @iov (qemu_iovec_init_external()) or allocated @iov + * (qemu_iovec_init()), @size is the cumulative size of iovecs and + * @local_iov is invalid and unused. + * + * For embedded @iov (QEMU_IOVEC_INIT_BUF() or qemu_iovec_init_buf()), + * @iov is equal to &@local_iov, and @size is valid, as it has same + * offset and type as @local_iov.iov_len, which is guaranteed by + * static assertion below. + * + * @nalloc is always valid and is -1 both for embedded and external + * cases. It is included in the union only to ensure the padding prior + * to the @size field will not result in a 0-length array. + */ + union { + struct { + int nalloc; + struct iovec local_iov; + }; + struct { + char __pad[sizeof(int) + offsetof(struct iovec, iov_len)]; + size_t size; + }; + }; } QEMUIOVector; +QEMU_BUILD_BUG_ON(offsetof(QEMUIOVector, size) != + offsetof(QEMUIOVector, local_iov.iov_len)); + +#define QEMU_IOVEC_INIT_BUF(self, buf, len) \ +{ \ + .iov = &(self).local_iov, \ + .niov = 1, \ + .nalloc = -1, \ + .local_iov = { \ + .iov_base = (void *)(buf), /* cast away const */ \ + .iov_len = (len), \ + }, \ +} + +/* + * qemu_iovec_init_buf + * + * Initialize embedded QEMUIOVector. + * + * Note: "const" is used over @buf pointer to make it simple to pass + * const pointers, appearing in read functions. Then this "const" is + * cast away by QEMU_IOVEC_INIT_BUF(). + */ +static inline void qemu_iovec_init_buf(QEMUIOVector *qiov, + const void *buf, size_t len) +{ + *qiov = (QEMUIOVector) QEMU_IOVEC_INIT_BUF(*qiov, buf, len); +} + +static inline void *qemu_iovec_buf(QEMUIOVector *qiov) +{ + /* Only supports embedded iov */ + assert(qiov->nalloc == -1 && qiov->iov == &qiov->local_iov); + + return qiov->local_iov.iov_base; +} + void qemu_iovec_init(QEMUIOVector *qiov, int alloc_hint); void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov); void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len); -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new qemu_iovec_init_buf() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. While being here, use qemu_try_blockalign0 as well. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-3-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-3-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/io.c | 89 ++++++++++++------------------------------------------ 1 file changed, 20 insertions(+), 69 deletions(-) diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ static int bdrv_prwv_co(BdrvChild *child, int64_t offset, static int bdrv_rw_co(BdrvChild *child, int64_t sector_num, uint8_t *buf, int nb_sectors, bool is_write, BdrvRequestFlags flags) { - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = (void *)buf, - .iov_len = nb_sectors * BDRV_SECTOR_SIZE, - }; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, + nb_sectors * BDRV_SECTOR_SIZE); if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) { return -EINVAL; } - qemu_iovec_init_external(&qiov, &iov, 1); return bdrv_prwv_co(child, sector_num << BDRV_SECTOR_BITS, &qiov, is_write, flags); } @@ -XXX,XX +XXX,XX @@ int bdrv_write(BdrvChild *child, int64_t sector_num, int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset, int bytes, BdrvRequestFlags flags) { - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = NULL, - .iov_len = bytes, - }; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes); - qemu_iovec_init_external(&qiov, &iov, 1); return bdrv_prwv_co(child, offset, &qiov, true, BDRV_REQ_ZERO_WRITE | flags); } @@ -XXX,XX +XXX,XX @@ int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov) int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int bytes) { - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = (void *)buf, - .iov_len = bytes, - }; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); if (bytes < 0) { return -EINVAL; } - qemu_iovec_init_external(&qiov, &iov, 1); return bdrv_preadv(child, offset, &qiov); } @@ -XXX,XX +XXX,XX @@ int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov) int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int bytes) { - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = (void *) buf, - .iov_len = bytes, - }; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); if (bytes < 0) { return -EINVAL; } - qemu_iovec_init_external(&qiov, &iov, 1); return bdrv_pwritev(child, offset, &qiov); } @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child, void *bounce_buffer; BlockDriver *drv = bs->drv; - struct iovec iov; QEMUIOVector local_qiov; int64_t cluster_offset; int64_t cluster_bytes; @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child, if (ret <= 0) { /* Must copy-on-read; use the bounce buffer */ - iov.iov_base = bounce_buffer; - iov.iov_len = pnum = MIN(pnum, MAX_BOUNCE_BUFFER); - qemu_iovec_init_external(&local_qiov, &iov, 1); + pnum = MIN(pnum, MAX_BOUNCE_BUFFER); + qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum); ret = bdrv_driver_preadv(bs, cluster_offset, pnum, &local_qiov, 0); @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, { BlockDriver *drv = bs->drv; QEMUIOVector qiov; - struct iovec iov = {0}; + void *buf = NULL; int ret = 0; bool need_flush = false; int head = 0; @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, need_flush = true; } num = MIN(num, max_transfer); - iov.iov_len = num; - if (iov.iov_base == NULL) { - iov.iov_base = qemu_try_blockalign(bs, num); - if (iov.iov_base == NULL) { + if (buf == NULL) { + buf = qemu_try_blockalign0(bs, num); + if (buf == NULL) { ret = -ENOMEM; goto fail; } - memset(iov.iov_base, 0, num); } - qemu_iovec_init_external(&qiov, &iov, 1); + qemu_iovec_init_buf(&qiov, buf, num); ret = bdrv_driver_pwritev(bs, offset, num, &qiov, write_flags); @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, * all future requests. */ if (num < max_transfer) { - qemu_vfree(iov.iov_base); - iov.iov_base = NULL; + qemu_vfree(buf); + buf = NULL; } } @@ -XXX,XX +XXX,XX @@ fail: if (ret == 0 && need_flush) { ret = bdrv_co_flush(bs); } - qemu_vfree(iov.iov_base); + qemu_vfree(buf); return ret; } @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child, BlockDriverState *bs = child->bs; uint8_t *buf = NULL; QEMUIOVector local_qiov; - struct iovec iov; uint64_t align = bs->bl.request_alignment; unsigned int head_padding_bytes, tail_padding_bytes; int ret = 0; @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child, assert(flags & BDRV_REQ_ZERO_WRITE); if (head_padding_bytes || tail_padding_bytes) { buf = qemu_blockalign(bs, align); - iov = (struct iovec) { - .iov_base = buf, - .iov_len = align, - }; - qemu_iovec_init_external(&local_qiov, &iov, 1); + qemu_iovec_init_buf(&local_qiov, buf, align); } if (head_padding_bytes) { uint64_t zero_bytes = MIN(bytes, align - head_padding_bytes); @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, if (offset & (align - 1)) { QEMUIOVector head_qiov; - struct iovec head_iov; mark_request_serialising(&req, align); wait_serialising_requests(&req); head_buf = qemu_blockalign(bs, align); - head_iov = (struct iovec) { - .iov_base = head_buf, - .iov_len = align, - }; - qemu_iovec_init_external(&head_qiov, &head_iov, 1); + qemu_iovec_init_buf(&head_qiov, head_buf, align); bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD); ret = bdrv_aligned_preadv(child, &req, offset & ~(align - 1), align, @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, if ((offset + bytes) & (align - 1)) { QEMUIOVector tail_qiov; - struct iovec tail_iov; size_t tail_bytes; bool waited; @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, assert(!waited || !use_local_qiov); tail_buf = qemu_blockalign(bs, align); - tail_iov = (struct iovec) { - .iov_base = tail_buf, - .iov_len = align, - }; - qemu_iovec_init_external(&tail_qiov, &tail_iov, 1); + qemu_iovec_init_buf(&tail_qiov, tail_buf, align); bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL); ret = bdrv_aligned_preadv(child, &req, (offset + bytes) & ~(align - 1), @@ -XXX,XX +XXX,XX @@ bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf, int64_t pos, int size) { - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = (void *) buf, - .iov_len = size, - }; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size); int ret; - qemu_iovec_init_external(&qiov, &iov, 1); - ret = bdrv_writev_vmstate(bs, &qiov, pos); if (ret < 0) { return ret; @@ -XXX,XX +XXX,XX @@ int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos) int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf, int64_t pos, int size) { - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = buf, - .iov_len = size, - }; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size); int ret; - qemu_iovec_init_external(&qiov, &iov, 1); ret = bdrv_readv_vmstate(bs, &qiov, pos); if (ret < 0) { return ret; -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new QEMU_IOVEC_INIT_BUF() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-4-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-4-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/block-backend.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index XXXXXXX..XXXXXXX 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -XXX,XX +XXX,XX @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf, int64_t bytes, CoroutineEntry co_entry, BdrvRequestFlags flags) { - QEMUIOVector qiov; - struct iovec iov; - BlkRwCo rwco; - - iov = (struct iovec) { - .iov_base = buf, - .iov_len = bytes, - }; - qemu_iovec_init_external(&qiov, &iov, 1); - - rwco = (BlkRwCo) { + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); + BlkRwCo rwco = { .blk = blk, .offset = offset, .iobuf = &qiov, -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new qemu_iovec_init_buf() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-5-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-5-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/backup.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/block/backup.c b/block/backup.c index XXXXXXX..XXXXXXX 100644 --- a/block/backup.c +++ b/block/backup.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job, void **bounce_buffer) { int ret; - struct iovec iov; QEMUIOVector qiov; BlockBackend *blk = job->common.blk; int nbytes; @@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job, if (!*bounce_buffer) { *bounce_buffer = blk_blockalign(blk, job->cluster_size); } - iov.iov_base = *bounce_buffer; - iov.iov_len = nbytes; - qemu_iovec_init_external(&qiov, &iov, 1); + qemu_iovec_init_buf(&qiov, *bounce_buffer, nbytes); ret = blk_co_preadv(blk, start, qiov.size, &qiov, read_flags); if (ret < 0) { -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new QEMU_IOVEC_INIT_BUF() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-6-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-6-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/commit.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/block/commit.c b/block/commit.c index XXXXXXX..XXXXXXX 100644 --- a/block/commit.c +++ b/block/commit.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn commit_populate(BlockBackend *bs, BlockBackend *base, void *buf) { int ret = 0; - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = buf, - .iov_len = bytes, - }; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); assert(bytes < SIZE_MAX); - qemu_iovec_init_external(&qiov, &iov, 1); ret = blk_co_preadv(bs, offset, qiov.size, &qiov, 0); if (ret < 0) { -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new QEMU_IOVEC_INIT_BUF() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-7-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-7-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/stream.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/block/stream.c b/block/stream.c index XXXXXXX..XXXXXXX 100644 --- a/block/stream.c +++ b/block/stream.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn stream_populate(BlockBackend *blk, int64_t offset, uint64_t bytes, void *buf) { - struct iovec iov = { - .iov_base = buf, - .iov_len = bytes, - }; - QEMUIOVector qiov; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); assert(bytes < SIZE_MAX); - qemu_iovec_init_external(&qiov, &iov, 1); /* Copy-on-read the unallocated clusters */ return blk_co_preadv(blk, offset, qiov.size, &qiov, BDRV_REQ_COPY_ON_READ); -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new QEMU_IOVEC_INIT_BUF() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-8-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-8-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/parallels.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index XXXXXXX..XXXXXXX 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -XXX,XX +XXX,XX @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num, if (bs->backing) { int64_t nb_cow_sectors = to_allocate * s->tracks; int64_t nb_cow_bytes = nb_cow_sectors << BDRV_SECTOR_BITS; - QEMUIOVector qiov; - struct iovec iov = { - .iov_len = nb_cow_bytes, - .iov_base = qemu_blockalign(bs, nb_cow_bytes) - }; - qemu_iovec_init_external(&qiov, &iov, 1); + QEMUIOVector qiov = + QEMU_IOVEC_INIT_BUF(qiov, qemu_blockalign(bs, nb_cow_bytes), + nb_cow_bytes); ret = bdrv_co_preadv(bs->backing, idx * s->tracks * BDRV_SECTOR_SIZE, nb_cow_bytes, &qiov, 0); if (ret < 0) { - qemu_vfree(iov.iov_base); + qemu_vfree(qemu_iovec_buf(&qiov)); return ret; } ret = bdrv_co_pwritev(bs->file, s->data_end * BDRV_SECTOR_SIZE, nb_cow_bytes, &qiov, 0); - qemu_vfree(iov.iov_base); + qemu_vfree(qemu_iovec_buf(&qiov)); if (ret < 0) { return ret; } -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new qemu_iovec_init_buf() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-9-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-9-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/qcow.c | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, uint64_t offset, int offset_in_cluster; int ret = 0, n; uint64_t cluster_offset; - struct iovec hd_iov; QEMUIOVector hd_qiov; uint8_t *buf; void *orig_buf; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, uint64_t offset, if (!cluster_offset) { if (bs->backing) { /* read from the base image */ - hd_iov.iov_base = (void *)buf; - hd_iov.iov_len = n; - qemu_iovec_init_external(&hd_qiov, &hd_iov, 1); + qemu_iovec_init_buf(&hd_qiov, buf, n); qemu_co_mutex_unlock(&s->lock); /* qcow2 emits this on bs->file instead of bs->backing */ BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO); @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_preadv(BlockDriverState *bs, uint64_t offset, ret = -EIO; break; } - hd_iov.iov_base = (void *)buf; - hd_iov.iov_len = n; - qemu_iovec_init_external(&hd_qiov, &hd_iov, 1); + qemu_iovec_init_buf(&hd_qiov, buf, n); qemu_co_mutex_unlock(&s->lock); BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO); ret = bdrv_co_preadv(bs->file, cluster_offset + offset_in_cluster, @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_pwritev(BlockDriverState *bs, uint64_t offset, int offset_in_cluster; uint64_t cluster_offset; int ret = 0, n; - struct iovec hd_iov; QEMUIOVector hd_qiov; uint8_t *buf; void *orig_buf; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow_co_pwritev(BlockDriverState *bs, uint64_t offset, } } - hd_iov.iov_base = (void *)buf; - hd_iov.iov_len = n; - qemu_iovec_init_external(&hd_qiov, &hd_iov, 1); + qemu_iovec_init_buf(&hd_qiov, buf, n); qemu_co_mutex_unlock(&s->lock); BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO); ret = bdrv_co_pwritev(bs->file, cluster_offset + offset_in_cluster, @@ -XXX,XX +XXX,XX @@ qcow_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, { BDRVQcowState *s = bs->opaque; QEMUIOVector hd_qiov; - struct iovec iov; z_stream strm; int ret, out_len; uint8_t *buf, *out_buf; @@ -XXX,XX +XXX,XX @@ qcow_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, } cluster_offset &= s->cluster_offset_mask; - iov = (struct iovec) { - .iov_base = out_buf, - .iov_len = out_len, - }; - qemu_iovec_init_external(&hd_qiov, &iov, 1); + qemu_iovec_init_buf(&hd_qiov, out_buf, out_len); BLKDBG_EVENT(bs->file, BLKDBG_WRITE_COMPRESSED); ret = bdrv_co_pwritev(bs->file, cluster_offset, out_len, &hd_qiov, 0); if (ret < 0) { -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new qemu_iovec_init_buf() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-10-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-10-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/qcow2.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, { BDRVQcow2State *s = bs->opaque; QEMUIOVector hd_qiov; - struct iovec iov; int ret; size_t out_len; uint8_t *buf, *out_buf; @@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, goto fail; } - iov = (struct iovec) { - .iov_base = out_buf, - .iov_len = out_len, - }; - qemu_iovec_init_external(&hd_qiov, &iov, 1); + qemu_iovec_init_buf(&hd_qiov, out_buf, out_len); BLKDBG_EVENT(bs->file, BLKDBG_WRITE_COMPRESSED); ret = bdrv_co_pwritev(bs->file, cluster_offset, out_len, &hd_qiov, 0); @@ -XXX,XX +XXX,XX @@ qcow2_co_preadv_compressed(BlockDriverState *bs, int ret = 0, csize, nb_csectors; uint64_t coffset; uint8_t *buf, *out_buf; - struct iovec iov; QEMUIOVector local_qiov; int offset_in_cluster = offset_into_cluster(s, offset); @@ -XXX,XX +XXX,XX @@ qcow2_co_preadv_compressed(BlockDriverState *bs, if (!buf) { return -ENOMEM; } - iov.iov_base = buf; - iov.iov_len = csize; - qemu_iovec_init_external(&local_qiov, &iov, 1); + qemu_iovec_init_buf(&local_qiov, buf, csize); out_buf = qemu_blockalign(bs, s->cluster_size); -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new qemu_iovec_init_buf() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-11-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-11-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/qed-table.c | 16 +++------------- block/qed.c | 31 +++++++++---------------------- 2 files changed, 12 insertions(+), 35 deletions(-) diff --git a/block/qed-table.c b/block/qed-table.c index XXXXXXX..XXXXXXX 100644 --- a/block/qed-table.c +++ b/block/qed-table.c @@ -XXX,XX +XXX,XX @@ /* Called with table_lock held. */ static int qed_read_table(BDRVQEDState *s, uint64_t offset, QEDTable *table) { - QEMUIOVector qiov; + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF( + qiov, table->offsets, s->header.cluster_size * s->header.table_size); int noffsets; int i, ret; - struct iovec iov = { - .iov_base = table->offsets, - .iov_len = s->header.cluster_size * s->header.table_size, - }; - qemu_iovec_init_external(&qiov, &iov, 1); - trace_qed_read_table(s, offset, table); qemu_co_mutex_unlock(&s->table_lock); @@ -XXX,XX +XXX,XX @@ static int qed_write_table(BDRVQEDState *s, uint64_t offset, QEDTable *table, unsigned int sector_mask = BDRV_SECTOR_SIZE / sizeof(uint64_t) - 1; unsigned int start, end, i; QEDTable *new_table; - struct iovec iov; QEMUIOVector qiov; size_t len_bytes; int ret; @@ -XXX,XX +XXX,XX @@ static int qed_write_table(BDRVQEDState *s, uint64_t offset, QEDTable *table, len_bytes = (end - start) * sizeof(uint64_t); new_table = qemu_blockalign(s->bs, len_bytes); - iov = (struct iovec) { - .iov_base = new_table->offsets, - .iov_len = len_bytes, - }; - qemu_iovec_init_external(&qiov, &iov, 1); + qemu_iovec_init_buf(&qiov, new_table->offsets, len_bytes); /* Byteswap table */ for (i = start; i < end; i++) { diff --git a/block/qed.c b/block/qed.c index XXXXXXX..XXXXXXX 100644 --- a/block/qed.c +++ b/block/qed.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qed_write_header(BDRVQEDState *s) int nsectors = DIV_ROUND_UP(sizeof(QEDHeader), BDRV_SECTOR_SIZE); size_t len = nsectors * BDRV_SECTOR_SIZE; uint8_t *buf; - struct iovec iov; QEMUIOVector qiov; int ret; assert(s->allocating_acb || s->allocating_write_reqs_plugged); buf = qemu_blockalign(s->bs, len); - iov = (struct iovec) { - .iov_base = buf, - .iov_len = len, - }; - qemu_iovec_init_external(&qiov, &iov, 1); + qemu_iovec_init_buf(&qiov, buf, len); ret = bdrv_co_preadv(s->bs->file, 0, qiov.size, &qiov, 0); if (ret < 0) { @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qed_copy_from_backing_file(BDRVQEDState *s, { QEMUIOVector qiov; QEMUIOVector *backing_qiov = NULL; - struct iovec iov; int ret; /* Skip copy entirely if there is no work to do */ @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qed_copy_from_backing_file(BDRVQEDState *s, return 0; } - iov = (struct iovec) { - .iov_base = qemu_blockalign(s->bs, len), - .iov_len = len, - }; - qemu_iovec_init_external(&qiov, &iov, 1); + qemu_iovec_init_buf(&qiov, qemu_blockalign(s->bs, len), len); ret = qed_read_backing_file(s, pos, &qiov, &backing_qiov); @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qed_copy_from_backing_file(BDRVQEDState *s, } ret = 0; out: - qemu_vfree(iov.iov_base); + qemu_vfree(qemu_iovec_buf(&qiov)); return ret; } @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs, BdrvRequestFlags flags) { BDRVQEDState *s = bs->opaque; - QEMUIOVector qiov; - struct iovec iov; + + /* + * Zero writes start without an I/O buffer. If a buffer becomes necessary + * then it will be allocated during request processing. + */ + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes); /* Fall back if the request is not aligned */ if (qed_offset_into_cluster(s, offset) || @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs, return -ENOTSUP; } - /* Zero writes start without an I/O buffer. If a buffer becomes necessary - * then it will be allocated during request processing. - */ - iov.iov_base = NULL; - iov.iov_len = bytes; - - qemu_iovec_init_external(&qiov, &iov, 1); return qed_co_request(bs, offset >> BDRV_SECTOR_BITS, &qiov, bytes >> BDRV_SECTOR_BITS, QED_AIOCB_WRITE | QED_AIOCB_ZERO); -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new qemu_iovec_init_buf() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-12-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-12-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/vmdk.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index XXXXXXX..XXXXXXX 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -XXX,XX +XXX,XX @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset, VmdkGrainMarker *data = NULL; uLongf buf_len; QEMUIOVector local_qiov; - struct iovec iov; int64_t write_offset; int64_t write_end_sector; @@ -XXX,XX +XXX,XX @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset, data->size = cpu_to_le32(buf_len); n_bytes = buf_len + sizeof(VmdkGrainMarker); - iov = (struct iovec) { - .iov_base = data, - .iov_len = n_bytes, - }; - qemu_iovec_init_external(&local_qiov, &iov, 1); + qemu_iovec_init_buf(&local_qiov, data, n_bytes); BLKDBG_EVENT(extent->file, BLKDBG_WRITE_COMPRESSED); } else { -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new qemu_iovec_init_buf() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-13-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-13-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- qemu-img.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index XXXXXXX..XXXXXXX 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num, { int n, ret; QEMUIOVector qiov; - struct iovec iov; assert(nb_sectors <= s->buf_sectors); while (nb_sectors > 0) { @@ -XXX,XX +XXX,XX @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num, bs_sectors = s->src_sectors[src_cur]; n = MIN(nb_sectors, bs_sectors - (sector_num - src_cur_offset)); - iov.iov_base = buf; - iov.iov_len = n << BDRV_SECTOR_BITS; - qemu_iovec_init_external(&qiov, &iov, 1); + qemu_iovec_init_buf(&qiov, buf, n << BDRV_SECTOR_BITS); ret = blk_co_preadv( blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS, @@ -XXX,XX +XXX,XX @@ static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num, { int ret; QEMUIOVector qiov; - struct iovec iov; while (nb_sectors > 0) { int n = nb_sectors; @@ -XXX,XX +XXX,XX @@ static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num, (s->compressed && !buffer_is_zero(buf, n * BDRV_SECTOR_SIZE))) { - iov.iov_base = buf; - iov.iov_len = n << BDRV_SECTOR_BITS; - qemu_iovec_init_external(&qiov, &iov, 1); + qemu_iovec_init_buf(&qiov, buf, n << BDRV_SECTOR_BITS); ret = blk_co_pwritev(s->target, sector_num << BDRV_SECTOR_BITS, n << BDRV_SECTOR_BITS, &qiov, flags); -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new qemu_iovec_init_buf() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-14-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-14-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- migration/block.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/migration/block.c b/migration/block.c index XXXXXXX..XXXXXXX 100644 --- a/migration/block.c +++ b/migration/block.c @@ -XXX,XX +XXX,XX @@ typedef struct BlkMigBlock { BlkMigDevState *bmds; int64_t sector; int nr_sectors; - struct iovec iov; QEMUIOVector qiov; BlockAIOCB *aiocb; @@ -XXX,XX +XXX,XX @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds) blk->sector = cur_sector; blk->nr_sectors = nr_sectors; - blk->iov.iov_base = blk->buf; - blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE; - qemu_iovec_init_external(&blk->qiov, &blk->iov, 1); + qemu_iovec_init_buf(&blk->qiov, blk->buf, nr_sectors * BDRV_SECTOR_SIZE); blk_mig_lock(); block_mig_state.submitted++; @@ -XXX,XX +XXX,XX @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds, blk->nr_sectors = nr_sectors; if (is_async) { - blk->iov.iov_base = blk->buf; - blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE; - qemu_iovec_init_external(&blk->qiov, &blk->iov, 1); + qemu_iovec_init_buf(&blk->qiov, blk->buf, + nr_sectors * BDRV_SECTOR_SIZE); blk->aiocb = blk_aio_preadv(bmds->blk, sector * BDRV_SECTOR_SIZE, -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Use new QEMU_IOVEC_INIT_BUF() instead of qemu_iovec_init_external( ... , 1), which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-15-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-15-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- tests/test-bdrv-drain.c | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index XXXXXXX..XXXXXXX 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -XXX,XX +XXX,XX @@ static void test_drv_cb_common(enum drain_type drain_type, bool recursive) BlockAIOCB *acb; int aio_ret; - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = NULL, - .iov_len = 0, - }; - qemu_iovec_init_external(&qiov, &iov, 1); + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, 0); blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL); bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR, @@ -XXX,XX +XXX,XX @@ static void test_iothread_common(enum drain_type drain_type, int drain_thread) AioContext *ctx_a = iothread_get_aio_context(a); AioContext *ctx_b = iothread_get_aio_context(b); - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = NULL, - .iov_len = 0, - }; - qemu_iovec_init_external(&qiov, &iov, 1); + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, 0); /* bdrv_drain_all() may only be called from the main loop thread */ if (drain_type == BDRV_DRAIN_ALL && drain_thread != 0) { @@ -XXX,XX +XXX,XX @@ static void coroutine_fn test_co_delete_by_drain(void *opaque) BlockDriverState *bs = blk_bs(blk); BDRVTestTopState *tts = bs->opaque; void *buffer = g_malloc(65536); - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = buffer, - .iov_len = 65536, - }; - - qemu_iovec_init_external(&qiov, &iov, 1); + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buffer, 65536); /* Pretend some internal write operation from parent to child. * Important: We have to read from the child, not from the parent! @@ -XXX,XX +XXX,XX @@ static void test_detach_indirect(bool by_parent_cb) BdrvChild *child_a, *child_b; BlockAIOCB *acb; - QEMUIOVector qiov; - struct iovec iov = { - .iov_base = NULL, - .iov_len = 0, - }; - qemu_iovec_init_external(&qiov, &iov, 1); + QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, 0); if (!by_parent_cb) { detach_by_driver_cb_role = child_file; -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> @iov is used only to initialize @qiov. Let's use new qemu_iovec_init_buf() instead, which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-16-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-16-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/ide/internal.h | 1 - hw/ide/atapi.c | 9 ++++----- hw/ide/core.c | 8 ++------ 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ide/internal.h +++ b/include/hw/ide/internal.h @@ -XXX,XX +XXX,XX @@ struct IDEState { int atapi_dma; /* true if dma is requested for the packet cmd */ BlockAcctCookie acct; BlockAIOCB *pio_aiocb; - struct iovec iov; QEMUIOVector qiov; QLIST_HEAD(, IDEBufferedRequest) buffered_requests; /* ATA DMA state */ diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -XXX,XX +XXX,XX @@ static void cd_read_sector_cb(void *opaque, int ret) static int cd_read_sector(IDEState *s) { + void *buf; + if (s->cd_sector_size != 2048 && s->cd_sector_size != 2352) { block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ); return -EINVAL; } - s->iov.iov_base = (s->cd_sector_size == 2352) ? - s->io_buffer + 16 : s->io_buffer; - - s->iov.iov_len = ATAPI_SECTOR_SIZE; - qemu_iovec_init_external(&s->qiov, &s->iov, 1); + buf = (s->cd_sector_size == 2352) ? s->io_buffer + 16 : s->io_buffer; + qemu_iovec_init_buf(&s->qiov, buf, ATAPI_SECTOR_SIZE); trace_cd_read_sector(s->lba); diff --git a/hw/ide/core.c b/hw/ide/core.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -XXX,XX +XXX,XX @@ static void ide_sector_read(IDEState *s) return; } - s->iov.iov_base = s->io_buffer; - s->iov.iov_len = n * BDRV_SECTOR_SIZE; - qemu_iovec_init_external(&s->qiov, &s->iov, 1); + qemu_iovec_init_buf(&s->qiov, s->io_buffer, n * BDRV_SECTOR_SIZE); block_acct_start(blk_get_stats(s->blk), &s->acct, n * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ); @@ -XXX,XX +XXX,XX @@ static void ide_sector_write(IDEState *s) return; } - s->iov.iov_base = s->io_buffer; - s->iov.iov_len = n * BDRV_SECTOR_SIZE; - qemu_iovec_init_external(&s->qiov, &s->iov, 1); + qemu_iovec_init_buf(&s->qiov, s->io_buffer, n * BDRV_SECTOR_SIZE); block_acct_start(blk_get_stats(s->blk), &s->acct, n * BDRV_SECTOR_SIZE, BLOCK_ACCT_WRITE); -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> @iov is used only to initialize @qiov. Let's use new qemu_iovec_init_buf() instead, which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-17-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-17-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/ide/internal.h | 1 - hw/ide/core.c | 11 ++++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ide/internal.h +++ b/include/hw/ide/internal.h @@ -XXX,XX +XXX,XX @@ extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT]; typedef struct IDEBufferedRequest { QLIST_ENTRY(IDEBufferedRequest) list; - struct iovec iov; QEMUIOVector qiov; QEMUIOVector *original_qiov; BlockCompletionFunc *original_cb; diff --git a/hw/ide/core.c b/hw/ide/core.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -XXX,XX +XXX,XX @@ static void ide_buffered_readv_cb(void *opaque, int ret) IDEBufferedRequest *req = opaque; if (!req->orphaned) { if (!ret) { - qemu_iovec_from_buf(req->original_qiov, 0, req->iov.iov_base, + assert(req->qiov.size == req->original_qiov->size); + qemu_iovec_from_buf(req->original_qiov, 0, + req->qiov.local_iov.iov_base, req->original_qiov->size); } req->original_cb(req->original_opaque, ret); } QLIST_REMOVE(req, list); - qemu_vfree(req->iov.iov_base); + qemu_vfree(qemu_iovec_buf(&req->qiov)); g_free(req); } @@ -XXX,XX +XXX,XX @@ BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num, req->original_qiov = iov; req->original_cb = cb; req->original_opaque = opaque; - req->iov.iov_base = qemu_blockalign(blk_bs(s->blk), iov->size); - req->iov.iov_len = iov->size; - qemu_iovec_init_external(&req->qiov, &req->iov, 1); + qemu_iovec_init_buf(&req->qiov, blk_blockalign(s->blk, iov->size), + iov->size); aioreq = blk_aio_preadv(s->blk, sector_num << BDRV_SECTOR_BITS, &req->qiov, 0, ide_buffered_readv_cb, req); -- 2.20.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> @iov is used only to initialize @qiov. Let's use new qemu_iovec_init_buf() instead, which simplifies the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20190218140926.333779-18-vsementsov@virtuozzo.com Message-Id: <20190218140926.333779-18-vsementsov@virtuozzo.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/ide/internal.h | 1 - hw/ide/atapi.c | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ide/internal.h +++ b/include/hw/ide/internal.h @@ -XXX,XX +XXX,XX @@ struct IDEDMAOps { struct IDEDMA { const struct IDEDMAOps *ops; - struct iovec iov; QEMUIOVector qiov; BlockAIOCB *aiocb; }; diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -XXX,XX +XXX,XX @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret) data_offset = 0; } trace_ide_atapi_cmd_read_dma_cb_aio(s, s->lba, n); - s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset); - s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE; - qemu_iovec_init_external(&s->bus->dma->qiov, &s->bus->dma->iov, 1); + qemu_iovec_init_buf(&s->bus->dma->qiov, s->io_buffer + data_offset, + n * ATAPI_SECTOR_SIZE); s->bus->dma->aiocb = ide_buffered_readv(s, (int64_t)s->lba << 2, &s->bus->dma->qiov, n * 4, -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> We add acct_failed param in order to use virtio_blk_handle_rw_error() also when is not required to call block_acct_failed(). (eg. a discard operation is failed) Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-2-sgarzare@redhat.com Message-Id: <20190221103314.58500-2-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/block/virtio-blk.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index XXXXXXX..XXXXXXX 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -XXX,XX +XXX,XX @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) } static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, - bool is_read) + bool is_read, bool acct_failed) { VirtIOBlock *s = req->dev; BlockErrorAction action = blk_get_error_action(s->blk, is_read, error); @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, s->rq = req; } else if (action == BLOCK_ERROR_ACTION_REPORT) { virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); - block_acct_failed(blk_get_stats(s->blk), &req->acct); + if (acct_failed) { + block_acct_failed(blk_get_stats(s->blk), &req->acct); + } virtio_blk_free_request(req); } @@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret) * the memory until the request is completed (which will * happen on the other side of the migration). */ - if (virtio_blk_handle_rw_error(req, -ret, is_read)) { + if (virtio_blk_handle_rw_error(req, -ret, is_read, true)) { continue; } } @@ -XXX,XX +XXX,XX @@ static void virtio_blk_flush_complete(void *opaque, int ret) aio_context_acquire(blk_get_aio_context(s->conf.conf.blk)); if (ret) { - if (virtio_blk_handle_rw_error(req, -ret, 0)) { + if (virtio_blk_handle_rw_error(req, -ret, 0, true)) { goto out; } } -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> Since configurable features for virtio-blk are growing, this patch adds host_features field in the struct VirtIOBlock. (as in virtio-net) In this way, we can avoid to add new fields for new properties and we can directly set VIRTIO_BLK_F* flags in the host_features. We update "config-wce" and "scsi" property definition to use the new host_features field without change the behaviour. Suggested-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-3-sgarzare@redhat.com Message-Id: <20190221103314.58500-3-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/virtio/virtio-blk.h | 3 +-- hw/block/virtio-blk.c | 16 +++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf BlockConf conf; IOThread *iothread; char *serial; - uint32_t scsi; - uint32_t config_wce; uint32_t request_merging; uint16_t num_queues; uint16_t queue_size; @@ -XXX,XX +XXX,XX @@ typedef struct VirtIOBlock { bool dataplane_disabled; bool dataplane_started; struct VirtIOBlockDataPlane *dataplane; + uint64_t host_features; } VirtIOBlock; typedef struct VirtIOBlockReq { diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index XXXXXXX..XXXXXXX 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req) */ scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base; - if (!blk->conf.scsi) { + if (!virtio_has_feature(blk->host_features, VIRTIO_BLK_F_SCSI)) { status = VIRTIO_BLK_S_UNSUPP; goto fail; } @@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, { VirtIOBlock *s = VIRTIO_BLK(vdev); + /* Firstly sync all virtio-blk possible supported features */ + features |= s->host_features; + virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX); virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY); virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY); virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE); if (virtio_has_feature(features, VIRTIO_F_VERSION_1)) { - if (s->conf.scsi) { + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_SCSI)) { error_setg(errp, "Please set scsi=off for virtio-blk devices in order to use virtio 1.0"); return 0; } @@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); } - if (s->conf.config_wce) { - virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE); - } if (blk_enable_write_cache(s->blk)) { virtio_add_feature(&features, VIRTIO_BLK_F_WCE); } @@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = { DEFINE_BLOCK_ERROR_PROPERTIES(VirtIOBlock, conf.conf), DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, conf.conf), DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial), - DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true), + DEFINE_PROP_BIT64("config-wce", VirtIOBlock, host_features, + VIRTIO_BLK_F_CONFIG_WCE, true), #ifdef __linux__ - DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false), + DEFINE_PROP_BIT64("scsi", VirtIOBlock, host_features, + VIRTIO_BLK_F_SCSI, false), #endif DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0, true), -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> In order to avoid migration issues, we enable DISCARD and WRITE_ZEROES features only for machine type >= 4.0 As discussed with Michael S. Tsirkin and Stefan Hajnoczi on the list [1], DISCARD operation should not have security implications (eg. page cache attacks), so we can enable it by default. [1] https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00504.html Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-4-sgarzare@redhat.com Message-Id: <20190221103314.58500-4-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/block/virtio-blk.c | 4 ++++ hw/core/machine.c | 2 ++ 2 files changed, 6 insertions(+) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index XXXXXXX..XXXXXXX 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = { DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128), DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD, IOThread *), + DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, + VIRTIO_BLK_F_DISCARD, true), + DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, + VIRTIO_BLK_F_WRITE_ZEROES, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/core/machine.c b/hw/core/machine.c index XXXXXXX..XXXXXXX 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -XXX,XX +XXX,XX @@ GlobalProperty hw_compat_3_1[] = { { "usb-kbd", "serial", "42" }, { "usb-mouse", "serial", "42" }, { "usb-kbd", "serial", "42" }, + { "virtio-blk-device", "discard", "false" }, + { "virtio-blk-device", "write-zeroes", "false" }, }; const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1); -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> In order to use VirtIOFeature also in other virtio devices, we move its declaration and the endof() macro (renamed in virtio_endof()) in virtio.h. We add virtio_feature_get_config_size() function to iterate the array of VirtIOFeature and to return the config size depending on the features enabled. (as virtio_net_set_config_size() did) Suggested-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-5-sgarzare@redhat.com Message-Id: <20190221103314.58500-5-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/virtio/virtio.h | 15 +++++++++++++++ hw/net/virtio-net.c | 31 +++++++------------------------ hw/virtio/virtio.c | 15 +++++++++++++++ 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -XXX,XX +XXX,XX @@ static inline hwaddr vring_align(hwaddr addr, return QEMU_ALIGN_UP(addr, align); } +/* + * Calculate the number of bytes up to and including the given 'field' of + * 'container'. + */ +#define virtio_endof(container, field) \ + (offsetof(container, field) + sizeof_field(container, field)) + +typedef struct VirtIOFeature { + uint64_t flags; + size_t end; +} VirtIOFeature; + +size_t virtio_feature_get_config_size(VirtIOFeature *features, + uint64_t host_features); + typedef struct VirtQueue VirtQueue; #define VIRTQUEUE_MAX_SIZE 1024 diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -XXX,XX +XXX,XX @@ static inline __virtio16 *virtio_net_rsc_ext_num_dupacks( #endif -/* - * Calculate the number of bytes up to and including the given 'field' of - * 'container'. - */ -#define endof(container, field) \ - (offsetof(container, field) + sizeof_field(container, field)) - -typedef struct VirtIOFeature { - uint64_t flags; - size_t end; -} VirtIOFeature; - static VirtIOFeature feature_sizes[] = { {.flags = 1ULL << VIRTIO_NET_F_MAC, - .end = endof(struct virtio_net_config, mac)}, + .end = virtio_endof(struct virtio_net_config, mac)}, {.flags = 1ULL << VIRTIO_NET_F_STATUS, - .end = endof(struct virtio_net_config, status)}, + .end = virtio_endof(struct virtio_net_config, status)}, {.flags = 1ULL << VIRTIO_NET_F_MQ, - .end = endof(struct virtio_net_config, max_virtqueue_pairs)}, + .end = virtio_endof(struct virtio_net_config, max_virtqueue_pairs)}, {.flags = 1ULL << VIRTIO_NET_F_MTU, - .end = endof(struct virtio_net_config, mtu)}, + .end = virtio_endof(struct virtio_net_config, mtu)}, {.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX, - .end = endof(struct virtio_net_config, duplex)}, + .end = virtio_endof(struct virtio_net_config, duplex)}, {} }; @@ -XXX,XX +XXX,XX @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features) { - int i, config_size = 0; virtio_add_feature(&host_features, VIRTIO_NET_F_MAC); - for (i = 0; feature_sizes[i].flags != 0; i++) { - if (host_features & feature_sizes[i].flags) { - config_size = MAX(feature_sizes[i].end, config_size); - } - } - n->config_size = config_size; + n->config_size = virtio_feature_get_config_size(feature_sizes, + host_features); } void virtio_net_set_netclient_name(VirtIONet *n, const char *name, diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -XXX,XX +XXX,XX @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) return ret; } +size_t virtio_feature_get_config_size(VirtIOFeature *feature_sizes, + uint64_t host_features) +{ + size_t config_size = 0; + int i; + + for (i = 0; feature_sizes[i].flags != 0; i++) { + if (host_features & feature_sizes[i].flags) { + config_size = MAX(feature_sizes[i].end, config_size); + } + } + + return config_size; +} + int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) { int i, ret; -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> Starting from DISABLE and WRITE_ZEROES features, we use an array of VirtIOFeature (as virtio-net) to properly set the config size depending on the features enabled. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-6-sgarzare@redhat.com Message-Id: <20190221103314.58500-6-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/virtio/virtio-blk.h | 1 + hw/block/virtio-blk.c | 31 +++++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -XXX,XX +XXX,XX @@ typedef struct VirtIOBlock { bool dataplane_started; struct VirtIOBlockDataPlane *dataplane; uint64_t host_features; + size_t config_size; } VirtIOBlock; typedef struct VirtIOBlockReq { diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index XXXXXXX..XXXXXXX 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -XXX,XX +XXX,XX @@ #include "hw/virtio/virtio-bus.h" #include "hw/virtio/virtio-access.h" -/* We don't support discard yet, hide associated config fields. */ +/* Config size before the discard support (hide associated config fields) */ #define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \ max_discard_sectors) +/* + * Starting from the discard feature, we can use this array to properly + * set the config size depending on the features enabled. + */ +static VirtIOFeature feature_sizes[] = { + {.flags = 1ULL << VIRTIO_BLK_F_DISCARD, + .end = virtio_endof(struct virtio_blk_config, discard_sector_alignment)}, + {.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES, + .end = virtio_endof(struct virtio_blk_config, write_zeroes_may_unmap)}, + {} +}; + +static void virtio_blk_set_config_size(VirtIOBlock *s, uint64_t host_features) +{ + s->config_size = MAX(VIRTIO_BLK_CFG_SIZE, + virtio_feature_get_config_size(feature_sizes, host_features)); + + assert(s->config_size <= sizeof(struct virtio_blk_config)); +} static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, VirtIOBlockReq *req) @@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) blkcfg.alignment_offset = 0; blkcfg.wce = blk_enable_write_cache(s->blk); virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); - memcpy(config, &blkcfg, VIRTIO_BLK_CFG_SIZE); - QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE > sizeof(blkcfg)); + memcpy(config, &blkcfg, s->config_size); } static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config) @@ -XXX,XX +XXX,XX @@ static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config) VirtIOBlock *s = VIRTIO_BLK(vdev); struct virtio_blk_config blkcfg; - memcpy(&blkcfg, config, VIRTIO_BLK_CFG_SIZE); - QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE > sizeof(blkcfg)); + memcpy(&blkcfg, config, s->config_size); aio_context_acquire(blk_get_aio_context(s->blk)); blk_set_enable_write_cache(s->blk, blkcfg.wce != 0); @@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) return; } - virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE); + virtio_blk_set_config_size(s, s->host_features); + + virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, s->config_size); s->blk = conf->conf.blk; s->rq = NULL; -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> This patch adds the support of DISCARD and WRITE_ZEROES commands, that have been introduced in the virtio-blk protocol to have better performance when using SSD backend. We support only one segment per request since multiple segments are not widely used and there are no userspace APIs that allow applications to submit multiple segments in a single call. Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-7-sgarzare@redhat.com Message-Id: <20190221103314.58500-7-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/virtio/virtio-blk.h | 2 + hw/block/virtio-blk.c | 184 +++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+) diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf uint32_t request_merging; uint16_t num_queues; uint16_t queue_size; + uint32_t max_discard_sectors; + uint32_t max_write_zeroes_sectors; }; struct VirtIOBlockDataPlane; diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index XXXXXXX..XXXXXXX 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -XXX,XX +XXX,XX @@ out: aio_context_release(blk_get_aio_context(s->conf.conf.blk)); } +static void virtio_blk_discard_write_zeroes_complete(void *opaque, int ret) +{ + VirtIOBlockReq *req = opaque; + VirtIOBlock *s = req->dev; + bool is_write_zeroes = (virtio_ldl_p(VIRTIO_DEVICE(s), &req->out.type) & + ~VIRTIO_BLK_T_BARRIER) == VIRTIO_BLK_T_WRITE_ZEROES; + + aio_context_acquire(blk_get_aio_context(s->conf.conf.blk)); + if (ret) { + if (virtio_blk_handle_rw_error(req, -ret, false, is_write_zeroes)) { + goto out; + } + } + + virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); + if (is_write_zeroes) { + block_acct_done(blk_get_stats(s->blk), &req->acct); + } + virtio_blk_free_request(req); + +out: + aio_context_release(blk_get_aio_context(s->conf.conf.blk)); +} + #ifdef __linux__ typedef struct { @@ -XXX,XX +XXX,XX @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev, return true; } +static uint8_t virtio_blk_handle_discard_write_zeroes(VirtIOBlockReq *req, + struct virtio_blk_discard_write_zeroes *dwz_hdr, bool is_write_zeroes) +{ + VirtIOBlock *s = req->dev; + VirtIODevice *vdev = VIRTIO_DEVICE(s); + uint64_t sector; + uint32_t num_sectors, flags, max_sectors; + uint8_t err_status; + int bytes; + + sector = virtio_ldq_p(vdev, &dwz_hdr->sector); + num_sectors = virtio_ldl_p(vdev, &dwz_hdr->num_sectors); + flags = virtio_ldl_p(vdev, &dwz_hdr->flags); + max_sectors = is_write_zeroes ? s->conf.max_write_zeroes_sectors : + s->conf.max_discard_sectors; + + /* + * max_sectors is at most BDRV_REQUEST_MAX_SECTORS, this check + * make us sure that "num_sectors << BDRV_SECTOR_BITS" can fit in + * the integer variable. + */ + if (unlikely(num_sectors > max_sectors)) { + err_status = VIRTIO_BLK_S_IOERR; + goto err; + } + + bytes = num_sectors << BDRV_SECTOR_BITS; + + if (unlikely(!virtio_blk_sect_range_ok(s, sector, bytes))) { + err_status = VIRTIO_BLK_S_IOERR; + goto err; + } + + /* + * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for discard + * and write zeroes commands if any unknown flag is set. + */ + if (unlikely(flags & ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) { + err_status = VIRTIO_BLK_S_UNSUPP; + goto err; + } + + if (is_write_zeroes) { /* VIRTIO_BLK_T_WRITE_ZEROES */ + int blk_aio_flags = 0; + + if (flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) { + blk_aio_flags |= BDRV_REQ_MAY_UNMAP; + } + + block_acct_start(blk_get_stats(s->blk), &req->acct, bytes, + BLOCK_ACCT_WRITE); + + blk_aio_pwrite_zeroes(s->blk, sector << BDRV_SECTOR_BITS, + bytes, blk_aio_flags, + virtio_blk_discard_write_zeroes_complete, req); + } else { /* VIRTIO_BLK_T_DISCARD */ + /* + * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for + * discard commands if the unmap flag is set. + */ + if (unlikely(flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) { + err_status = VIRTIO_BLK_S_UNSUPP; + goto err; + } + + blk_aio_pdiscard(s->blk, sector << BDRV_SECTOR_BITS, bytes, + virtio_blk_discard_write_zeroes_complete, req); + } + + return VIRTIO_BLK_S_OK; + +err: + if (is_write_zeroes) { + block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE); + } + return err_status; +} + static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) virtio_blk_free_request(req); break; } + /* + * VIRTIO_BLK_T_DISCARD and VIRTIO_BLK_T_WRITE_ZEROES are defined with + * VIRTIO_BLK_T_OUT flag set. We masked this flag in the switch statement, + * so we must mask it for these requests, then we will check if it is set. + */ + case VIRTIO_BLK_T_DISCARD & ~VIRTIO_BLK_T_OUT: + case VIRTIO_BLK_T_WRITE_ZEROES & ~VIRTIO_BLK_T_OUT: + { + struct virtio_blk_discard_write_zeroes dwz_hdr; + size_t out_len = iov_size(out_iov, out_num); + bool is_write_zeroes = (type & ~VIRTIO_BLK_T_BARRIER) == + VIRTIO_BLK_T_WRITE_ZEROES; + uint8_t err_status; + + /* + * Unsupported if VIRTIO_BLK_T_OUT is not set or the request contains + * more than one segment. + */ + if (unlikely(!(type & VIRTIO_BLK_T_OUT) || + out_len > sizeof(dwz_hdr))) { + virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); + virtio_blk_free_request(req); + return 0; + } + + if (unlikely(iov_to_buf(out_iov, out_num, 0, &dwz_hdr, + sizeof(dwz_hdr)) != sizeof(dwz_hdr))) { + virtio_error(vdev, "virtio-blk discard/write_zeroes header" + " too short"); + return -1; + } + + err_status = virtio_blk_handle_discard_write_zeroes(req, &dwz_hdr, + is_write_zeroes); + if (err_status != VIRTIO_BLK_S_OK) { + virtio_blk_req_complete(req, err_status); + virtio_blk_free_request(req); + } + + break; + } default: virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); virtio_blk_free_request(req); @@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) blkcfg.alignment_offset = 0; blkcfg.wce = blk_enable_write_cache(s->blk); virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) { + virtio_stl_p(vdev, &blkcfg.max_discard_sectors, + s->conf.max_discard_sectors); + virtio_stl_p(vdev, &blkcfg.discard_sector_alignment, + blk_size >> BDRV_SECTOR_BITS); + /* + * We support only one segment per request since multiple segments + * are not widely used and there are no userspace APIs that allow + * applications to submit multiple segments in a single call. + */ + virtio_stl_p(vdev, &blkcfg.max_discard_seg, 1); + } + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_WRITE_ZEROES)) { + virtio_stl_p(vdev, &blkcfg.max_write_zeroes_sectors, + s->conf.max_write_zeroes_sectors); + blkcfg.write_zeroes_may_unmap = 1; + virtio_stl_p(vdev, &blkcfg.max_write_zeroes_seg, 1); + } memcpy(config, &blkcfg, s->config_size); } @@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) return; } + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD) && + (!conf->max_discard_sectors || + conf->max_discard_sectors > BDRV_REQUEST_MAX_SECTORS)) { + error_setg(errp, "invalid max-discard-sectors property (%" PRIu32 ")" + ", must be between 1 and %d", + conf->max_discard_sectors, (int)BDRV_REQUEST_MAX_SECTORS); + return; + } + + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_WRITE_ZEROES) && + (!conf->max_write_zeroes_sectors || + conf->max_write_zeroes_sectors > BDRV_REQUEST_MAX_SECTORS)) { + error_setg(errp, "invalid max-write-zeroes-sectors property (%" PRIu32 + "), must be between 1 and %d", + conf->max_write_zeroes_sectors, + (int)BDRV_REQUEST_MAX_SECTORS); + return; + } + virtio_blk_set_config_size(s, s->host_features); virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, s->config_size); @@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = { VIRTIO_BLK_F_DISCARD, true), DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, VIRTIO_BLK_F_WRITE_ZEROES, true), + DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock, + conf.max_discard_sectors, BDRV_REQUEST_MAX_SECTORS), + DEFINE_PROP_UINT32("max-write-zeroes-sectors", VirtIOBlock, + conf.max_write_zeroes_sectors, BDRV_REQUEST_MAX_SECTORS), DEFINE_PROP_END_OF_LIST(), }; -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> The size of data in the virtio_blk_request must be a multiple of 512 bytes for IN and OUT requests, or a multiple of the size of struct virtio_blk_discard_write_zeroes for DISCARD and WRITE_ZEROES requests. Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-8-sgarzare@redhat.com Message-Id: <20190221103314.58500-8-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- tests/virtio-blk-test.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/virtio-blk-test.c +++ b/tests/virtio-blk-test.c @@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d, uint64_t addr; uint8_t status = 0xFF; - g_assert_cmpuint(data_size % 512, ==, 0); + switch (req->type) { + case VIRTIO_BLK_T_IN: + case VIRTIO_BLK_T_OUT: + g_assert_cmpuint(data_size % 512, ==, 0); + break; + case VIRTIO_BLK_T_DISCARD: + case VIRTIO_BLK_T_WRITE_ZEROES: + g_assert_cmpuint(data_size % + sizeof(struct virtio_blk_discard_write_zeroes), ==, 0); + break; + default: + g_assert_cmpuint(data_size, ==, 0); + } + addr = guest_alloc(alloc, sizeof(*req) + data_size); virtio_blk_fix_request(d, req); -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> This function is useful to fix the endianness of struct virtio_blk_discard_write_zeroes headers. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-9-sgarzare@redhat.com Message-Id: <20190221103314.58500-9-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- tests/virtio-blk-test.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/virtio-blk-test.c +++ b/tests/virtio-blk-test.c @@ -XXX,XX +XXX,XX @@ typedef struct QVirtioBlkReq { uint8_t status; } QVirtioBlkReq; +#ifdef HOST_WORDS_BIGENDIAN +const bool host_is_big_endian = true; +#else +const bool host_is_big_endian; /* false */ +#endif + static char *drive_create(void) { int fd, ret; @@ -XXX,XX +XXX,XX @@ static QVirtioPCIDevice *virtio_blk_pci_init(QPCIBus *bus, int slot) static inline void virtio_blk_fix_request(QVirtioDevice *d, QVirtioBlkReq *req) { -#ifdef HOST_WORDS_BIGENDIAN - const bool host_is_big_endian = true; -#else - const bool host_is_big_endian = false; -#endif - if (qvirtio_is_big_endian(d) != host_is_big_endian) { req->type = bswap32(req->type); req->ioprio = bswap32(req->ioprio); @@ -XXX,XX +XXX,XX @@ static inline void virtio_blk_fix_request(QVirtioDevice *d, QVirtioBlkReq *req) } } + +static inline void virtio_blk_fix_dwz_hdr(QVirtioDevice *d, + struct virtio_blk_discard_write_zeroes *dwz_hdr) +{ + if (qvirtio_is_big_endian(d) != host_is_big_endian) { + dwz_hdr->sector = bswap64(dwz_hdr->sector); + dwz_hdr->num_sectors = bswap32(dwz_hdr->num_sectors); + dwz_hdr->flags = bswap32(dwz_hdr->flags); + } +} + static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d, QVirtioBlkReq *req, uint64_t data_size) { -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> If the WRITE_ZEROES feature is enabled, we check this command in the test_basic(). Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-10-sgarzare@redhat.com Message-Id: <20190221103314.58500-10-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- tests/virtio-blk-test.c | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/virtio-blk-test.c +++ b/tests/virtio-blk-test.c @@ -XXX,XX +XXX,XX @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc, guest_free(alloc, req_addr); + if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) { + struct virtio_blk_discard_write_zeroes dwz_hdr; + void *expected; + + /* + * WRITE_ZEROES request on the same sector of previous test where + * we wrote "TEST". + */ + req.type = VIRTIO_BLK_T_WRITE_ZEROES; + req.data = (char *) &dwz_hdr; + dwz_hdr.sector = 0; + dwz_hdr.num_sectors = 1; + dwz_hdr.flags = 0; + + virtio_blk_fix_dwz_hdr(dev, &dwz_hdr); + + req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr)); + + free_head = qvirtqueue_add(vq, req_addr, 16, false, true); + qvirtqueue_add(vq, req_addr + 16, sizeof(dwz_hdr), false, true); + qvirtqueue_add(vq, req_addr + 16 + sizeof(dwz_hdr), 1, true, false); + + qvirtqueue_kick(dev, vq, free_head); + + qvirtio_wait_used_elem(dev, vq, free_head, NULL, + QVIRTIO_BLK_TIMEOUT_US); + status = readb(req_addr + 16 + sizeof(dwz_hdr)); + g_assert_cmpint(status, ==, 0); + + guest_free(alloc, req_addr); + + /* Read request to check if the sector contains all zeroes */ + req.type = VIRTIO_BLK_T_IN; + req.ioprio = 1; + req.sector = 0; + req.data = g_malloc0(512); + + req_addr = virtio_blk_request(alloc, dev, &req, 512); + + g_free(req.data); + + free_head = qvirtqueue_add(vq, req_addr, 16, false, true); + qvirtqueue_add(vq, req_addr + 16, 512, true, true); + qvirtqueue_add(vq, req_addr + 528, 1, true, false); + + qvirtqueue_kick(dev, vq, free_head); + + qvirtio_wait_used_elem(dev, vq, free_head, NULL, + QVIRTIO_BLK_TIMEOUT_US); + status = readb(req_addr + 528); + g_assert_cmpint(status, ==, 0); + + data = g_malloc(512); + expected = g_malloc0(512); + memread(req_addr + 16, data, 512); + g_assert_cmpmem(data, 512, expected, 512); + g_free(expected); + g_free(data); + + guest_free(alloc, req_addr); + } + if (features & (1u << VIRTIO_F_ANY_LAYOUT)) { /* Write and read with 2 descriptor layout */ /* Write request */ -- 2.20.1 From: Stefano Garzarella <sgarzare@redhat.com> If the DISCARD feature is enabled, we try this command in the test_basic(), checking only the status returned by the request. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20190221103314.58500-11-sgarzare@redhat.com Message-Id: <20190221103314.58500-11-sgarzare@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- tests/virtio-blk-test.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/virtio-blk-test.c +++ b/tests/virtio-blk-test.c @@ -XXX,XX +XXX,XX @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc, guest_free(alloc, req_addr); } + if (features & (1u << VIRTIO_BLK_F_DISCARD)) { + struct virtio_blk_discard_write_zeroes dwz_hdr; + + req.type = VIRTIO_BLK_T_DISCARD; + req.data = (char *) &dwz_hdr; + dwz_hdr.sector = 0; + dwz_hdr.num_sectors = 1; + dwz_hdr.flags = 0; + + virtio_blk_fix_dwz_hdr(dev, &dwz_hdr); + + req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr)); + + free_head = qvirtqueue_add(vq, req_addr, 16, false, true); + qvirtqueue_add(vq, req_addr + 16, sizeof(dwz_hdr), false, true); + qvirtqueue_add(vq, req_addr + 16 + sizeof(dwz_hdr), 1, true, false); + + qvirtqueue_kick(dev, vq, free_head); + + qvirtio_wait_used_elem(dev, vq, free_head, NULL, + QVIRTIO_BLK_TIMEOUT_US); + status = readb(req_addr + 16 + sizeof(dwz_hdr)); + g_assert_cmpint(status, ==, 0); + + guest_free(alloc, req_addr); + } + if (features & (1u << VIRTIO_F_ANY_LAYOUT)) { /* Write and read with 2 descriptor layout */ /* Write request */ -- 2.20.1 The following changes since commit 9d662a6b22a0838a85c5432385f35db2488a33a5: Merge remote-tracking branch 'remotes/legoater/tags/pull-ppc-20220305' into staging (2022-03-05 18:03:15 +0000) are available in the Git repository at: https://gitlab.com/hreitz/qemu.git tags/pull-block-2022-03-07 for you to fetch changes up to 743da0b401cdc3ee94bc519975e339a3cdbe0ad1: iotests/image-fleecing: test push backup with fleecing (2022-03-07 09:33:31 +0100) ---------------------------------------------------------------- Block patches for 7.0-rc0: - New fleecing backup scheme - iotest fixes - Fixes for the curl block driver - Fix for the preallocate block driver - IDE fix for zero-length TRIM requests ---------------------------------------------------------------- Hanna Reitz (2): ide: Increment BB in-flight counter for TRIM BH iotests: Write test output to TEST_DIR Peter Maydell (2): block/curl.c: Set error message string if curl_init_state() fails block/curl.c: Check error return from curl_easy_setopt() Thomas Huth (2): tests/qemu-iotests/040: Skip TestCommitWithFilters without 'throttle' tests/qemu-iotests/testrunner: Quote "case not run" lines in TAP mode Vladimir Sementsov-Ogievskiy (17): block: fix preallocate filter: don't do unaligned preallocate requests block/block-copy: move copy_bitmap initialization to block_copy_state_new() block/dirty-bitmap: bdrv_merge_dirty_bitmap(): add return value block/block-copy: block_copy_state_new(): add bitmap parameter block/copy-before-write: add bitmap open parameter block/block-copy: add block_copy_reset() block: intoduce reqlist block/reqlist: reqlist_find_conflict(): use ranges_overlap() block/dirty-bitmap: introduce bdrv_dirty_bitmap_status() block/reqlist: add reqlist_wait_all() block/io: introduce block driver snapshot-access API block: introduce snapshot-access block driver block: copy-before-write: realize snapshot-access API iotests/image-fleecing: add test-case for fleecing format node iotests.py: add qemu_io_pipe_and_status() iotests/image-fleecing: add test case with bitmap iotests/image-fleecing: test push backup with fleecing qapi/block-core.json | 14 +- include/block/block-common.h | 3 +- include/block/block-copy.h | 2 + include/block/block_int-common.h | 24 ++ include/block/block_int-io.h | 9 + include/block/dirty-bitmap.h | 4 +- include/block/reqlist.h | 75 ++++++ include/qemu/hbitmap.h | 12 + block/block-copy.c | 150 +++++------ block/copy-before-write.c | 265 +++++++++++++++++++- block/curl.c | 92 ++++--- block/dirty-bitmap.c | 15 +- block/io.c | 76 ++++++ block/monitor/bitmap-qmp-cmds.c | 5 +- block/preallocate.c | 15 +- block/reqlist.c | 85 +++++++ block/snapshot-access.c | 132 ++++++++++ hw/ide/core.c | 7 + util/hbitmap.c | 33 +++ MAINTAINERS | 5 +- block/meson.build | 2 + tests/qemu-iotests/040 | 1 + tests/qemu-iotests/257.out | 224 +++++++++++++++++ tests/qemu-iotests/common.rc | 6 +- tests/qemu-iotests/iotests.py | 8 +- tests/qemu-iotests/testenv.py | 5 +- tests/qemu-iotests/testrunner.py | 19 +- tests/qemu-iotests/tests/image-fleecing | 185 +++++++++++--- tests/qemu-iotests/tests/image-fleecing.out | 221 +++++++++++++++- 29 files changed, 1499 insertions(+), 195 deletions(-) create mode 100644 include/block/reqlist.h create mode 100644 block/reqlist.c create mode 100644 block/snapshot-access.c -- 2.34.1 When we still have an AIOCB registered for DMA operations, we try to settle the respective operation by draining the BlockBackend associated with the IDE device. However, this assumes that every DMA operation is associated with an increment of the BlockBackend’s in-flight counter (e.g. through some ongoing I/O operation), so that draining the BB until its in-flight counter reaches 0 will settle all DMA operations. That is not the case: For TRIM, the guest can issue a zero-length operation that will not result in any I/O operation forwarded to the BlockBackend, and also not increment the in-flight counter in any other way. In such a case, blk_drain() will be a no-op if no other operations are in flight. It is clear that if blk_drain() is a no-op, the value of s->bus->dma->aiocb will not change between checking it in the `if` condition and asserting that it is NULL after blk_drain(). The particular problem is that ide_issue_trim() creates a BH (ide_trim_bh_cb()) to settle the TRIM request: iocb->common.cb() is ide_dma_cb(), which will either create a new request, or find the transfer to be done and call ide_set_inactive(), which clears s->bus->dma->aiocb. Therefore, the blk_drain() must wait for ide_trim_bh_cb() to run, which currently it will not always do. To fix this issue, we increment the BlockBackend's in-flight counter when the TRIM operation begins (in ide_issue_trim(), when the ide_trim_bh_cb() BH is created) and decrement it when ide_trim_bh_cb() is done. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2029980 Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220120142259.120189-1-hreitz@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Tested-by: John Snow <jsnow@redhat.com> --- hw/ide/core.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/ide/core.c b/hw/ide/core.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -XXX,XX +XXX,XX @@ static const AIOCBInfo trim_aiocb_info = { static void ide_trim_bh_cb(void *opaque) { TrimAIOCB *iocb = opaque; + BlockBackend *blk = iocb->s->blk; iocb->common.cb(iocb->common.opaque, iocb->ret); qemu_bh_delete(iocb->bh); iocb->bh = NULL; qemu_aio_unref(iocb); + + /* Paired with an increment in ide_issue_trim() */ + blk_dec_in_flight(blk); } static void ide_issue_trim_cb(void *opaque, int ret) @@ -XXX,XX +XXX,XX @@ BlockAIOCB *ide_issue_trim( IDEState *s = opaque; TrimAIOCB *iocb; + /* Paired with a decrement in ide_trim_bh_cb() */ + blk_inc_in_flight(s->blk); + iocb = blk_aio_get(&trim_aiocb_info, s->blk, cb, cb_opaque); iocb->s = s; iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb); -- 2.34.1 From: Peter Maydell <peter.maydell@linaro.org> In curl_open(), the 'out' label assumes that the state->errmsg string has been set (either by curl_easy_perform() or by manually copying a string into it); however if curl_init_state() fails we will jump to that label without setting the string. Add the missing error string setup. (We can't be specific about the cause of failure: the documentation of curl_easy_init() just says "If this function returns NULL, something went wrong".) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20220222152341.850419-2-peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- block/curl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/curl.c b/block/curl.c index XXXXXXX..XXXXXXX 100644 --- a/block/curl.c +++ b/block/curl.c @@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, // Get file size if (curl_init_state(s, state) < 0) { + pstrcpy(state->errmsg, CURL_ERROR_SIZE, + "curl library initialization failed."); goto out; } -- 2.34.1 From: Peter Maydell <peter.maydell@linaro.org> Coverity points out that we aren't checking the return value from curl_easy_setopt() for any of the calls to it we make in block/curl.c. Some of these options are documented as always succeeding (e.g. CURLOPT_VERBOSE) but others have documented failure cases (e.g. CURLOPT_URL). For consistency we check every call, even the ones that theoretically cannot fail. Fixes: Coverity CID 1459336, 1459482, 1460331 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20220222152341.850419-3-peter.maydell@linaro.org> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- block/curl.c | 90 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 33 deletions(-) diff --git a/block/curl.c b/block/curl.c index XXXXXXX..XXXXXXX 100644 --- a/block/curl.c +++ b/block/curl.c @@ -XXX,XX +XXX,XX @@ static int curl_init_state(BDRVCURLState *s, CURLState *state) if (!state->curl) { return -EIO; } - curl_easy_setopt(state->curl, CURLOPT_URL, s->url); - curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER, - (long) s->sslverify); - curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYHOST, - s->sslverify ? 2L : 0L); + if (curl_easy_setopt(state->curl, CURLOPT_URL, s->url) || + curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER, + (long) s->sslverify) || + curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYHOST, + s->sslverify ? 2L : 0L)) { + goto err; + } if (s->cookie) { - curl_easy_setopt(state->curl, CURLOPT_COOKIE, s->cookie); + if (curl_easy_setopt(state->curl, CURLOPT_COOKIE, s->cookie)) { + goto err; + } + } + if (curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, (long)s->timeout) || + curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, + (void *)curl_read_cb) || + curl_easy_setopt(state->curl, CURLOPT_WRITEDATA, (void *)state) || + curl_easy_setopt(state->curl, CURLOPT_PRIVATE, (void *)state) || + curl_easy_setopt(state->curl, CURLOPT_AUTOREFERER, 1) || + curl_easy_setopt(state->curl, CURLOPT_FOLLOWLOCATION, 1) || + curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1) || + curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg) || + curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1)) { + goto err; } - curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, (long)s->timeout); - curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, - (void *)curl_read_cb); - curl_easy_setopt(state->curl, CURLOPT_WRITEDATA, (void *)state); - curl_easy_setopt(state->curl, CURLOPT_PRIVATE, (void *)state); - curl_easy_setopt(state->curl, CURLOPT_AUTOREFERER, 1); - curl_easy_setopt(state->curl, CURLOPT_FOLLOWLOCATION, 1); - curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1); - curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg); - curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1); - if (s->username) { - curl_easy_setopt(state->curl, CURLOPT_USERNAME, s->username); + if (curl_easy_setopt(state->curl, CURLOPT_USERNAME, s->username)) { + goto err; + } } if (s->password) { - curl_easy_setopt(state->curl, CURLOPT_PASSWORD, s->password); + if (curl_easy_setopt(state->curl, CURLOPT_PASSWORD, s->password)) { + goto err; + } } if (s->proxyusername) { - curl_easy_setopt(state->curl, - CURLOPT_PROXYUSERNAME, s->proxyusername); + if (curl_easy_setopt(state->curl, + CURLOPT_PROXYUSERNAME, s->proxyusername)) { + goto err; + } } if (s->proxypassword) { - curl_easy_setopt(state->curl, - CURLOPT_PROXYPASSWORD, s->proxypassword); + if (curl_easy_setopt(state->curl, + CURLOPT_PROXYPASSWORD, s->proxypassword)) { + goto err; + } } /* Restrict supported protocols to avoid security issues in the more @@ -XXX,XX +XXX,XX @@ static int curl_init_state(BDRVCURLState *s, CURLState *state) * Restricting protocols is only supported from 7.19.4 upwards. */ #if LIBCURL_VERSION_NUM >= 0x071304 - curl_easy_setopt(state->curl, CURLOPT_PROTOCOLS, PROTOCOLS); - curl_easy_setopt(state->curl, CURLOPT_REDIR_PROTOCOLS, PROTOCOLS); + if (curl_easy_setopt(state->curl, CURLOPT_PROTOCOLS, PROTOCOLS) || + curl_easy_setopt(state->curl, CURLOPT_REDIR_PROTOCOLS, PROTOCOLS)) { + goto err; + } #endif #ifdef DEBUG_VERBOSE - curl_easy_setopt(state->curl, CURLOPT_VERBOSE, 1); + if (curl_easy_setopt(state->curl, CURLOPT_VERBOSE, 1)) { + goto err; + } #endif } state->s = s; return 0; + +err: + curl_easy_cleanup(state->curl); + state->curl = NULL; + return -EIO; } /* Called with s->mutex held. */ @@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, } s->accept_range = false; - curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1); - curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION, - curl_header_cb); - curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s); + if (curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1) || + curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION, curl_header_cb) || + curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s)) { + pstrcpy(state->errmsg, CURL_ERROR_SIZE, + "curl library initialization failed."); + goto out; + } if (curl_easy_perform(state->curl)) goto out; if (curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d)) { @@ -XXX,XX +XXX,XX @@ static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb) snprintf(state->range, 127, "%" PRIu64 "-%" PRIu64, start, end); trace_curl_setup_preadv(acb->bytes, start, state->range); - curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range); - - if (curl_multi_add_handle(s->multi, state->curl) != CURLM_OK) { + if (curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range) || + curl_multi_add_handle(s->multi, state->curl) != CURLM_OK) { state->acb[0] = NULL; acb->ret = -EIO; -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> There is a bug in handling BDRV_REQ_NO_WAIT flag: we still may wait in wait_serialising_requests() if request is unaligned. And this is possible for the only user of this flag (preallocate filter) if underlying file is unaligned to its request_alignment on start. So, we have to fix preallocate filter to do only aligned preallocate requests. Next, we should fix generic block/io.c somehow. Keeping in mind that preallocate is the only user of BDRV_REQ_NO_WAIT and that we have to fix its behavior now, it seems more safe to just assert that we never use BDRV_REQ_NO_WAIT with unaligned requests and add corresponding comment. Let's do so. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Message-Id: <20220215121609.38570-1-vsementsov@virtuozzo.com> [hreitz: Rebased on block GS/IO split] Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/block-common.h | 3 ++- block/io.c | 4 ++++ block/preallocate.c | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/include/block/block-common.h b/include/block/block-common.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block-common.h +++ b/include/block/block-common.h @@ -XXX,XX +XXX,XX @@ typedef enum { /* * If we need to wait for other requests, just fail immediately. Used - * only together with BDRV_REQ_SERIALISING. + * only together with BDRV_REQ_SERIALISING. Used only with requests aligned + * to request_alignment (corresponding assertions are in block/io.c). */ BDRV_REQ_NO_WAIT = 0x400, diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child, padding = bdrv_init_padding(bs, offset, bytes, &pad); if (padding) { + assert(!(flags & BDRV_REQ_NO_WAIT)); bdrv_make_request_serialising(req, align); bdrv_padding_rmw_read(child, req, &pad, true); @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child, * serialize the request to prevent interactions of the * widened region with other transactions. */ + assert(!(flags & BDRV_REQ_NO_WAIT)); bdrv_make_request_serialising(&req, align); bdrv_padding_rmw_read(child, &req, &pad, false); } @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal( /* TODO We can support BDRV_REQ_NO_FALLBACK here */ assert(!(read_flags & BDRV_REQ_NO_FALLBACK)); assert(!(write_flags & BDRV_REQ_NO_FALLBACK)); + assert(!(read_flags & BDRV_REQ_NO_WAIT)); + assert(!(write_flags & BDRV_REQ_NO_WAIT)); if (!dst || !dst->bs || !bdrv_is_inserted(dst->bs)) { return -ENOMEDIUM; diff --git a/block/preallocate.c b/block/preallocate.c index XXXXXXX..XXXXXXX 100644 --- a/block/preallocate.c +++ b/block/preallocate.c @@ -XXX,XX +XXX,XX @@ static bool coroutine_fn handle_write(BlockDriverState *bs, int64_t offset, int64_t end = offset + bytes; int64_t prealloc_start, prealloc_end; int ret; + uint32_t file_align = bs->file->bs->bl.request_alignment; + uint32_t prealloc_align = MAX(s->opts.prealloc_align, file_align); + + assert(QEMU_IS_ALIGNED(prealloc_align, file_align)); if (!has_prealloc_perms(bs)) { /* We don't have state neither should try to recover it */ @@ -XXX,XX +XXX,XX @@ static bool coroutine_fn handle_write(BlockDriverState *bs, int64_t offset, /* Now we want new preallocation, as request writes beyond s->file_end. */ - prealloc_start = want_merge_zero ? MIN(offset, s->file_end) : s->file_end; - prealloc_end = QEMU_ALIGN_UP(end + s->opts.prealloc_size, - s->opts.prealloc_align); + prealloc_start = QEMU_ALIGN_UP( + want_merge_zero ? MIN(offset, s->file_end) : s->file_end, + file_align); + prealloc_end = QEMU_ALIGN_UP( + MAX(prealloc_start, end) + s->opts.prealloc_size, + prealloc_align); + + want_merge_zero = want_merge_zero && (prealloc_start <= offset); ret = bdrv_co_pwrite_zeroes( bs->file, prealloc_start, prealloc_end - prealloc_start, -- 2.34.1 From: Thomas Huth <thuth@redhat.com> iotest 040 already has some checks for the availability of the 'throttle' driver, but some new code has been added in the course of time that depends on 'throttle' but does not check for its availability. Add a check to the TestCommitWithFilters class so that this iotest now also passes again if 'throttle' has not been enabled in the QEMU binaries. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20220223123127.3206042-1-thuth@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- tests/qemu-iotests/040 | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -XXX,XX +XXX,XX @@ class TestCommitWithFilters(iotests.QMPTestCase): pattern_file) self.assertFalse('Pattern verification failed' in result) + @iotests.skip_if_unsupported(['throttle']) def setUp(self): qemu_img('create', '-f', iotests.imgfmt, self.img0, '64M') qemu_img('create', '-f', iotests.imgfmt, self.img1, '64M') -- 2.34.1 From: Thomas Huth <thuth@redhat.com> In TAP mode, the stdout is reserved for the TAP protocol, so we have to make sure to mark other lines with a comment '#' character at the beginning to avoid that the TAP parser at the other end gets confused. To test this condition, run "configure" for example with: --block-drv-rw-whitelist=copy-before-write,qcow2,raw,file,host_device,blkdebug,null-co,copy-on-read so that iotest 041 will report that some tests are not run due to the missing "quorum" driver. Without this change, "make check-block" fails since the meson tap parser gets confused by these messages. Signed-off-by: Thomas Huth <thuth@redhat.com> Message-Id: <20220223124353.3273898-1-thuth@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- tests/qemu-iotests/testrunner.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -XXX,XX +XXX,XX @@ def run_test(self, test: str, description=res.description) if res.casenotrun: - print(res.casenotrun) + if self.tap: + print('#' + res.casenotrun.replace('\n', '\n#')) + else: + print(res.casenotrun) return res -- 2.34.1 Drop the use of OUTPUT_DIR (test/qemu-iotests under the build directory), and instead write test output files (.out.bad, .notrun, and .casenotrun) to TEST_DIR. With this, the same test can be run concurrently without the separate instances interfering, because they will need separate TEST_DIRs anyway. Running the same test separately is useful when running the iotests with various format/protocol combinations in parallel, or when you just want to aggressively exercise a single test (e.g. when it fails only sporadically). Putting this output into TEST_DIR means that it will stick around for inspection after the test run is done (though running the same test in the same TEST_DIR will overwrite it, just as it used to be); but given that TEST_DIR is a scratch directory, it should be clear that users can delete all of its content at any point. (And if TEST_DIR is on tmpfs, it will just disappear on shutdown.) Contrarily, alternative approaches that would put these output files into OUTPUT_DIR with some prefix to differentiate between separate test runs might easily lead to cluttering OUTPUT_DIR. (This change means OUTPUT_DIR is no longer written to by the iotests, so we can drop its usage altogether.) Signed-off-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220221172909.762858-1-hreitz@redhat.com> [hreitz: Simplified `Path(os.path.join(x, y))` to `Path(x, y)`, as suggested by Vladimir; and rebased on 9086c7639822b6 ("tests/qemu-iotests: Rework the checks and spots using GNU sed")] Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/common.rc | 6 +++--- tests/qemu-iotests/iotests.py | 5 ++--- tests/qemu-iotests/testenv.py | 5 +---- tests/qemu-iotests/testrunner.py | 14 ++++++++------ 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -XXX,XX +XXX,XX @@ # bail out, setting up .notrun file _notrun() { - echo "$*" >"$OUTPUT_DIR/$seq.notrun" + echo "$*" >"$TEST_DIR/$seq.notrun" echo "$seq not run: $*" status=0 exit @@ -XXX,XX +XXX,XX @@ _img_info() # _casenotrun() { - echo " [case not run] $*" >>"$OUTPUT_DIR/$seq.casenotrun" + echo " [case not run] $*" >>"$TEST_DIR/$seq.casenotrun" } # just plain bail out # _fail() { - echo "$*" | tee -a "$OUTPUT_DIR/$seq.full" + echo "$*" | tee -a "$TEST_DIR/$seq.full" echo "(see $seq.full for details)" status=1 exit 1 diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -XXX,XX +XXX,XX @@ imgfmt = os.environ.get('IMGFMT', 'raw') imgproto = os.environ.get('IMGPROTO', 'file') -output_dir = os.environ.get('OUTPUT_DIR', '.') try: test_dir = os.environ['TEST_DIR'] @@ -XXX,XX +XXX,XX @@ def notrun(reason): # Each test in qemu-iotests has a number ("seq") seq = os.path.basename(sys.argv[0]) - with open('%s/%s.notrun' % (output_dir, seq), 'w', encoding='utf-8') \ + with open('%s/%s.notrun' % (test_dir, seq), 'w', encoding='utf-8') \ as outfile: outfile.write(reason + '\n') logger.warning("%s not run: %s", seq, reason) @@ -XXX,XX +XXX,XX @@ def case_notrun(reason): # Each test in qemu-iotests has a number ("seq") seq = os.path.basename(sys.argv[0]) - with open('%s/%s.casenotrun' % (output_dir, seq), 'a', encoding='utf-8') \ + with open('%s/%s.casenotrun' % (test_dir, seq), 'a', encoding='utf-8') \ as outfile: outfile.write(' [case not run] ' + reason + '\n') diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -XXX,XX +XXX,XX @@ class TestEnv(ContextManager['TestEnv']): # pylint: disable=too-many-instance-attributes env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR', - 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG', + 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG', 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS', 'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT', @@ -XXX,XX +XXX,XX @@ def init_directories(self) -> None: TEST_DIR SOCK_DIR SAMPLE_IMG_DIR - OUTPUT_DIR """ # Path where qemu goodies live in this source tree. @@ -XXX,XX +XXX,XX @@ def init_directories(self) -> None: os.path.join(self.source_iotests, 'sample_images')) - self.output_dir = os.getcwd() # OUTPUT_DIR - def init_binaries(self) -> None: """Init binary path variables: PYTHON (for bash tests) diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -XXX,XX +XXX,XX @@ def do_run_test(self, test: str, mp: bool) -> TestResult: """ f_test = Path(test) - f_bad = Path(f_test.name + '.out.bad') - f_notrun = Path(f_test.name + '.notrun') - f_casenotrun = Path(f_test.name + '.casenotrun') f_reference = Path(self.find_reference(test)) if not f_test.exists(): @@ -XXX,XX +XXX,XX @@ def do_run_test(self, test: str, mp: bool) -> TestResult: description='No qualified output ' f'(expected {f_reference})') - for p in (f_bad, f_notrun, f_casenotrun): - silent_unlink(p) - args = [str(f_test.resolve())] env = self.env.prepare_subprocess(args) if mp: @@ -XXX,XX +XXX,XX @@ def do_run_test(self, test: str, mp: bool) -> TestResult: env[d] = os.path.join(env[d], f_test.name) Path(env[d]).mkdir(parents=True, exist_ok=True) + test_dir = env['TEST_DIR'] + f_bad = Path(test_dir, f_test.name + '.out.bad') + f_notrun = Path(test_dir, f_test.name + '.notrun') + f_casenotrun = Path(test_dir, f_test.name + '.casenotrun') + + for p in (f_notrun, f_casenotrun): + silent_unlink(p) + t0 = time.time() with f_bad.open('w', encoding="utf-8") as f: with subprocess.Popen(args, cwd=str(f_test.parent), env=env, -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> We are going to complicate bitmap initialization in the further commit. And in future, backup job will be able to work without filter (when source is immutable), so we'll need same bitmap initialization in copy-before-write filter and in backup job. So, it's reasonable to do it in block-copy. Note that for now cbw_open() is the only caller of block_copy_state_new(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-2-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- block/block-copy.c | 1 + block/copy-before-write.c | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index XXXXXXX..XXXXXXX 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, return NULL; } bdrv_disable_dirty_bitmap(copy_bitmap); + bdrv_set_dirty_bitmap(copy_bitmap, 0, bdrv_dirty_bitmap_size(copy_bitmap)); /* * If source is in backing chain of target assume that target is going to be diff --git a/block/copy-before-write.c b/block/copy-before-write.c index XXXXXXX..XXXXXXX 100644 --- a/block/copy-before-write.c +++ b/block/copy-before-write.c @@ -XXX,XX +XXX,XX @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVCopyBeforeWriteState *s = bs->opaque; - BdrvDirtyBitmap *copy_bitmap; bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds, BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, @@ -XXX,XX +XXX,XX @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags, return -EINVAL; } - copy_bitmap = block_copy_dirty_bitmap(s->bcs); - bdrv_set_dirty_bitmap(copy_bitmap, 0, bdrv_dirty_bitmap_size(copy_bitmap)); - return 0; } -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> That simplifies handling failure in existing code and in further new usage of bdrv_merge_dirty_bitmap(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-3-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/dirty-bitmap.h | 2 +- block/dirty-bitmap.c | 9 +++++++-- block/monitor/bitmap-qmp-cmds.c | 5 +---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -XXX,XX +XXX,XX @@ void bdrv_dirty_bitmap_set_persistence(BdrvDirtyBitmap *bitmap, bool persistent); void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap); void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy); -void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, +bool bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, HBitmap **backup, Error **errp); void bdrv_dirty_bitmap_skip_store(BdrvDirtyBitmap *bitmap, bool skip); bool bdrv_dirty_bitmap_get(BdrvDirtyBitmap *bitmap, int64_t offset); diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index XXXXXXX..XXXXXXX 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -XXX,XX +XXX,XX @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap, * Ensures permissions on bitmaps are reasonable; use for public API. * * @backup: If provided, make a copy of dest here prior to merge. + * + * Returns true on success, false on failure. In case of failure bitmaps are + * untouched. */ -void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, +bool bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, HBitmap **backup, Error **errp) { - bool ret; + bool ret = false; bdrv_dirty_bitmaps_lock(dest->bs); if (src->bs != dest->bs) { @@ -XXX,XX +XXX,XX @@ out: if (src->bs != dest->bs) { bdrv_dirty_bitmaps_unlock(src->bs); } + + return ret; } /** diff --git a/block/monitor/bitmap-qmp-cmds.c b/block/monitor/bitmap-qmp-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/block/monitor/bitmap-qmp-cmds.c +++ b/block/monitor/bitmap-qmp-cmds.c @@ -XXX,XX +XXX,XX @@ BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target, BlockDriverState *bs; BdrvDirtyBitmap *dst, *src, *anon; BlockDirtyBitmapMergeSourceList *lst; - Error *local_err = NULL; GLOBAL_STATE_CODE(); @@ -XXX,XX +XXX,XX @@ BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target, abort(); } - bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err); - if (local_err) { - error_propagate(errp, local_err); + if (!bdrv_merge_dirty_bitmap(anon, src, NULL, errp)) { dst = NULL; goto out; } -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> This will be used in the following commit to bring "incremental" mode to copy-before-write filter. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-4-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/block-copy.h | 1 + block/block-copy.c | 14 +++++++++++++- block/copy-before-write.c | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/include/block/block-copy.h b/include/block/block-copy.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyState BlockCopyState; typedef struct BlockCopyCallState BlockCopyCallState; BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, + const BdrvDirtyBitmap *bitmap, Error **errp); /* Function should be called prior any actual copy request */ diff --git a/block/block-copy.c b/block/block-copy.c index XXXXXXX..XXXXXXX 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -XXX,XX +XXX,XX @@ static int64_t block_copy_calculate_cluster_size(BlockDriverState *target, } BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, + const BdrvDirtyBitmap *bitmap, Error **errp) { + ERRP_GUARD(); BlockCopyState *s; int64_t cluster_size; BdrvDirtyBitmap *copy_bitmap; @@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, return NULL; } bdrv_disable_dirty_bitmap(copy_bitmap); - bdrv_set_dirty_bitmap(copy_bitmap, 0, bdrv_dirty_bitmap_size(copy_bitmap)); + if (bitmap) { + if (!bdrv_merge_dirty_bitmap(copy_bitmap, bitmap, NULL, errp)) { + error_prepend(errp, "Failed to merge bitmap '%s' to internal " + "copy-bitmap: ", bdrv_dirty_bitmap_name(bitmap)); + bdrv_release_dirty_bitmap(copy_bitmap); + return NULL; + } + } else { + bdrv_set_dirty_bitmap(copy_bitmap, 0, + bdrv_dirty_bitmap_size(copy_bitmap)); + } /* * If source is in backing chain of target assume that target is going to be diff --git a/block/copy-before-write.c b/block/copy-before-write.c index XXXXXXX..XXXXXXX 100644 --- a/block/copy-before-write.c +++ b/block/copy-before-write.c @@ -XXX,XX +XXX,XX @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags, ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) & bs->file->bs->supported_zero_flags); - s->bcs = block_copy_state_new(bs->file, s->target, errp); + s->bcs = block_copy_state_new(bs->file, s->target, NULL, errp); if (!s->bcs) { error_prepend(errp, "Cannot create block-copy-state: "); return -EINVAL; -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> This brings "incremental" mode to copy-before-write filter: user can specify bitmap so that filter will copy only "dirty" areas. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20220303194349.2304213-5-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- qapi/block-core.json | 10 +++++++- block/copy-before-write.c | 51 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index XXXXXXX..XXXXXXX 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -XXX,XX +XXX,XX @@ # # @target: The target for copy-before-write operations. # +# @bitmap: If specified, copy-before-write filter will do +# copy-before-write operations only for dirty regions of the +# bitmap. Bitmap size must be equal to length of file and +# target child of the filter. Note also, that bitmap is used +# only to initialize internal bitmap of the process, so further +# modifications (or removing) of specified bitmap doesn't +# influence the filter. (Since 7.0) +# # Since: 6.2 ## { 'struct': 'BlockdevOptionsCbw', 'base': 'BlockdevOptionsGenericFormat', - 'data': { 'target': 'BlockdevRef' } } + 'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap' } } ## # @BlockdevOptions: diff --git a/block/copy-before-write.c b/block/copy-before-write.c index XXXXXXX..XXXXXXX 100644 --- a/block/copy-before-write.c +++ b/block/copy-before-write.c @@ -XXX,XX +XXX,XX @@ #include "block/copy-before-write.h" +#include "qapi/qapi-visit-block-core.h" + typedef struct BDRVCopyBeforeWriteState { BlockCopyState *bcs; BdrvChild *target; @@ -XXX,XX +XXX,XX @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c, } } +static bool cbw_parse_bitmap_option(QDict *options, BdrvDirtyBitmap **bitmap, + Error **errp) +{ + QDict *bitmap_qdict = NULL; + BlockDirtyBitmap *bmp_param = NULL; + Visitor *v = NULL; + bool ret = false; + + *bitmap = NULL; + + qdict_extract_subqdict(options, &bitmap_qdict, "bitmap."); + if (!qdict_size(bitmap_qdict)) { + ret = true; + goto out; + } + + v = qobject_input_visitor_new_flat_confused(bitmap_qdict, errp); + if (!v) { + goto out; + } + + visit_type_BlockDirtyBitmap(v, NULL, &bmp_param, errp); + if (!bmp_param) { + goto out; + } + + *bitmap = block_dirty_bitmap_lookup(bmp_param->node, bmp_param->name, NULL, + errp); + if (!*bitmap) { + goto out; + } + + ret = true; + +out: + qapi_free_BlockDirtyBitmap(bmp_param); + visit_free(v); + qobject_unref(bitmap_qdict); + + return ret; +} + static int cbw_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVCopyBeforeWriteState *s = bs->opaque; + BdrvDirtyBitmap *bitmap = NULL; bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds, BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, @@ -XXX,XX +XXX,XX @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags, return -EINVAL; } + if (!cbw_parse_bitmap_option(options, &bitmap, errp)) { + return -EINVAL; + } + bs->total_sectors = bs->file->bs->total_sectors; bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED | (BDRV_REQ_FUA & bs->file->bs->supported_write_flags); @@ -XXX,XX +XXX,XX @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags, ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) & bs->file->bs->supported_zero_flags); - s->bcs = block_copy_state_new(bs->file, s->target, NULL, errp); + s->bcs = block_copy_state_new(bs->file, s->target, bitmap, errp); if (!s->bcs) { error_prepend(errp, "Cannot create block-copy-state: "); return -EINVAL; -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Split block_copy_reset() out of block_copy_reset_unallocated() to be used separately later. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-6-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/block-copy.h | 1 + block/block-copy.c | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/include/block/block-copy.h b/include/block/block-copy.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -XXX,XX +XXX,XX @@ void block_copy_set_progress_meter(BlockCopyState *s, ProgressMeter *pm); void block_copy_state_free(BlockCopyState *s); +void block_copy_reset(BlockCopyState *s, int64_t offset, int64_t bytes); int64_t block_copy_reset_unallocated(BlockCopyState *s, int64_t offset, int64_t *count); diff --git a/block/block-copy.c b/block/block-copy.c index XXXXXXX..XXXXXXX 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -XXX,XX +XXX,XX @@ static int block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset, } } +void block_copy_reset(BlockCopyState *s, int64_t offset, int64_t bytes) +{ + QEMU_LOCK_GUARD(&s->lock); + + bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes); + if (s->progress) { + progress_set_remaining(s->progress, + bdrv_get_dirty_count(s->copy_bitmap) + + s->in_flight_bytes); + } +} + /* * Reset bits in copy_bitmap starting at offset if they represent unallocated * data in the image. May reset subsequent contiguous bits. @@ -XXX,XX +XXX,XX @@ int64_t block_copy_reset_unallocated(BlockCopyState *s, bytes = clusters * s->cluster_size; if (!ret) { - qemu_co_mutex_lock(&s->lock); - bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes); - if (s->progress) { - progress_set_remaining(s->progress, - bdrv_get_dirty_count(s->copy_bitmap) + - s->in_flight_bytes); - } - qemu_co_mutex_unlock(&s->lock); + block_copy_reset(s, offset, bytes); } *count = bytes; -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Split intersecting-requests functionality out of block-copy to be reused in copy-before-write filter. Note: while being here, fix tiny typo in MAINTAINERS. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-7-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/reqlist.h | 67 +++++++++++++++++++++++ block/block-copy.c | 116 +++++++++++++--------------------------- block/reqlist.c | 76 ++++++++++++++++++++++++++ MAINTAINERS | 4 +- block/meson.build | 1 + 5 files changed, 184 insertions(+), 80 deletions(-) create mode 100644 include/block/reqlist.h create mode 100644 block/reqlist.c diff --git a/include/block/reqlist.h b/include/block/reqlist.h new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/include/block/reqlist.h @@ -XXX,XX +XXX,XX @@ +/* + * reqlist API + * + * Copyright (C) 2013 Proxmox Server Solutions + * Copyright (c) 2021 Virtuozzo International GmbH. + * + * Authors: + * Dietmar Maurer (dietmar@proxmox.com) + * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef REQLIST_H +#define REQLIST_H + +#include "qemu/coroutine.h" + +/* + * The API is not thread-safe and shouldn't be. The struct is public to be part + * of other structures and protected by third-party locks, see + * block/block-copy.c for example. + */ + +typedef struct BlockReq { + int64_t offset; + int64_t bytes; + + CoQueue wait_queue; /* coroutines blocked on this req */ + QLIST_ENTRY(BlockReq) list; +} BlockReq; + +typedef QLIST_HEAD(, BlockReq) BlockReqList; + +/* + * Initialize new request and add it to the list. Caller must be sure that + * there are no conflicting requests in the list. + */ +void reqlist_init_req(BlockReqList *reqs, BlockReq *req, int64_t offset, + int64_t bytes); +/* Search for request in the list intersecting with @offset/@bytes area. */ +BlockReq *reqlist_find_conflict(BlockReqList *reqs, int64_t offset, + int64_t bytes); + +/* + * If there are no intersecting requests return false. Otherwise, wait for the + * first found intersecting request to finish and return true. + * + * @lock is passed to qemu_co_queue_wait() + * False return value proves that lock was released at no point. + */ +bool coroutine_fn reqlist_wait_one(BlockReqList *reqs, int64_t offset, + int64_t bytes, CoMutex *lock); + +/* + * Shrink request and wake all waiting coroutines (maybe some of them are not + * intersecting with shrunk request). + */ +void coroutine_fn reqlist_shrink_req(BlockReq *req, int64_t new_bytes); + +/* + * Remove request and wake all waiting coroutines. Do not release any memory. + */ +void coroutine_fn reqlist_remove_req(BlockReq *req); + +#endif /* REQLIST_H */ diff --git a/block/block-copy.c b/block/block-copy.c index XXXXXXX..XXXXXXX 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -XXX,XX +XXX,XX @@ #include "trace.h" #include "qapi/error.h" #include "block/block-copy.h" +#include "block/reqlist.h" #include "sysemu/block-backend.h" #include "qemu/units.h" #include "qemu/coroutine.h" @@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyTask { */ BlockCopyState *s; BlockCopyCallState *call_state; - int64_t offset; /* * @method can also be set again in the while loop of * block_copy_dirty_clusters(), but it is never accessed concurrently @@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyTask { BlockCopyMethod method; /* - * Fields whose state changes throughout the execution - * Protected by lock in BlockCopyState. + * Generally, req is protected by lock in BlockCopyState, Still req.offset + * is only set on task creation, so may be read concurrently after creation. + * req.bytes is changed at most once, and need only protecting the case of + * parallel read while updating @bytes value in block_copy_task_shrink(). */ - CoQueue wait_queue; /* coroutines blocked on this task */ - /* - * Only protect the case of parallel read while updating @bytes - * value in block_copy_task_shrink(). - */ - int64_t bytes; - QLIST_ENTRY(BlockCopyTask) list; + BlockReq req; } BlockCopyTask; static int64_t task_end(BlockCopyTask *task) { - return task->offset + task->bytes; + return task->req.offset + task->req.bytes; } typedef struct BlockCopyState { @@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyState { CoMutex lock; int64_t in_flight_bytes; BlockCopyMethod method; - QLIST_HEAD(, BlockCopyTask) tasks; /* All tasks from all block-copy calls */ + BlockReqList reqs; QLIST_HEAD(, BlockCopyCallState) calls; /* * skip_unallocated: @@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyState { RateLimit rate_limit; } BlockCopyState; -/* Called with lock held */ -static BlockCopyTask *find_conflicting_task(BlockCopyState *s, - int64_t offset, int64_t bytes) -{ - BlockCopyTask *t; - - QLIST_FOREACH(t, &s->tasks, list) { - if (offset + bytes > t->offset && offset < t->offset + t->bytes) { - return t; - } - } - - return NULL; -} - -/* - * If there are no intersecting tasks return false. Otherwise, wait for the - * first found intersecting tasks to finish and return true. - * - * Called with lock held. May temporary release the lock. - * Return value of 0 proves that lock was NOT released. - */ -static bool coroutine_fn block_copy_wait_one(BlockCopyState *s, int64_t offset, - int64_t bytes) -{ - BlockCopyTask *task = find_conflicting_task(s, offset, bytes); - - if (!task) { - return false; - } - - qemu_co_queue_wait(&task->wait_queue, &s->lock); - - return true; -} - /* Called with lock held */ static int64_t block_copy_chunk_size(BlockCopyState *s) { @@ -XXX,XX +XXX,XX @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state, bytes = QEMU_ALIGN_UP(bytes, s->cluster_size); /* region is dirty, so no existent tasks possible in it */ - assert(!find_conflicting_task(s, offset, bytes)); + assert(!reqlist_find_conflict(&s->reqs, offset, bytes)); bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes); s->in_flight_bytes += bytes; @@ -XXX,XX +XXX,XX @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state, .task.func = block_copy_task_entry, .s = s, .call_state = call_state, - .offset = offset, - .bytes = bytes, .method = s->method, }; - qemu_co_queue_init(&task->wait_queue); - QLIST_INSERT_HEAD(&s->tasks, task, list); + reqlist_init_req(&s->reqs, &task->req, offset, bytes); return task; } @@ -XXX,XX +XXX,XX @@ static void coroutine_fn block_copy_task_shrink(BlockCopyTask *task, int64_t new_bytes) { QEMU_LOCK_GUARD(&task->s->lock); - if (new_bytes == task->bytes) { + if (new_bytes == task->req.bytes) { return; } - assert(new_bytes > 0 && new_bytes < task->bytes); + assert(new_bytes > 0 && new_bytes < task->req.bytes); - task->s->in_flight_bytes -= task->bytes - new_bytes; + task->s->in_flight_bytes -= task->req.bytes - new_bytes; bdrv_set_dirty_bitmap(task->s->copy_bitmap, - task->offset + new_bytes, task->bytes - new_bytes); + task->req.offset + new_bytes, + task->req.bytes - new_bytes); - task->bytes = new_bytes; - qemu_co_queue_restart_all(&task->wait_queue); + reqlist_shrink_req(&task->req, new_bytes); } static void coroutine_fn block_copy_task_end(BlockCopyTask *task, int ret) { QEMU_LOCK_GUARD(&task->s->lock); - task->s->in_flight_bytes -= task->bytes; + task->s->in_flight_bytes -= task->req.bytes; if (ret < 0) { - bdrv_set_dirty_bitmap(task->s->copy_bitmap, task->offset, task->bytes); + bdrv_set_dirty_bitmap(task->s->copy_bitmap, task->req.offset, + task->req.bytes); } - QLIST_REMOVE(task, list); if (task->s->progress) { progress_set_remaining(task->s->progress, bdrv_get_dirty_count(task->s->copy_bitmap) + task->s->in_flight_bytes); } - qemu_co_queue_restart_all(&task->wait_queue); + reqlist_remove_req(&task->req); } void block_copy_state_free(BlockCopyState *s) @@ -XXX,XX +XXX,XX @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, ratelimit_init(&s->rate_limit); qemu_co_mutex_init(&s->lock); - QLIST_INIT(&s->tasks); + QLIST_INIT(&s->reqs); QLIST_INIT(&s->calls); return s; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int block_copy_task_run(AioTaskPool *pool, aio_task_pool_wait_slot(pool); if (aio_task_pool_status(pool) < 0) { - co_put_to_shres(task->s->mem, task->bytes); + co_put_to_shres(task->s->mem, task->req.bytes); block_copy_task_end(task, -ECANCELED); g_free(task); return -ECANCELED; @@ -XXX,XX +XXX,XX @@ static coroutine_fn int block_copy_task_entry(AioTask *task) BlockCopyMethod method = t->method; int ret; - ret = block_copy_do_copy(s, t->offset, t->bytes, &method, &error_is_read); + ret = block_copy_do_copy(s, t->req.offset, t->req.bytes, &method, + &error_is_read); WITH_QEMU_LOCK_GUARD(&s->lock) { if (s->method == t->method) { @@ -XXX,XX +XXX,XX @@ static coroutine_fn int block_copy_task_entry(AioTask *task) t->call_state->error_is_read = error_is_read; } } else if (s->progress) { - progress_work_done(s->progress, t->bytes); + progress_work_done(s->progress, t->req.bytes); } } - co_put_to_shres(s->mem, t->bytes); + co_put_to_shres(s->mem, t->req.bytes); block_copy_task_end(t, ret); return ret; @@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state) trace_block_copy_skip_range(s, offset, bytes); break; } - if (task->offset > offset) { - trace_block_copy_skip_range(s, offset, task->offset - offset); + if (task->req.offset > offset) { + trace_block_copy_skip_range(s, offset, task->req.offset - offset); } found_dirty = true; - ret = block_copy_block_status(s, task->offset, task->bytes, + ret = block_copy_block_status(s, task->req.offset, task->req.bytes, &status_bytes); assert(ret >= 0); /* never fail */ - if (status_bytes < task->bytes) { + if (status_bytes < task->req.bytes) { block_copy_task_shrink(task, status_bytes); } if (qatomic_read(&s->skip_unallocated) && !(ret & BDRV_BLOCK_ALLOCATED)) { block_copy_task_end(task, 0); - trace_block_copy_skip_range(s, task->offset, task->bytes); + trace_block_copy_skip_range(s, task->req.offset, task->req.bytes); offset = task_end(task); bytes = end - offset; g_free(task); @@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state) } } - ratelimit_calculate_delay(&s->rate_limit, task->bytes); + ratelimit_calculate_delay(&s->rate_limit, task->req.bytes); - trace_block_copy_process(s, task->offset); + trace_block_copy_process(s, task->req.offset); - co_get_from_shres(s->mem, task->bytes); + co_get_from_shres(s->mem, task->req.bytes); offset = task_end(task); bytes = end - offset; @@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) * Check that there is no task we still need to * wait to complete */ - ret = block_copy_wait_one(s, call_state->offset, - call_state->bytes); + ret = reqlist_wait_one(&s->reqs, call_state->offset, + call_state->bytes, &s->lock); if (ret == 0) { /* * No pending tasks, but check again the bitmap in this @@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) * between this and the critical section in * block_copy_dirty_clusters(). * - * block_copy_wait_one return value 0 also means that it + * reqlist_wait_one return value 0 also means that it * didn't release the lock. So, we are still in the same * critical section, not interrupted by any concurrent * access to state. diff --git a/block/reqlist.c b/block/reqlist.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/block/reqlist.c @@ -XXX,XX +XXX,XX @@ +/* + * reqlist API + * + * Copyright (C) 2013 Proxmox Server Solutions + * Copyright (c) 2021 Virtuozzo International GmbH. + * + * Authors: + * Dietmar Maurer (dietmar@proxmox.com) + * Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" + +#include "block/reqlist.h" + +void reqlist_init_req(BlockReqList *reqs, BlockReq *req, int64_t offset, + int64_t bytes) +{ + assert(!reqlist_find_conflict(reqs, offset, bytes)); + + *req = (BlockReq) { + .offset = offset, + .bytes = bytes, + }; + qemu_co_queue_init(&req->wait_queue); + QLIST_INSERT_HEAD(reqs, req, list); +} + +BlockReq *reqlist_find_conflict(BlockReqList *reqs, int64_t offset, + int64_t bytes) +{ + BlockReq *r; + + QLIST_FOREACH(r, reqs, list) { + if (offset + bytes > r->offset && offset < r->offset + r->bytes) { + return r; + } + } + + return NULL; +} + +bool coroutine_fn reqlist_wait_one(BlockReqList *reqs, int64_t offset, + int64_t bytes, CoMutex *lock) +{ + BlockReq *r = reqlist_find_conflict(reqs, offset, bytes); + + if (!r) { + return false; + } + + qemu_co_queue_wait(&r->wait_queue, lock); + + return true; +} + +void coroutine_fn reqlist_shrink_req(BlockReq *req, int64_t new_bytes) +{ + if (new_bytes == req->bytes) { + return; + } + + assert(new_bytes > 0 && new_bytes < req->bytes); + + req->bytes = new_bytes; + qemu_co_queue_restart_all(&req->wait_queue); +} + +void coroutine_fn reqlist_remove_req(BlockReq *req) +{ + QLIST_REMOVE(req, list); + qemu_co_queue_restart_all(&req->wait_queue); +} diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: block/stream.c F: block/mirror.c F: qapi/job.json F: block/block-copy.c -F: include/block/block-copy.c +F: include/block/block-copy.h +F: block/reqlist.c +F: include/block/reqlist.h F: block/copy-before-write.h F: block/copy-before-write.c F: include/block/aio_task.h diff --git a/block/meson.build b/block/meson.build index XXXXXXX..XXXXXXX 100644 --- a/block/meson.build +++ b/block/meson.build @@ -XXX,XX +XXX,XX @@ block_ss.add(files( 'qcow2.c', 'quorum.c', 'raw-format.c', + 'reqlist.c', 'snapshot.c', 'throttle-groups.c', 'throttle.c', -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Let's reuse convenient helper. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-8-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- block/reqlist.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/reqlist.c b/block/reqlist.c index XXXXXXX..XXXXXXX 100644 --- a/block/reqlist.c +++ b/block/reqlist.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" +#include "qemu/range.h" #include "block/reqlist.h" @@ -XXX,XX +XXX,XX @@ BlockReq *reqlist_find_conflict(BlockReqList *reqs, int64_t offset, BlockReq *r; QLIST_FOREACH(r, reqs, list) { - if (offset + bytes > r->offset && offset < r->offset + r->bytes) { + if (ranges_overlap(offset, bytes, r->offset, r->bytes)) { return r; } } -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Add a convenient function similar with bdrv_block_status() to get status of dirty bitmap. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-9-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/dirty-bitmap.h | 2 ++ include/qemu/hbitmap.h | 12 ++++++++++++ block/dirty-bitmap.c | 6 ++++++ util/hbitmap.c | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+) diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -XXX,XX +XXX,XX @@ int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, int64_t offset, bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap, int64_t start, int64_t end, int64_t max_dirty_count, int64_t *dirty_start, int64_t *dirty_count); +bool bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap, int64_t offset, + int64_t bytes, int64_t *count); BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap, Error **errp); diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/hbitmap.h +++ b/include/qemu/hbitmap.h @@ -XXX,XX +XXX,XX @@ bool hbitmap_next_dirty_area(const HBitmap *hb, int64_t start, int64_t end, int64_t max_dirty_count, int64_t *dirty_start, int64_t *dirty_count); +/* + * bdrv_dirty_bitmap_status: + * @hb: The HBitmap to operate on + * @start: The bit to start from + * @count: Number of bits to proceed + * @pnum: Out-parameter. How many bits has same value starting from @start + * + * Returns true if bitmap is dirty at @start, false otherwise. + */ +bool hbitmap_status(const HBitmap *hb, int64_t start, int64_t count, + int64_t *pnum); + /** * hbitmap_iter_next: * @hbi: HBitmapIter to operate on. diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index XXXXXXX..XXXXXXX 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -XXX,XX +XXX,XX @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap, dirty_start, dirty_count); } +bool bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap, int64_t offset, + int64_t bytes, int64_t *count) +{ + return hbitmap_status(bitmap->bitmap, offset, bytes, count); +} + /** * bdrv_merge_dirty_bitmap: merge src into dest. * Ensures permissions on bitmaps are reasonable; use for public API. diff --git a/util/hbitmap.c b/util/hbitmap.c index XXXXXXX..XXXXXXX 100644 --- a/util/hbitmap.c +++ b/util/hbitmap.c @@ -XXX,XX +XXX,XX @@ bool hbitmap_next_dirty_area(const HBitmap *hb, int64_t start, int64_t end, return true; } +bool hbitmap_status(const HBitmap *hb, int64_t start, int64_t count, + int64_t *pnum) +{ + int64_t next_dirty, next_zero; + + assert(start >= 0); + assert(count > 0); + assert(start + count <= hb->orig_size); + + next_dirty = hbitmap_next_dirty(hb, start, count); + if (next_dirty == -1) { + *pnum = count; + return false; + } + + if (next_dirty > start) { + *pnum = next_dirty - start; + return false; + } + + assert(next_dirty == start); + + next_zero = hbitmap_next_zero(hb, start, count); + if (next_zero == -1) { + *pnum = count; + return true; + } + + assert(next_zero > start); + *pnum = next_zero - start; + return false; +} + bool hbitmap_empty(const HBitmap *hb) { return hb->count == 0; -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Add function to wait for all intersecting requests. To be used in the further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Nikita Lapshin <nikita.lapshin@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-10-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/reqlist.h | 8 ++++++++ block/reqlist.c | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/include/block/reqlist.h b/include/block/reqlist.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/reqlist.h +++ b/include/block/reqlist.h @@ -XXX,XX +XXX,XX @@ BlockReq *reqlist_find_conflict(BlockReqList *reqs, int64_t offset, bool coroutine_fn reqlist_wait_one(BlockReqList *reqs, int64_t offset, int64_t bytes, CoMutex *lock); +/* + * Wait for all intersecting requests. It just calls reqlist_wait_one() in a + * loop, caller is responsible to stop producing new requests in this region + * in parallel, otherwise reqlist_wait_all() may never return. + */ +void coroutine_fn reqlist_wait_all(BlockReqList *reqs, int64_t offset, + int64_t bytes, CoMutex *lock); + /* * Shrink request and wake all waiting coroutines (maybe some of them are not * intersecting with shrunk request). diff --git a/block/reqlist.c b/block/reqlist.c index XXXXXXX..XXXXXXX 100644 --- a/block/reqlist.c +++ b/block/reqlist.c @@ -XXX,XX +XXX,XX @@ bool coroutine_fn reqlist_wait_one(BlockReqList *reqs, int64_t offset, return true; } +void coroutine_fn reqlist_wait_all(BlockReqList *reqs, int64_t offset, + int64_t bytes, CoMutex *lock) +{ + while (reqlist_wait_one(reqs, offset, bytes, lock)) { + /* continue */ + } +} + void coroutine_fn reqlist_shrink_req(BlockReq *req, int64_t new_bytes) { if (new_bytes == req->bytes) { -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Add new block driver handlers and corresponding generic wrappers. It will be used to allow copy-before-write filter to provide reach fleecing interface in further commit. In future this approach may be used to allow reading qcow2 internal snapshots, for example to export them through NBD. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-11-vsementsov@virtuozzo.com> [hreitz: Rebased on block GS/IO split] Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- include/block/block_int-common.h | 18 ++++++++ include/block/block_int-io.h | 9 ++++ block/io.c | 72 ++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+) diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -XXX,XX +XXX,XX @@ struct BlockDriver { bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum, int64_t *map, BlockDriverState **file); + /* + * Snapshot-access API. + * + * Block-driver may provide snapshot-access API: special functions to access + * some internal "snapshot". The functions are similar with normal + * read/block_status/discard handler, but don't have any specific handling + * in generic block-layer: no serializing, no alignment, no tracked + * requests. So, block-driver that realizes these APIs is fully responsible + * for synchronization between snapshot-access API and normal IO requests. + */ + int coroutine_fn (*bdrv_co_preadv_snapshot)(BlockDriverState *bs, + int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset); + int coroutine_fn (*bdrv_co_snapshot_block_status)(BlockDriverState *bs, + bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum, + int64_t *map, BlockDriverState **file); + int coroutine_fn (*bdrv_co_pdiscard_snapshot)(BlockDriverState *bs, + int64_t offset, int64_t bytes); + /* * Invalidate any cached meta-data. */ diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block_int-io.h +++ b/include/block/block_int-io.h @@ -XXX,XX +XXX,XX @@ * the I/O API. */ +int coroutine_fn bdrv_co_preadv_snapshot(BdrvChild *child, + int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset); +int coroutine_fn bdrv_co_snapshot_block_status(BlockDriverState *bs, + bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum, + int64_t *map, BlockDriverState **file); +int coroutine_fn bdrv_co_pdiscard_snapshot(BlockDriverState *bs, + int64_t offset, int64_t bytes); + + int coroutine_fn bdrv_co_preadv(BdrvChild *child, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); diff --git a/block/io.c b/block/io.c index XXXXXXX..XXXXXXX 100644 --- a/block/io.c +++ b/block/io.c @@ -XXX,XX +XXX,XX @@ void bdrv_cancel_in_flight(BlockDriverState *bs) bs->drv->bdrv_cancel_in_flight(bs); } } + +int coroutine_fn +bdrv_co_preadv_snapshot(BdrvChild *child, int64_t offset, int64_t bytes, + QEMUIOVector *qiov, size_t qiov_offset) +{ + BlockDriverState *bs = child->bs; + BlockDriver *drv = bs->drv; + int ret; + IO_CODE(); + + if (!drv) { + return -ENOMEDIUM; + } + + if (!drv->bdrv_co_preadv_snapshot) { + return -ENOTSUP; + } + + bdrv_inc_in_flight(bs); + ret = drv->bdrv_co_preadv_snapshot(bs, offset, bytes, qiov, qiov_offset); + bdrv_dec_in_flight(bs); + + return ret; +} + +int coroutine_fn +bdrv_co_snapshot_block_status(BlockDriverState *bs, + bool want_zero, int64_t offset, int64_t bytes, + int64_t *pnum, int64_t *map, + BlockDriverState **file) +{ + BlockDriver *drv = bs->drv; + int ret; + IO_CODE(); + + if (!drv) { + return -ENOMEDIUM; + } + + if (!drv->bdrv_co_snapshot_block_status) { + return -ENOTSUP; + } + + bdrv_inc_in_flight(bs); + ret = drv->bdrv_co_snapshot_block_status(bs, want_zero, offset, bytes, + pnum, map, file); + bdrv_dec_in_flight(bs); + + return ret; +} + +int coroutine_fn +bdrv_co_pdiscard_snapshot(BlockDriverState *bs, int64_t offset, int64_t bytes) +{ + BlockDriver *drv = bs->drv; + int ret; + IO_CODE(); + + if (!drv) { + return -ENOMEDIUM; + } + + if (!drv->bdrv_co_pdiscard_snapshot) { + return -ENOTSUP; + } + + bdrv_inc_in_flight(bs); + ret = drv->bdrv_co_pdiscard_snapshot(bs, offset, bytes); + bdrv_dec_in_flight(bs); + + return ret; +} -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> The new block driver simply utilizes snapshot-access API of underlying block node. In further patches we want to use it like this: [guest] [NBD export] | | | root | root v file v [copy-before-write]<------[snapshot-access] | | | file | target v v [active-disk] [temp.img] This way, NBD client will be able to read snapshotted state of active disk, when active disk is continued to be written by guest. This is known as "fleecing", and currently uses another scheme based on qcow2 temporary image which backing file is active-disk. New scheme comes with benefits - see next commit. The other possible application is exporting internal snapshots of qcow2, like this: [guest] [NBD export] | | | root | root v file v [qcow2]<---------[snapshot-access] For this, we'll need to implement snapshot-access API handlers in qcow2 driver, and improve snapshot-access block driver (and API) to make it possible to select snapshot by name. Another thing to improve is size of snapshot. Now for simplicity we just use size of bs->file, which is OK for backup, but for qcow2 snapshots export we'll need to imporve snapshot-access API to get size of snapshot. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20220303194349.2304213-12-vsementsov@virtuozzo.com> [hreitz: Rebased on block GS/IO split] Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- qapi/block-core.json | 4 +- include/block/block_int-common.h | 6 ++ block/snapshot-access.c | 132 +++++++++++++++++++++++++++++++ MAINTAINERS | 1 + block/meson.build | 1 + 5 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 block/snapshot-access.c diff --git a/qapi/block-core.json b/qapi/block-core.json index XXXXXXX..XXXXXXX 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -XXX,XX +XXX,XX @@ # @blkreplay: Since 4.2 # @compress: Since 5.0 # @copy-before-write: Since 6.2 +# @snapshot-access: Since 7.0 # # Since: 2.9 ## { 'enum': 'BlockdevDriver', 'data': [ 'blkdebug', 'blklogwrites', 'blkreplay', 'blkverify', 'bochs', 'cloop', 'compress', 'copy-before-write', 'copy-on-read', 'dmg', - 'file', 'ftp', 'ftps', 'gluster', + 'file', 'snapshot-access', 'ftp', 'ftps', 'gluster', {'name': 'host_cdrom', 'if': 'HAVE_HOST_BLOCK_DEVICE' }, {'name': 'host_device', 'if': 'HAVE_HOST_BLOCK_DEVICE' }, 'http', 'https', 'iscsi', @@ -XXX,XX +XXX,XX @@ 'rbd': 'BlockdevOptionsRbd', 'replication': { 'type': 'BlockdevOptionsReplication', 'if': 'CONFIG_REPLICATION' }, + 'snapshot-access': 'BlockdevOptionsGenericFormat', 'ssh': 'BlockdevOptionsSsh', 'throttle': 'BlockdevOptionsThrottle', 'vdi': 'BlockdevOptionsGenericFormat', diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index XXXXXXX..XXXXXXX 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -XXX,XX +XXX,XX @@ struct BlockDriver { * in generic block-layer: no serializing, no alignment, no tracked * requests. So, block-driver that realizes these APIs is fully responsible * for synchronization between snapshot-access API and normal IO requests. + * + * TODO: To be able to support qcow2's internal snapshots, this API will + * need to be extended to: + * - be able to select a specific snapshot + * - receive the snapshot's actual length (which may differ from bs's + * length) */ int coroutine_fn (*bdrv_co_preadv_snapshot)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset); diff --git a/block/snapshot-access.c b/block/snapshot-access.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/block/snapshot-access.c @@ -XXX,XX +XXX,XX @@ +/* + * snapshot_access block driver + * + * Copyright (c) 2022 Virtuozzo International GmbH. + * + * Author: + * Sementsov-Ogievskiy Vladimir <vsementsov@virtuozzo.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "qemu/osdep.h" + +#include "sysemu/block-backend.h" +#include "qemu/cutils.h" +#include "block/block_int.h" + +static coroutine_fn int +snapshot_access_co_preadv_part(BlockDriverState *bs, + int64_t offset, int64_t bytes, + QEMUIOVector *qiov, size_t qiov_offset, + BdrvRequestFlags flags) +{ + if (flags) { + return -ENOTSUP; + } + + return bdrv_co_preadv_snapshot(bs->file, offset, bytes, qiov, qiov_offset); +} + +static int coroutine_fn +snapshot_access_co_block_status(BlockDriverState *bs, + bool want_zero, int64_t offset, + int64_t bytes, int64_t *pnum, + int64_t *map, BlockDriverState **file) +{ + return bdrv_co_snapshot_block_status(bs->file->bs, want_zero, offset, + bytes, pnum, map, file); +} + +static int coroutine_fn snapshot_access_co_pdiscard(BlockDriverState *bs, + int64_t offset, int64_t bytes) +{ + return bdrv_co_pdiscard_snapshot(bs->file->bs, offset, bytes); +} + +static int coroutine_fn +snapshot_access_co_pwrite_zeroes(BlockDriverState *bs, + int64_t offset, int64_t bytes, + BdrvRequestFlags flags) +{ + return -ENOTSUP; +} + +static coroutine_fn int +snapshot_access_co_pwritev_part(BlockDriverState *bs, + int64_t offset, int64_t bytes, + QEMUIOVector *qiov, size_t qiov_offset, + BdrvRequestFlags flags) +{ + return -ENOTSUP; +} + + +static void snapshot_access_refresh_filename(BlockDriverState *bs) +{ + pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), + bs->file->bs->filename); +} + +static int snapshot_access_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) +{ + bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds, + BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY, + false, errp); + if (!bs->file) { + return -EINVAL; + } + + bs->total_sectors = bs->file->bs->total_sectors; + + return 0; +} + +static void snapshot_access_child_perm(BlockDriverState *bs, BdrvChild *c, + BdrvChildRole role, + BlockReopenQueue *reopen_queue, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + /* + * Currently, we don't need any permissions. If bs->file provides + * snapshot-access API, we can use it. + */ + *nperm = 0; + *nshared = BLK_PERM_ALL; +} + +BlockDriver bdrv_snapshot_access_drv = { + .format_name = "snapshot-access", + + .bdrv_open = snapshot_access_open, + + .bdrv_co_preadv_part = snapshot_access_co_preadv_part, + .bdrv_co_pwritev_part = snapshot_access_co_pwritev_part, + .bdrv_co_pwrite_zeroes = snapshot_access_co_pwrite_zeroes, + .bdrv_co_pdiscard = snapshot_access_co_pdiscard, + .bdrv_co_block_status = snapshot_access_co_block_status, + + .bdrv_refresh_filename = snapshot_access_refresh_filename, + + .bdrv_child_perm = snapshot_access_child_perm, +}; + +static void snapshot_access_init(void) +{ + bdrv_register(&bdrv_snapshot_access_drv); +} + +block_init(snapshot_access_init); diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: block/reqlist.c F: include/block/reqlist.h F: block/copy-before-write.h F: block/copy-before-write.c +F: block/snapshot-access.c F: include/block/aio_task.h F: block/aio_task.c F: util/qemu-co-shared-resource.c diff --git a/block/meson.build b/block/meson.build index XXXXXXX..XXXXXXX 100644 --- a/block/meson.build +++ b/block/meson.build @@ -XXX,XX +XXX,XX @@ block_ss.add(files( 'raw-format.c', 'reqlist.c', 'snapshot.c', + 'snapshot-access.c', 'throttle-groups.c', 'throttle.c', 'vhdx-endian.c', -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Current scheme of image fleecing looks like this: [guest] [NBD export] | | |root | root v v [copy-before-write] -----> [temp.qcow2] | target | |file |backing v | [active disk] <-------------+ - On guest writes copy-before-write filter copies old data from active disk to temp.qcow2. So fleecing client (NBD export) when reads changed regions from temp.qcow2 image and unchanged from active disk through backing link. This patch makes possible new image fleecing scheme: [guest] [NBD export] | | | root | root v file v [copy-before-write]<------[snapshot-access] | | | file | target v v [active-disk] [temp.img] - copy-before-write does CBW operations and also provides snapshot-access API. The API may be accessed through snapshot-access driver. Benefits of new scheme: 1. Access control: if remote client try to read data that not covered by original dirty bitmap used on copy-before-write open, client gets -EACCES. 2. Discard support: if remote client do DISCARD, this additionally to discarding data in temp.img informs block-copy process to not copy these clusters. Next read from discarded area will return -EACCES. This is significant thing: when fleecing user reads data that was not yet copied to temp.img, we can avoid copying it on further guest write. 3. Synchronisation between client reads and block-copy write is more efficient. In old scheme we just rely on BDRV_REQ_SERIALISING flag used for writes to temp.qcow2. New scheme is less blocking: - fleecing reads are never blocked: if data region is untouched or in-flight, we just read from active-disk, otherwise we read from temp.img - writes to temp.img are not blocked by fleecing reads - still, guest writes of-course are blocked by in-flight fleecing reads, that currently read from active-disk - it's the minimum necessary blocking 4. Temporary image may be of any format, as we don't rely on backing feature. 5. Permission relation are simplified. With old scheme we have to share write permission on target child of copy-before-write, otherwise backing link conflicts with copy-before-write file child write permissions. With new scheme we don't have backing link, and copy-before-write node may have unshared access to temporary node. (Not realized in this commit, will be in future). 6. Having control on fleecing reads we'll be able to implement alternative behavior on failed copy-before-write operations. Currently we just break guest request (that's a historical behavior of backup). But in some scenarios it's a bad behavior: better is to drop the backup as failed but don't break guest request. With new scheme we can simply unset some bits in a bitmap on CBW failure and further fleecing reads will -EACCES, or something like this. (Not implemented in this commit, will be in future) Additional application for this is implementing timeout for CBW operations. Iotest 257 output is updated, as two more bitmaps now live in copy-before-write filter. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20220303194349.2304213-13-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- block/copy-before-write.c | 212 ++++++++++++++++++++++++++++++++++- tests/qemu-iotests/257.out | 224 +++++++++++++++++++++++++++++++++++++ 2 files changed, 435 insertions(+), 1 deletion(-) diff --git a/block/copy-before-write.c b/block/copy-before-write.c index XXXXXXX..XXXXXXX 100644 --- a/block/copy-before-write.c +++ b/block/copy-before-write.c @@ -XXX,XX +XXX,XX @@ #include "block/block-copy.h" #include "block/copy-before-write.h" +#include "block/reqlist.h" #include "qapi/qapi-visit-block-core.h" typedef struct BDRVCopyBeforeWriteState { BlockCopyState *bcs; BdrvChild *target; + + /* + * @lock: protects access to @access_bitmap, @done_bitmap and + * @frozen_read_reqs + */ + CoMutex lock; + + /* + * @access_bitmap: represents areas allowed for reading by fleecing user. + * Reading from non-dirty areas leads to -EACCES. + */ + BdrvDirtyBitmap *access_bitmap; + + /* + * @done_bitmap: represents areas that was successfully copied to @target by + * copy-before-write operations. + */ + BdrvDirtyBitmap *done_bitmap; + + /* + * @frozen_read_reqs: current read requests for fleecing user in bs->file + * node. These areas must not be rewritten by guest. + */ + BlockReqList frozen_read_reqs; } BDRVCopyBeforeWriteState; static coroutine_fn int cbw_co_preadv( @@ -XXX,XX +XXX,XX @@ static coroutine_fn int cbw_co_preadv( return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); } +/* + * Do copy-before-write operation. + * + * On failure guest request must be failed too. + * + * On success, we also wait for all in-flight fleecing read requests in source + * node, and it's guaranteed that after cbw_do_copy_before_write() successful + * return there are no such requests and they will never appear. + */ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs, uint64_t offset, uint64_t bytes, BdrvRequestFlags flags) { BDRVCopyBeforeWriteState *s = bs->opaque; + int ret; uint64_t off, end; int64_t cluster_size = block_copy_cluster_size(s->bcs); @@ -XXX,XX +XXX,XX @@ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs, off = QEMU_ALIGN_DOWN(offset, cluster_size); end = QEMU_ALIGN_UP(offset + bytes, cluster_size); - return block_copy(s->bcs, off, end - off, true); + ret = block_copy(s->bcs, off, end - off, true); + if (ret < 0) { + return ret; + } + + WITH_QEMU_LOCK_GUARD(&s->lock) { + bdrv_set_dirty_bitmap(s->done_bitmap, off, end - off); + reqlist_wait_all(&s->frozen_read_reqs, off, end - off, &s->lock); + } + + return 0; } static int coroutine_fn cbw_co_pdiscard(BlockDriverState *bs, @@ -XXX,XX +XXX,XX @@ static int coroutine_fn cbw_co_flush(BlockDriverState *bs) return bdrv_co_flush(bs->file->bs); } +/* + * If @offset not accessible - return NULL. + * + * Otherwise, set @pnum to some bytes that accessible from @file (@file is set + * to bs->file or to s->target). Return newly allocated BlockReq object that + * should be than passed to cbw_snapshot_read_unlock(). + * + * It's guaranteed that guest writes will not interact in the region until + * cbw_snapshot_read_unlock() called. + */ +static BlockReq *cbw_snapshot_read_lock(BlockDriverState *bs, + int64_t offset, int64_t bytes, + int64_t *pnum, BdrvChild **file) +{ + BDRVCopyBeforeWriteState *s = bs->opaque; + BlockReq *req = g_new(BlockReq, 1); + bool done; + + QEMU_LOCK_GUARD(&s->lock); + + if (bdrv_dirty_bitmap_next_zero(s->access_bitmap, offset, bytes) != -1) { + g_free(req); + return NULL; + } + + done = bdrv_dirty_bitmap_status(s->done_bitmap, offset, bytes, pnum); + if (done) { + /* + * Special invalid BlockReq, that is handled in + * cbw_snapshot_read_unlock(). We don't need to lock something to read + * from s->target. + */ + *req = (BlockReq) {.offset = -1, .bytes = -1}; + *file = s->target; + } else { + reqlist_init_req(&s->frozen_read_reqs, req, offset, bytes); + *file = bs->file; + } + + return req; +} + +static void cbw_snapshot_read_unlock(BlockDriverState *bs, BlockReq *req) +{ + BDRVCopyBeforeWriteState *s = bs->opaque; + + if (req->offset == -1 && req->bytes == -1) { + g_free(req); + return; + } + + QEMU_LOCK_GUARD(&s->lock); + + reqlist_remove_req(req); + g_free(req); +} + +static coroutine_fn int +cbw_co_preadv_snapshot(BlockDriverState *bs, int64_t offset, int64_t bytes, + QEMUIOVector *qiov, size_t qiov_offset) +{ + BlockReq *req; + BdrvChild *file; + int ret; + + /* TODO: upgrade to async loop using AioTask */ + while (bytes) { + int64_t cur_bytes; + + req = cbw_snapshot_read_lock(bs, offset, bytes, &cur_bytes, &file); + if (!req) { + return -EACCES; + } + + ret = bdrv_co_preadv_part(file, offset, cur_bytes, + qiov, qiov_offset, 0); + cbw_snapshot_read_unlock(bs, req); + if (ret < 0) { + return ret; + } + + bytes -= cur_bytes; + offset += cur_bytes; + qiov_offset += cur_bytes; + } + + return 0; +} + +static int coroutine_fn +cbw_co_snapshot_block_status(BlockDriverState *bs, + bool want_zero, int64_t offset, int64_t bytes, + int64_t *pnum, int64_t *map, + BlockDriverState **file) +{ + BDRVCopyBeforeWriteState *s = bs->opaque; + BlockReq *req; + int ret; + int64_t cur_bytes; + BdrvChild *child; + + req = cbw_snapshot_read_lock(bs, offset, bytes, &cur_bytes, &child); + if (!req) { + return -EACCES; + } + + ret = bdrv_block_status(child->bs, offset, cur_bytes, pnum, map, file); + if (child == s->target) { + /* + * We refer to s->target only for areas that we've written to it. + * And we can not report unallocated blocks in s->target: this will + * break generic block-status-above logic, that will go to + * copy-before-write filtered child in this case. + */ + assert(ret & BDRV_BLOCK_ALLOCATED); + } + + cbw_snapshot_read_unlock(bs, req); + + return ret; +} + +static int coroutine_fn cbw_co_pdiscard_snapshot(BlockDriverState *bs, + int64_t offset, int64_t bytes) +{ + BDRVCopyBeforeWriteState *s = bs->opaque; + + WITH_QEMU_LOCK_GUARD(&s->lock) { + bdrv_reset_dirty_bitmap(s->access_bitmap, offset, bytes); + } + + block_copy_reset(s->bcs, offset, bytes); + + return bdrv_co_pdiscard(s->target, offset, bytes); +} + static void cbw_refresh_filename(BlockDriverState *bs) { pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), @@ -XXX,XX +XXX,XX @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags, { BDRVCopyBeforeWriteState *s = bs->opaque; BdrvDirtyBitmap *bitmap = NULL; + int64_t cluster_size; bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds, BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, @@ -XXX,XX +XXX,XX @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags, return -EINVAL; } + cluster_size = block_copy_cluster_size(s->bcs); + + s->done_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp); + if (!s->done_bitmap) { + return -EINVAL; + } + bdrv_disable_dirty_bitmap(s->done_bitmap); + + /* s->access_bitmap starts equal to bcs bitmap */ + s->access_bitmap = bdrv_create_dirty_bitmap(bs, cluster_size, NULL, errp); + if (!s->access_bitmap) { + return -EINVAL; + } + bdrv_disable_dirty_bitmap(s->access_bitmap); + bdrv_dirty_bitmap_merge_internal(s->access_bitmap, + block_copy_dirty_bitmap(s->bcs), NULL, + true); + + qemu_co_mutex_init(&s->lock); + QLIST_INIT(&s->frozen_read_reqs); + return 0; } @@ -XXX,XX +XXX,XX @@ static void cbw_close(BlockDriverState *bs) { BDRVCopyBeforeWriteState *s = bs->opaque; + bdrv_release_dirty_bitmap(s->access_bitmap); + bdrv_release_dirty_bitmap(s->done_bitmap); + block_copy_state_free(s->bcs); s->bcs = NULL; } @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_cbw_filter = { .bdrv_co_pdiscard = cbw_co_pdiscard, .bdrv_co_flush = cbw_co_flush, + .bdrv_co_preadv_snapshot = cbw_co_preadv_snapshot, + .bdrv_co_pdiscard_snapshot = cbw_co_pdiscard_snapshot, + .bdrv_co_snapshot_block_status = cbw_co_snapshot_block_status, + .bdrv_refresh_filename = cbw_refresh_filename, .bdrv_child_perm = cbw_child_perm, diff --git a/tests/qemu-iotests/257.out b/tests/qemu-iotests/257.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/257.out +++ b/tests/qemu-iotests/257.out @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, @@ -XXX,XX +XXX,XX @@ write -P0x67 0x3fe0000 0x20000 {"return": ""} { "bitmaps": { + "backup-top": [ + { + "busy": false, + "count": 67108864, + "granularity": 65536, + "persistent": false, + "recording": false + }, + { + "busy": false, + "count": 458752, + "granularity": 65536, + "persistent": false, + "recording": false + } + ], "drive0": [ { "busy": false, -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Hanna Reitz <hreitz@redhat.com> Message-Id: <20220303194349.2304213-14-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- tests/qemu-iotests/tests/image-fleecing | 64 +++++++++++++----- tests/qemu-iotests/tests/image-fleecing.out | 74 ++++++++++++++++++++- 2 files changed, 119 insertions(+), 19 deletions(-) diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing index XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/tests/image-fleecing +++ b/tests/qemu-iotests/tests/image-fleecing @@ -XXX,XX +XXX,XX @@ remainder = [('0xd5', '0x108000', '32k'), # Right-end of partial-left [1] ('0xdc', '32M', '32k'), # Left-end of partial-right [2] ('0xcd', '0x3ff0000', '64k')] # patterns[3] -def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): +def do_test(use_cbw, use_snapshot_access_filter, base_img_path, + fleece_img_path, nbd_sock_path, vm): log('--- Setting up images ---') log('') assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0 - assert qemu_img('create', '-f', 'qcow2', fleece_img_path, '64M') == 0 + if use_snapshot_access_filter: + assert use_cbw + assert qemu_img('create', '-f', 'raw', fleece_img_path, '64M') == 0 + else: + assert qemu_img('create', '-f', 'qcow2', fleece_img_path, '64M') == 0 for p in patterns: qemu_io('-f', iotests.imgfmt, @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): log('') - # create tmp_node backed by src_node - log(vm.qmp('blockdev-add', { - 'driver': 'qcow2', - 'node-name': tmp_node, - 'file': { + if use_snapshot_access_filter: + log(vm.qmp('blockdev-add', { + 'node-name': tmp_node, 'driver': 'file', 'filename': fleece_img_path, - }, - 'backing': src_node, - })) + })) + else: + # create tmp_node backed by src_node + log(vm.qmp('blockdev-add', { + 'driver': 'qcow2', + 'node-name': tmp_node, + 'file': { + 'driver': 'file', + 'filename': fleece_img_path, + }, + 'backing': src_node, + })) # Establish CBW from source to fleecing node if use_cbw: @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): })) log(vm.qmp('qom-set', path=qom_path, property='drive', value='fl-cbw')) + + if use_snapshot_access_filter: + log(vm.qmp('blockdev-add', { + 'driver': 'snapshot-access', + 'node-name': 'fl-access', + 'file': 'fl-cbw', + })) else: log(vm.qmp('blockdev-backup', job_id='fleecing', @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): target=tmp_node, sync='none')) + export_node = 'fl-access' if use_snapshot_access_filter else tmp_node + log('') log('--- Setting up NBD Export ---') log('') - nbd_uri = 'nbd+unix:///%s?socket=%s' % (tmp_node, nbd_sock_path) + nbd_uri = 'nbd+unix:///%s?socket=%s' % (export_node, nbd_sock_path) log(vm.qmp('nbd-server-start', {'addr': {'type': 'unix', 'data': {'path': nbd_sock_path}}})) - log(vm.qmp('nbd-server-add', device=tmp_node)) + log(vm.qmp('nbd-server-add', device=export_node)) log('') log('--- Sanity Check ---') @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): log('--- Cleanup ---') log('') + log(vm.qmp('nbd-server-stop')) + if use_cbw: + if use_snapshot_access_filter: + log(vm.qmp('blockdev-del', node_name='fl-access')) log(vm.qmp('qom-set', path=qom_path, property='drive', value=src_node)) log(vm.qmp('blockdev-del', node_name='fl-cbw')) else: @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): assert e is not None log(e, filters=[iotests.filter_qmp_event]) - log(vm.qmp('nbd-server-stop')) log(vm.qmp('blockdev-del', node_name=tmp_node)) vm.shutdown() @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): log('Done') -def test(use_cbw): +def test(use_cbw, use_snapshot_access_filter): with iotests.FilePath('base.img') as base_img_path, \ iotests.FilePath('fleece.img') as fleece_img_path, \ iotests.FilePath('nbd.sock', base_dir=iotests.sock_dir) as nbd_sock_path, \ iotests.VM() as vm: - do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm) + do_test(use_cbw, use_snapshot_access_filter, base_img_path, + fleece_img_path, nbd_sock_path, vm) log('=== Test backup(sync=none) based fleecing ===\n') -test(False) +test(False, False) + +log('=== Test cbw-filter based fleecing ===\n') +test(True, False) -log('=== Test filter based fleecing ===\n') -test(True) +log('=== Test fleecing-format based fleecing ===\n') +test(True, True) diff --git a/tests/qemu-iotests/tests/image-fleecing.out b/tests/qemu-iotests/tests/image-fleecing.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/tests/image-fleecing.out +++ b/tests/qemu-iotests/tests/image-fleecing.out @@ -XXX,XX +XXX,XX @@ read -P0 0x3fe0000 64k --- Cleanup --- +{"return": {}} {"return": {}} {"data": {"device": "fleecing", "len": 67108864, "offset": 393216, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_CANCELLED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} {"return": {}} + +--- Confirming writes --- + +read -P0xab 0 64k +read -P0xad 0x00f8000 64k +read -P0x1d 0x2008000 64k +read -P0xea 0x3fe0000 64k +read -P0xd5 0x108000 32k +read -P0xdc 32M 32k +read -P0xcd 0x3ff0000 64k + +Done +=== Test cbw-filter based fleecing === + +--- Setting up images --- + +Done + +--- Launching VM --- + +Done + +--- Setting up Fleecing Graph --- + +{"return": {}} +{"return": {}} +{"return": {}} + +--- Setting up NBD Export --- + +{"return": {}} +{"return": {}} + +--- Sanity Check --- + +read -P0x5d 0 64k +read -P0xd5 1M 64k +read -P0xdc 32M 64k +read -P0xcd 0x3ff0000 64k +read -P0 0x00f8000 32k +read -P0 0x2010000 32k +read -P0 0x3fe0000 64k + +--- Testing COW --- + +write -P0xab 0 64k +{"return": ""} +write -P0xad 0x00f8000 64k +{"return": ""} +write -P0x1d 0x2008000 64k +{"return": ""} +write -P0xea 0x3fe0000 64k +{"return": ""} + +--- Verifying Data --- + +read -P0x5d 0 64k +read -P0xd5 1M 64k +read -P0xdc 32M 64k +read -P0xcd 0x3ff0000 64k +read -P0 0x00f8000 32k +read -P0 0x2010000 32k +read -P0 0x3fe0000 64k + +--- Cleanup --- + +{"return": {}} +{"return": {}} +{"return": {}} {"return": {}} --- Confirming writes --- @@ -XXX,XX +XXX,XX @@ read -P0xdc 32M 32k read -P0xcd 0x3ff0000 64k Done -=== Test filter based fleecing === +=== Test fleecing-format based fleecing === --- Setting up images --- @@ -XXX,XX +XXX,XX @@ Done {"return": {}} {"return": {}} {"return": {}} +{"return": {}} --- Setting up NBD Export --- @@ -XXX,XX +XXX,XX @@ read -P0 0x3fe0000 64k {"return": {}} {"return": {}} {"return": {}} +{"return": {}} --- Confirming writes --- -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Add helper that returns both status and output, to be used in the following commit Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20220303194349.2304213-15-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- tests/qemu-iotests/iotests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -XXX,XX +XXX,XX @@ def qemu_io(*args): '''Run qemu-io and return the stdout data''' return qemu_tool_pipe_and_status('qemu-io', qemu_io_wrap_args(args))[0] +def qemu_io_pipe_and_status(*args): + return qemu_tool_pipe_and_status('qemu-io', qemu_io_wrap_args(args)) + def qemu_io_log(*args): result = qemu_io(*args) log(result, filters=[filter_testfiles, filter_qemu_io]) -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Note that reads zero areas (not dirty in the bitmap) fails, that's correct. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20220303194349.2304213-16-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- tests/qemu-iotests/tests/image-fleecing | 38 +++++++--- tests/qemu-iotests/tests/image-fleecing.out | 84 +++++++++++++++++++++ 2 files changed, 113 insertions(+), 9 deletions(-) diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing index XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/tests/image-fleecing +++ b/tests/qemu-iotests/tests/image-fleecing @@ -XXX,XX +XXX,XX @@ # Creator/Owner: John Snow <jsnow@redhat.com> import iotests -from iotests import log, qemu_img, qemu_io, qemu_io_silent +from iotests import log, qemu_img, qemu_io, qemu_io_silent, \ + qemu_io_pipe_and_status iotests.script_initialize( - supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk', 'vhdx', 'raw'], + supported_fmts=['qcow2'], supported_platforms=['linux'], required_fmts=['copy-before-write'], + unsupported_imgopts=['compat'] ) patterns = [('0x5d', '0', '64k'), @@ -XXX,XX +XXX,XX @@ remainder = [('0xd5', '0x108000', '32k'), # Right-end of partial-left [1] ('0xcd', '0x3ff0000', '64k')] # patterns[3] def do_test(use_cbw, use_snapshot_access_filter, base_img_path, - fleece_img_path, nbd_sock_path, vm): + fleece_img_path, nbd_sock_path, vm, + bitmap=False): log('--- Setting up images ---') log('') assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0 + if bitmap: + assert qemu_img('bitmap', '--add', base_img_path, 'bitmap0') == 0 + if use_snapshot_access_filter: assert use_cbw assert qemu_img('create', '-f', 'raw', fleece_img_path, '64M') == 0 @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, # Establish CBW from source to fleecing node if use_cbw: - log(vm.qmp('blockdev-add', { + fl_cbw = { 'driver': 'copy-before-write', 'node-name': 'fl-cbw', 'file': src_node, 'target': tmp_node - })) + } + + if bitmap: + fl_cbw['bitmap'] = {'node': src_node, 'name': 'bitmap0'} + + log(vm.qmp('blockdev-add', fl_cbw)) log(vm.qmp('qom-set', path=qom_path, property='drive', value='fl-cbw')) @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, for p in patterns + zeroes: cmd = 'read -P%s %s %s' % p log(cmd) - assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0 + out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, + nbd_uri) + if ret != 0: + print(out) log('') log('--- Testing COW ---') @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, for p in patterns + zeroes: cmd = 'read -P%s %s %s' % p log(cmd) - assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0 + out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, + nbd_uri) + if ret != 0: + print(out) log('') log('--- Cleanup ---') @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, log('Done') -def test(use_cbw, use_snapshot_access_filter): +def test(use_cbw, use_snapshot_access_filter, bitmap=False): with iotests.FilePath('base.img') as base_img_path, \ iotests.FilePath('fleece.img') as fleece_img_path, \ iotests.FilePath('nbd.sock', base_dir=iotests.sock_dir) as nbd_sock_path, \ iotests.VM() as vm: do_test(use_cbw, use_snapshot_access_filter, base_img_path, - fleece_img_path, nbd_sock_path, vm) + fleece_img_path, nbd_sock_path, vm, bitmap=bitmap) log('=== Test backup(sync=none) based fleecing ===\n') @@ -XXX,XX +XXX,XX @@ test(True, False) log('=== Test fleecing-format based fleecing ===\n') test(True, True) + +log('=== Test fleecing-format based fleecing with bitmap ===\n') +test(True, True, bitmap=True) diff --git a/tests/qemu-iotests/tests/image-fleecing.out b/tests/qemu-iotests/tests/image-fleecing.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/tests/image-fleecing.out +++ b/tests/qemu-iotests/tests/image-fleecing.out @@ -XXX,XX +XXX,XX @@ read -P0 0x00f8000 32k read -P0 0x2010000 32k read -P0 0x3fe0000 64k +--- Cleanup --- + +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} + +--- Confirming writes --- + +read -P0xab 0 64k +read -P0xad 0x00f8000 64k +read -P0x1d 0x2008000 64k +read -P0xea 0x3fe0000 64k +read -P0xd5 0x108000 32k +read -P0xdc 32M 32k +read -P0xcd 0x3ff0000 64k + +Done +=== Test fleecing-format based fleecing with bitmap === + +--- Setting up images --- + +Done + +--- Launching VM --- + +Done + +--- Setting up Fleecing Graph --- + +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} + +--- Setting up NBD Export --- + +{"return": {}} +{"return": {}} + +--- Sanity Check --- + +read -P0x5d 0 64k +read -P0xd5 1M 64k +read -P0xdc 32M 64k +read -P0xcd 0x3ff0000 64k +read -P0 0x00f8000 32k +read failed: Invalid argument + +read -P0 0x2010000 32k +read failed: Invalid argument + +read -P0 0x3fe0000 64k +read failed: Invalid argument + + +--- Testing COW --- + +write -P0xab 0 64k +{"return": ""} +write -P0xad 0x00f8000 64k +{"return": ""} +write -P0x1d 0x2008000 64k +{"return": ""} +write -P0xea 0x3fe0000 64k +{"return": ""} + +--- Verifying Data --- + +read -P0x5d 0 64k +read -P0xd5 1M 64k +read -P0xdc 32M 64k +read -P0xcd 0x3ff0000 64k +read -P0 0x00f8000 32k +read failed: Invalid argument + +read -P0 0x2010000 32k +read failed: Invalid argument + +read -P0 0x3fe0000 64k +read failed: Invalid argument + + --- Cleanup --- {"return": {}} -- 2.34.1 From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20220303194349.2304213-17-vsementsov@virtuozzo.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- tests/qemu-iotests/tests/image-fleecing | 125 +++++++++++++++----- tests/qemu-iotests/tests/image-fleecing.out | 63 ++++++++++ 2 files changed, 156 insertions(+), 32 deletions(-) diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing index XXXXXXX..XXXXXXX 100755 --- a/tests/qemu-iotests/tests/image-fleecing +++ b/tests/qemu-iotests/tests/image-fleecing @@ -XXX,XX +XXX,XX @@ remainder = [('0xd5', '0x108000', '32k'), # Right-end of partial-left [1] ('0xdc', '32M', '32k'), # Left-end of partial-right [2] ('0xcd', '0x3ff0000', '64k')] # patterns[3] -def do_test(use_cbw, use_snapshot_access_filter, base_img_path, - fleece_img_path, nbd_sock_path, vm, +def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path, + fleece_img_path, nbd_sock_path=None, + target_img_path=None, bitmap=False): + push_backup = target_img_path is not None + assert (nbd_sock_path is not None) != push_backup + if push_backup: + assert use_cbw + log('--- Setting up images ---') log('') @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, else: assert qemu_img('create', '-f', 'qcow2', fleece_img_path, '64M') == 0 + if push_backup: + assert qemu_img('create', '-f', 'qcow2', target_img_path, '64M') == 0 + for p in patterns: qemu_io('-f', iotests.imgfmt, '-c', 'write -P%s %s %s' % p, base_img_path) @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, export_node = 'fl-access' if use_snapshot_access_filter else tmp_node - log('') - log('--- Setting up NBD Export ---') - log('') + if push_backup: + log('') + log('--- Starting actual backup ---') + log('') - nbd_uri = 'nbd+unix:///%s?socket=%s' % (export_node, nbd_sock_path) - log(vm.qmp('nbd-server-start', - {'addr': {'type': 'unix', - 'data': {'path': nbd_sock_path}}})) + log(vm.qmp('blockdev-add', **{ + 'driver': iotests.imgfmt, + 'node-name': 'target', + 'file': { + 'driver': 'file', + 'filename': target_img_path + } + })) + log(vm.qmp('blockdev-backup', device=export_node, + sync='full', target='target', + job_id='push-backup', speed=1)) + else: + log('') + log('--- Setting up NBD Export ---') + log('') - log(vm.qmp('nbd-server-add', device=export_node)) + nbd_uri = 'nbd+unix:///%s?socket=%s' % (export_node, nbd_sock_path) + log(vm.qmp('nbd-server-start', + {'addr': { 'type': 'unix', + 'data': { 'path': nbd_sock_path } } })) - log('') - log('--- Sanity Check ---') - log('') + log(vm.qmp('nbd-server-add', device=export_node)) - for p in patterns + zeroes: - cmd = 'read -P%s %s %s' % p - log(cmd) - out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, - nbd_uri) - if ret != 0: - print(out) + log('') + log('--- Sanity Check ---') + log('') + + for p in patterns + zeroes: + cmd = 'read -P%s %s %s' % p + log(cmd) + out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, + nbd_uri) + if ret != 0: + print(out) log('') log('--- Testing COW ---') @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, log(cmd) log(vm.hmp_qemu_io(qom_path, cmd, qdev=True)) + if push_backup: + # Check that previous operations were done during backup, not after + # If backup is already finished, it's possible that it was finished + # even before hmp qemu_io write, and we didn't actually test + # copy-before-write operation. This should not happen, as we use + # speed=1. But worth checking. + result = vm.qmp('query-block-jobs') + assert len(result['return']) == 1 + + result = vm.qmp('block-job-set-speed', device='push-backup', speed=0) + assert result == {'return': {}} + + log(vm.event_wait(name='BLOCK_JOB_COMPLETED', + match={'data': {'device': 'push-backup'}}), + filters=[iotests.filter_qmp_event]) + log(vm.qmp('blockdev-del', node_name='target')) + log('') log('--- Verifying Data ---') log('') @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, for p in patterns + zeroes: cmd = 'read -P%s %s %s' % p log(cmd) - out, ret = qemu_io_pipe_and_status('-r', '-f', 'raw', '-c', cmd, - nbd_uri) + args = ['-r', '-c', cmd] + if push_backup: + args += [target_img_path] + else: + args += ['-f', 'raw', nbd_uri] + out, ret = qemu_io_pipe_and_status(*args) if ret != 0: print(out) @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, log('--- Cleanup ---') log('') - log(vm.qmp('nbd-server-stop')) + if not push_backup: + log(vm.qmp('nbd-server-stop')) if use_cbw: if use_snapshot_access_filter: @@ -XXX,XX +XXX,XX @@ def do_test(use_cbw, use_snapshot_access_filter, base_img_path, log('Done') -def test(use_cbw, use_snapshot_access_filter, bitmap=False): +def test(use_cbw, use_snapshot_access_filter, + nbd_sock_path=None, target_img_path=None, bitmap=False): with iotests.FilePath('base.img') as base_img_path, \ iotests.FilePath('fleece.img') as fleece_img_path, \ - iotests.FilePath('nbd.sock', - base_dir=iotests.sock_dir) as nbd_sock_path, \ iotests.VM() as vm: - do_test(use_cbw, use_snapshot_access_filter, base_img_path, - fleece_img_path, nbd_sock_path, vm, bitmap=bitmap) + do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path, + fleece_img_path, nbd_sock_path, target_img_path, + bitmap=bitmap) + +def test_pull(use_cbw, use_snapshot_access_filter, bitmap=False): + with iotests.FilePath('nbd.sock', + base_dir=iotests.sock_dir) as nbd_sock_path: + test(use_cbw, use_snapshot_access_filter, nbd_sock_path, None, + bitmap=bitmap) + +def test_push(): + with iotests.FilePath('target.img') as target_img_path: + test(True, True, None, target_img_path) log('=== Test backup(sync=none) based fleecing ===\n') -test(False, False) +test_pull(False, False) log('=== Test cbw-filter based fleecing ===\n') -test(True, False) +test_pull(True, False) log('=== Test fleecing-format based fleecing ===\n') -test(True, True) +test_pull(True, True) log('=== Test fleecing-format based fleecing with bitmap ===\n') -test(True, True, bitmap=True) +test_pull(True, True, bitmap=True) + +log('=== Test push backup with fleecing ===\n') +test_push() diff --git a/tests/qemu-iotests/tests/image-fleecing.out b/tests/qemu-iotests/tests/image-fleecing.out index XXXXXXX..XXXXXXX 100644 --- a/tests/qemu-iotests/tests/image-fleecing.out +++ b/tests/qemu-iotests/tests/image-fleecing.out @@ -XXX,XX +XXX,XX @@ read -P0xdc 32M 32k read -P0xcd 0x3ff0000 64k Done +=== Test push backup with fleecing === + +--- Setting up images --- + +Done + +--- Launching VM --- + +Done + +--- Setting up Fleecing Graph --- + +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} + +--- Starting actual backup --- + +{"return": {}} +{"return": {}} + +--- Testing COW --- + +write -P0xab 0 64k +{"return": ""} +write -P0xad 0x00f8000 64k +{"return": ""} +write -P0x1d 0x2008000 64k +{"return": ""} +write -P0xea 0x3fe0000 64k +{"return": ""} +{"data": {"device": "push-backup", "len": 67108864, "offset": 67108864, "speed": 0, "type": "backup"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"return": {}} + +--- Verifying Data --- + +read -P0x5d 0 64k +read -P0xd5 1M 64k +read -P0xdc 32M 64k +read -P0xcd 0x3ff0000 64k +read -P0 0x00f8000 32k +read -P0 0x2010000 32k +read -P0 0x3fe0000 64k + +--- Cleanup --- + +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} + +--- Confirming writes --- + +read -P0xab 0 64k +read -P0xad 0x00f8000 64k +read -P0x1d 0x2008000 64k +read -P0xea 0x3fe0000 64k +read -P0xd5 0x108000 32k +read -P0xdc 32M 32k +read -P0xcd 0x3ff0000 64k + +Done -- 2.34.1