From dc71ce45ded4e872e25c2de32d5e7a71842b0985 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 24 Jun 2014 20:26:35 +0800 Subject: blockjob: Add block_job_yield() This will unset busy flag and put coroutine to sleep, can be used to wait for QMP complete/cancel. Signed-off-by: Fam Zheng Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- include/block/blockjob.h | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'include') diff --git a/include/block/blockjob.h b/include/block/blockjob.h index e443987ea8..67ca076380 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -146,6 +146,14 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, */ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns); +/** + * block_job_yield: + * @job: The job that calls the function. + * + * Yield the block job coroutine. + */ +void block_job_yield(BlockJob *job); + /** * block_job_completed: * @job: The job being completed. -- cgit v1.2.3-55-g7522 From 8ee79e707a005c9274df7ce34265bb7d008b8cef Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 4 Jun 2014 15:09:35 +0200 Subject: block: Catch backing files assigned to non-COW drivers Since we parse backing.* options to add a backing file from the command line when the driver didn't assign one, it has been possible to have a backing file for e.g. raw images (it just was never accessed). This is obvious nonsense and should be rejected. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 7 +++++++ block/cow.c | 1 + block/qcow.c | 1 + block/qcow2.c | 1 + block/qed.c | 1 + block/vmdk.c | 1 + include/block/block_int.h | 3 +++ tests/qemu-iotests/051 | 5 +++++ tests/qemu-iotests/051.out | 9 +++++++++ 9 files changed, 29 insertions(+) (limited to 'include') diff --git a/block.c b/block.c index 0978e8cd59..2c3c7608ee 100644 --- a/block.c +++ b/block.c @@ -1192,6 +1192,13 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) bdrv_get_full_backing_filename(bs, backing_filename, PATH_MAX); } + if (!bs->drv || !bs->drv->supports_backing) { + ret = -EINVAL; + error_setg(errp, "Driver doesn't support backing files"); + QDECREF(options); + goto free_exit; + } + backing_hd = bdrv_new("", errp); if (bs->backing_format[0] != '\0') { diff --git a/block/cow.c b/block/cow.c index a05a92cada..8f81ee6d56 100644 --- a/block/cow.c +++ b/block/cow.c @@ -414,6 +414,7 @@ static BlockDriver bdrv_cow = { .bdrv_close = cow_close, .bdrv_create = cow_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, + .supports_backing = true, .bdrv_read = cow_co_read, .bdrv_write = cow_co_write, diff --git a/block/qcow.c b/block/qcow.c index 1f2bac8a5f..a874056cf3 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -941,6 +941,7 @@ static BlockDriver bdrv_qcow = { .bdrv_reopen_prepare = qcow_reopen_prepare, .bdrv_create = qcow_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, + .supports_backing = true, .bdrv_co_readv = qcow_co_readv, .bdrv_co_writev = qcow_co_writev, diff --git a/block/qcow2.c b/block/qcow2.c index b9d2fa6632..67e55c9667 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2418,6 +2418,7 @@ static BlockDriver bdrv_qcow2 = { .bdrv_save_vmstate = qcow2_save_vmstate, .bdrv_load_vmstate = qcow2_load_vmstate, + .supports_backing = true, .bdrv_change_backing_file = qcow2_change_backing_file, .bdrv_refresh_limits = qcow2_refresh_limits, diff --git a/block/qed.c b/block/qed.c index 092e6fb1d2..eddae929eb 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1652,6 +1652,7 @@ static BlockDriver bdrv_qed = { .format_name = "qed", .instance_size = sizeof(BDRVQEDState), .create_opts = &qed_create_opts, + .supports_backing = true, .bdrv_probe = bdrv_qed_probe, .bdrv_rebind = bdrv_qed_rebind, diff --git a/block/vmdk.c b/block/vmdk.c index 83dd6fe4fb..d0de0193fc 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -2180,6 +2180,7 @@ static BlockDriver bdrv_vmdk = { .bdrv_detach_aio_context = vmdk_detach_aio_context, .bdrv_attach_aio_context = vmdk_attach_aio_context, + .supports_backing = true, .create_opts = &vmdk_create_opts, }; diff --git a/include/block/block_int.h b/include/block/block_int.h index 715c761fad..135c5dc0e9 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -100,6 +100,9 @@ struct BlockDriver { */ bool bdrv_needs_filename; + /* Set if a driver can support backing files */ + bool supports_backing; + /* For handling image reopen for split or non-split files */ int (*bdrv_reopen_prepare)(BDRVReopenState *reopen_state, BlockReopenQueue *queue, Error **errp); diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 30a712f847..a41334e022 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -100,6 +100,11 @@ echo echo "info block" | run_qemu -drive file="$TEST_IMG",driver=qcow2,backing.file.filename="$TEST_IMG.orig" -nodefaults +# Drivers that don't support backing files +run_qemu -drive file="$TEST_IMG",driver=raw,backing.file.filename="$TEST_IMG.orig" +run_qemu -drive file="$TEST_IMG",file.backing.driver=file,file.backing.filename="$TEST_IMG.orig" +run_qemu -drive file="$TEST_IMG",file.backing.driver=qcow2,file.backing.file.filename="$TEST_IMG.orig" + echo echo === Enable and disable lazy refcounting on the command line, plus some invalid values === echo diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 94c7107250..d7b0f503af 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -53,6 +53,15 @@ ide0-hd0: TEST_DIR/t.qcow2 (qcow2) Backing file: TEST_DIR/t.qcow2.orig (chain depth: 1) (qemu) qququiquit +Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,backing.file.filename=TEST_DIR/t.qcow2.orig +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,backing.file.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files + +Testing: -drive file=TEST_DIR/t.qcow2,file.backing.driver=file,file.backing.filename=TEST_DIR/t.qcow2.orig +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.backing.driver=file,file.backing.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files + +Testing: -drive file=TEST_DIR/t.qcow2,file.backing.driver=qcow2,file.backing.file.filename=TEST_DIR/t.qcow2.orig +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.backing.driver=qcow2,file.backing.file.filename=TEST_DIR/t.qcow2.orig: could not open disk image TEST_DIR/t.qcow2: Driver doesn't support backing files + === Enable and disable lazy refcounting on the command line, plus some invalid values === -- cgit v1.2.3-55-g7522 From 09f645877037590415016b59f6d32be1a27229c6 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:42 +0800 Subject: virtio-blk: Move VirtIOBlockReq to header For later reusing by dataplane code. Signed-off-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 11 ----------- include/hw/virtio/virtio-blk.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'include') diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 08562ea390..4ff64870bc 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -27,17 +27,6 @@ #endif #include "hw/virtio/virtio-bus.h" -typedef struct VirtIOBlockReq -{ - VirtIOBlock *dev; - VirtQueueElement elem; - struct virtio_blk_inhdr *in; - struct virtio_blk_outhdr *out; - QEMUIOVector qiov; - struct VirtIOBlockReq *next; - BlockAcctCookie acct; -} VirtIOBlockReq; - static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) { VirtIOBlock *s = req->dev; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 4bc9b549ad..d05d177ec5 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -17,6 +17,7 @@ #include "hw/virtio/virtio.h" #include "hw/block/block.h" #include "sysemu/iothread.h" +#include "block/block.h" #define TYPE_VIRTIO_BLK "virtio-blk-device" #define VIRTIO_BLK(obj) \ @@ -133,6 +134,16 @@ typedef struct VirtIOBlock { #endif } VirtIOBlock; +typedef struct VirtIOBlockReq { + VirtIOBlock *dev; + VirtQueueElement elem; + struct virtio_blk_inhdr *in; + struct virtio_blk_outhdr *out; + QEMUIOVector qiov; + struct VirtIOBlockReq *next; + BlockAcctCookie acct; +} VirtIOBlockReq; + #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \ DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) -- cgit v1.2.3-55-g7522 From 671ec3f056559f22a2531a91dce3a258b9b5eb8a Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:43 +0800 Subject: virtio-blk: Convert VirtIOBlockReq.elem to pointer This will make converging with dataplane code easier. Add virtio_blk_free_request to handle the freeing of request internal fields. Signed-off-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 85 +++++++++++++++++++++++------------------- include/hw/virtio/virtio-blk.h | 2 +- 2 files changed, 47 insertions(+), 40 deletions(-) (limited to 'include') diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 4ff64870bc..b5cc3855cf 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -27,6 +27,22 @@ #endif #include "hw/virtio/virtio-bus.h" +static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) +{ + VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); + req->dev = s; + req->elem = g_slice_new0(VirtQueueElement); + return req; +} + +static void virtio_blk_free_request(VirtIOBlockReq *req) +{ + if (req) { + g_slice_free(VirtQueueElement, req->elem); + g_slice_free(VirtIOBlockReq, req); + } +} + static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) { VirtIOBlock *s = req->dev; @@ -35,7 +51,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) trace_virtio_blk_req_complete(req, status); stb_p(&req->in->status, status); - virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in)); + virtqueue_push(s->vq, req->elem, req->qiov.size + sizeof(*req->in)); virtio_notify(vdev, s->vq); } @@ -51,7 +67,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, } else if (action == BLOCK_ERROR_ACTION_REPORT) { virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); bdrv_acct_done(s->bs, &req->acct); - g_free(req); + virtio_blk_free_request(req); } bdrv_error_action(s->bs, action, is_read, error); @@ -72,7 +88,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret) virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); bdrv_acct_done(req->dev->bs, &req->acct); - g_free(req); + virtio_blk_free_request(req); } static void virtio_blk_flush_complete(void *opaque, int ret) @@ -87,27 +103,16 @@ static void virtio_blk_flush_complete(void *opaque, int ret) virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); bdrv_acct_done(req->dev->bs, &req->acct); - g_free(req); -} - -static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) -{ - VirtIOBlockReq *req = g_malloc(sizeof(*req)); - req->dev = s; - req->qiov.size = 0; - req->next = NULL; - return req; + virtio_blk_free_request(req); } static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s) { VirtIOBlockReq *req = virtio_blk_alloc_request(s); - if (req != NULL) { - if (!virtqueue_pop(s->vq, &req->elem)) { - g_free(req); - return NULL; - } + if (!virtqueue_pop(s->vq, req->elem)) { + virtio_blk_free_request(req); + return NULL; } return req; @@ -236,9 +241,9 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) { int status; - status = virtio_blk_handle_scsi_req(req->dev, &req->elem); + status = virtio_blk_handle_scsi_req(req->dev, req->elem); virtio_blk_req_complete(req, status); - g_free(req); + virtio_blk_free_request(req); } typedef struct MultiReqBuffer { @@ -340,19 +345,19 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, { uint32_t type; - if (req->elem.out_num < 1 || req->elem.in_num < 1) { + if (req->elem->out_num < 1 || req->elem->in_num < 1) { error_report("virtio-blk missing headers"); exit(1); } - if (req->elem.out_sg[0].iov_len < sizeof(*req->out) || - req->elem.in_sg[req->elem.in_num - 1].iov_len < sizeof(*req->in)) { + if (req->elem->out_sg[0].iov_len < sizeof(*req->out) || + req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) { error_report("virtio-blk header not in correct element"); exit(1); } - req->out = (void *)req->elem.out_sg[0].iov_base; - req->in = (void *)req->elem.in_sg[req->elem.in_num - 1].iov_base; + req->out = (void *)req->elem->out_sg[0].iov_base; + req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base; type = ldl_p(&req->out->type); @@ -367,23 +372,23 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, * NB: per existing s/n string convention the string is * terminated by '\0' only when shorter than buffer. */ - strncpy(req->elem.in_sg[0].iov_base, + strncpy(req->elem->in_sg[0].iov_base, s->blk.serial ? s->blk.serial : "", - MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES)); + MIN(req->elem->in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES)); virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); - g_free(req); + virtio_blk_free_request(req); } else if (type & VIRTIO_BLK_T_OUT) { - qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1], - req->elem.out_num - 1); + qemu_iovec_init_external(&req->qiov, &req->elem->out_sg[1], + req->elem->out_num - 1); virtio_blk_handle_write(req, mrb); } else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) { /* VIRTIO_BLK_T_IN is 0, so we can't just & it. */ - qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0], - req->elem.in_num - 1); + qemu_iovec_init_external(&req->qiov, &req->elem->in_sg[0], + req->elem->in_num - 1); virtio_blk_handle_read(req); } else { virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); - g_free(req); + virtio_blk_free_request(req); } } @@ -598,7 +603,8 @@ static void virtio_blk_save(QEMUFile *f, void *opaque) while (req) { qemu_put_sbyte(f, 1); - qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem)); + qemu_put_buffer(f, (unsigned char *)req->elem, + sizeof(VirtQueueElement)); req = req->next; } qemu_put_sbyte(f, 0); @@ -620,14 +626,15 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id) while (qemu_get_sbyte(f)) { VirtIOBlockReq *req = virtio_blk_alloc_request(s); - qemu_get_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem)); + qemu_get_buffer(f, (unsigned char *)req->elem, + sizeof(VirtQueueElement)); req->next = s->rq; s->rq = req; - virtqueue_map_sg(req->elem.in_sg, req->elem.in_addr, - req->elem.in_num, 1); - virtqueue_map_sg(req->elem.out_sg, req->elem.out_addr, - req->elem.out_num, 0); + virtqueue_map_sg(req->elem->in_sg, req->elem->in_addr, + req->elem->in_num, 1); + virtqueue_map_sg(req->elem->out_sg, req->elem->out_addr, + req->elem->out_num, 0); } return 0; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index d05d177ec5..b495e42d6d 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -136,7 +136,7 @@ typedef struct VirtIOBlock { typedef struct VirtIOBlockReq { VirtIOBlock *dev; - VirtQueueElement elem; + VirtQueueElement *elem; struct virtio_blk_inhdr *in; struct virtio_blk_outhdr *out; QEMUIOVector qiov; -- cgit v1.2.3-55-g7522 From 04af2d70c58136be57d798ad684e1924dcde93f5 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:46 +0800 Subject: virtio-blk: Replace VirtIOBlockRequest with VirtIOBlockReq Field "inhdr" is added temporarily for a more mechanical change, and will be dropped in the next commit. Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 34 +++++++++++++++------------------- include/hw/virtio/virtio-blk.h | 4 ++++ 2 files changed, 19 insertions(+), 19 deletions(-) (limited to 'include') diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 4e5e4589bd..70e8c1452b 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -24,13 +24,6 @@ #include "hw/virtio/virtio-bus.h" #include "qom/object_interfaces.h" -typedef struct { - VirtIOBlockDataPlane *s; - QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */ - VirtQueueElement *elem; /* saved data from the virtqueue */ - QEMUIOVector qiov; /* original request iovecs */ -} VirtIOBlockRequest; - struct VirtIOBlockDataPlane { bool started; bool starting; @@ -68,7 +61,7 @@ static void notify_guest(VirtIOBlockDataPlane *s) static void complete_rdwr(void *opaque, int ret) { - VirtIOBlockRequest *req = opaque; + VirtIOBlockReq *req = opaque; struct virtio_blk_inhdr hdr; int len; @@ -80,7 +73,8 @@ static void complete_rdwr(void *opaque, int ret) len = 0; } - trace_virtio_blk_data_plane_complete_request(req->s, req->elem->index, ret); + trace_virtio_blk_data_plane_complete_request(req->dev->dataplane, + req->elem->index, ret); qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr)); qemu_iovec_destroy(req->inhdr); @@ -90,9 +84,9 @@ static void complete_rdwr(void *opaque, int ret) * written to, but for virtio-blk it seems to be the number of bytes * transferred plus the status bytes. */ - vring_push(&req->s->vring, req->elem, len + sizeof(hdr)); - notify_guest(req->s); - g_slice_free(VirtIOBlockRequest, req); + vring_push(&req->dev->dataplane->vring, req->elem, len + sizeof(hdr)); + notify_guest(req->dev->dataplane); + g_slice_free(VirtIOBlockReq, req); } static void complete_request_early(VirtIOBlockDataPlane *s, VirtQueueElement *elem, @@ -128,14 +122,15 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, int64_t sector_num, VirtQueueElement *elem, QEMUIOVector *inhdr) { - VirtIOBlockRequest *req = g_slice_new0(VirtIOBlockRequest); + VirtIOBlock *dev = VIRTIO_BLK(s->vdev); + VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); QEMUIOVector *qiov; int nb_sectors; /* Fill in virtio block metadata needed for completion */ - req->s = s; req->elem = elem; req->inhdr = inhdr; + req->dev = dev; qemu_iovec_init_external(&req->qiov, iov, iov_cnt); qiov = &req->qiov; @@ -153,7 +148,7 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, static void complete_flush(void *opaque, int ret) { - VirtIOBlockRequest *req = opaque; + VirtIOBlockReq *req = opaque; unsigned char status; if (ret == 0) { @@ -162,15 +157,16 @@ static void complete_flush(void *opaque, int ret) status = VIRTIO_BLK_S_IOERR; } - complete_request_early(req->s, req->elem, req->inhdr, status); - g_slice_free(VirtIOBlockRequest, req); + complete_request_early(req->dev->dataplane, req->elem, req->inhdr, status); + g_slice_free(VirtIOBlockReq, req); } static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, QEMUIOVector *inhdr) { - VirtIOBlockRequest *req = g_slice_new(VirtIOBlockRequest); - req->s = s; + VirtIOBlock *dev = VIRTIO_BLK(s->vdev); + VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); + req->dev = dev; req->elem = elem; req->inhdr = inhdr; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index b495e42d6d..4211cd645f 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -142,6 +142,10 @@ typedef struct VirtIOBlockReq { QEMUIOVector qiov; struct VirtIOBlockReq *next; BlockAcctCookie acct; + +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE + QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */ +#endif } VirtIOBlockReq; #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \ -- cgit v1.2.3-55-g7522 From eddb102e86f61d4b71877f8ac268ebc4bf7265bf Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:47 +0800 Subject: virtio-blk: Use VirtIOBlockReq.in to drop VirtIOBlockReq.inhdr In current virtio spec, inhdr is a single byte, and is unlikely to change for both functionality and compatibility considerations. Non-dataplane uses .in, and we are on the way to converge them. So let's unify it to get cleaner code. Remove .inhdr and use .in. Signed-off-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 54 +++++++++++++++++------------------------ include/hw/virtio/virtio-blk.h | 4 --- 2 files changed, 22 insertions(+), 36 deletions(-) (limited to 'include') diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 70e8c1452b..cef707fac2 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -76,9 +76,7 @@ static void complete_rdwr(void *opaque, int ret) trace_virtio_blk_data_plane_complete_request(req->dev->dataplane, req->elem->index, ret); - qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr)); - qemu_iovec_destroy(req->inhdr); - g_slice_free(QEMUIOVector, req->inhdr); + stb_p(&req->in->status, hdr.status); /* According to the virtio specification len should be the number of bytes * written to, but for virtio-blk it seems to be the number of bytes @@ -90,24 +88,20 @@ static void complete_rdwr(void *opaque, int ret) } static void complete_request_early(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - QEMUIOVector *inhdr, unsigned char status) + struct virtio_blk_inhdr *inhdr, + unsigned char status) { - struct virtio_blk_inhdr hdr = { - .status = status, - }; + stb_p(&inhdr->status, status); - qemu_iovec_from_buf(inhdr, 0, &hdr, sizeof(hdr)); - qemu_iovec_destroy(inhdr); - g_slice_free(QEMUIOVector, inhdr); - - vring_push(&s->vring, elem, sizeof(hdr)); + vring_push(&s->vring, elem, sizeof(*inhdr)); notify_guest(s); } /* Get disk serial number */ static void do_get_id_cmd(VirtIOBlockDataPlane *s, struct iovec *iov, unsigned int iov_cnt, - VirtQueueElement *elem, QEMUIOVector *inhdr) + VirtQueueElement *elem, + struct virtio_blk_inhdr *inhdr) { char id[VIRTIO_BLK_ID_BYTES]; @@ -120,7 +114,7 @@ static void do_get_id_cmd(VirtIOBlockDataPlane *s, static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, struct iovec *iov, unsigned iov_cnt, int64_t sector_num, VirtQueueElement *elem, - QEMUIOVector *inhdr) + struct virtio_blk_inhdr *inhdr) { VirtIOBlock *dev = VIRTIO_BLK(s->vdev); VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); @@ -129,8 +123,8 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, /* Fill in virtio block metadata needed for completion */ req->elem = elem; - req->inhdr = inhdr; req->dev = dev; + req->in = inhdr; qemu_iovec_init_external(&req->qiov, iov, iov_cnt); qiov = &req->qiov; @@ -157,24 +151,24 @@ static void complete_flush(void *opaque, int ret) status = VIRTIO_BLK_S_IOERR; } - complete_request_early(req->dev->dataplane, req->elem, req->inhdr, status); + complete_request_early(req->dev->dataplane, req->elem, req->in, status); g_slice_free(VirtIOBlockReq, req); } static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - QEMUIOVector *inhdr) + struct virtio_blk_inhdr *inhdr) { VirtIOBlock *dev = VIRTIO_BLK(s->vdev); VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); req->dev = dev; req->elem = elem; - req->inhdr = inhdr; + req->in = inhdr; bdrv_aio_flush(s->blk->conf.bs, complete_flush, req); } static void do_scsi_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, - QEMUIOVector *inhdr) + struct virtio_blk_inhdr *inhdr) { int status; @@ -189,8 +183,7 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) unsigned out_num = elem->out_num; unsigned in_num = elem->in_num; struct virtio_blk_outhdr outhdr; - QEMUIOVector *inhdr; - size_t in_size; + struct virtio_blk_inhdr *inhdr; /* Copy in outhdr */ if (unlikely(iov_to_buf(iov, out_num, 0, &outhdr, @@ -200,17 +193,16 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) } iov_discard_front(&iov, &out_num, sizeof(outhdr)); - /* Grab inhdr for later */ - in_size = iov_size(in_iov, in_num); - if (in_size < sizeof(struct virtio_blk_inhdr)) { - error_report("virtio_blk request inhdr too short"); + /* We are likely safe with the iov_len check, because inhdr is only 1 byte, + * but checking here in case the header gets bigger in the future. */ + if (in_num < 1 || in_iov[in_num - 1].iov_len < sizeof(*inhdr)) { + error_report("virtio-blk request inhdr too short"); return -EFAULT; } - inhdr = g_slice_new(QEMUIOVector); - qemu_iovec_init(inhdr, 1); - qemu_iovec_concat_iov(inhdr, in_iov, in_num, - in_size - sizeof(struct virtio_blk_inhdr), - sizeof(struct virtio_blk_inhdr)); + + /* Grab inhdr for later */ + inhdr = (void *)in_iov[in_num - 1].iov_base + + in_iov[in_num - 1].iov_len - sizeof(*inhdr); iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr)); /* TODO Linux sets the barrier bit even when not advertised! */ @@ -243,8 +235,6 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) default: error_report("virtio-blk unsupported request type %#x", outhdr.type); - qemu_iovec_destroy(inhdr); - g_slice_free(QEMUIOVector, inhdr); return -EFAULT; } } diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 4211cd645f..b495e42d6d 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -142,10 +142,6 @@ typedef struct VirtIOBlockReq { QEMUIOVector qiov; struct VirtIOBlockReq *next; BlockAcctCookie acct; - -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE - QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */ -#endif } VirtIOBlockReq; #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \ -- cgit v1.2.3-55-g7522 From 827805a2492c1bbf1c0712ed18ee069b4ebf3dd6 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 11 Jun 2014 12:11:48 +0800 Subject: virtio-blk: Convert VirtIOBlockReq.out to structrue The virtio code currently assumes that the outhdr is in its own iovec. This is not guaranteed by the spec, so we should relax this assumption. Convert the VirtIOBlockReq.out field to structrue so that we can use iov_to_buf and then discard the header from the beginning of iovec. Suggested-by: Paolo Bonzini Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 20 ++++++++++++++------ include/hw/virtio/virtio-blk.h | 2 +- 2 files changed, 15 insertions(+), 7 deletions(-) (limited to 'include') diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index b5cc3855cf..05610959a6 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -12,6 +12,7 @@ */ #include "qemu-common.h" +#include "qemu/iov.h" #include "qemu/error-report.h" #include "trace.h" #include "hw/block/block.h" @@ -81,7 +82,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret) trace_virtio_blk_rw_complete(req, ret); if (ret) { - bool is_read = !(ldl_p(&req->out->type) & VIRTIO_BLK_T_OUT); + bool is_read = !(ldl_p(&req->out.type) & VIRTIO_BLK_T_OUT); if (virtio_blk_handle_rw_error(req, -ret, is_read)) return; } @@ -287,7 +288,7 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb) BlockRequest *blkreq; uint64_t sector; - sector = ldq_p(&req->out->sector); + sector = ldq_p(&req->out.sector); bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_WRITE); @@ -321,7 +322,7 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req) { uint64_t sector; - sector = ldq_p(&req->out->sector); + sector = ldq_p(&req->out.sector); bdrv_acct_start(req->dev->bs, &req->acct, req->qiov.size, BDRV_ACCT_READ); @@ -344,22 +345,29 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; + struct iovec *iov = req->elem->out_sg; + unsigned out_num = req->elem->out_num; if (req->elem->out_num < 1 || req->elem->in_num < 1) { error_report("virtio-blk missing headers"); exit(1); } - if (req->elem->out_sg[0].iov_len < sizeof(*req->out) || + if (req->elem->out_sg[0].iov_len < sizeof(req->out) || req->elem->in_sg[req->elem->in_num - 1].iov_len < sizeof(*req->in)) { error_report("virtio-blk header not in correct element"); exit(1); } - req->out = (void *)req->elem->out_sg[0].iov_base; + if (unlikely(iov_to_buf(iov, out_num, 0, &req->out, + sizeof(req->out)) != sizeof(req->out))) { + error_report("virtio-blk request outhdr too short"); + exit(1); + } + iov_discard_front(&iov, &out_num, sizeof(req->out)); req->in = (void *)req->elem->in_sg[req->elem->in_num - 1].iov_base; - type = ldl_p(&req->out->type); + type = ldl_p(&req->out.type); if (type & VIRTIO_BLK_T_FLUSH) { virtio_blk_handle_flush(req, mrb); diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index b495e42d6d..2571e961ec 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -138,7 +138,7 @@ typedef struct VirtIOBlockReq { VirtIOBlock *dev; VirtQueueElement *elem; struct virtio_blk_inhdr *in; - struct virtio_blk_outhdr *out; + struct virtio_blk_outhdr out; QEMUIOVector qiov; struct VirtIOBlockReq *next; BlockAcctCookie acct; -- cgit v1.2.3-55-g7522 From ac46821f2c6eb0617ac911daff111cbc30a4c40c Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 17 Jun 2014 14:32:04 +0800 Subject: block: make bdrv_query_stats() static This function is only called from block/qapi.c. There is no need to keep it public. Signed-off-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/qapi.c | 2 +- include/block/qapi.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) (limited to 'include') diff --git a/block/qapi.c b/block/qapi.c index 97e16418ef..aeabaaf85c 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -293,7 +293,7 @@ void bdrv_query_info(BlockDriverState *bs, qapi_free_BlockInfo(info); } -BlockStats *bdrv_query_stats(const BlockDriverState *bs) +static BlockStats *bdrv_query_stats(const BlockDriverState *bs) { BlockStats *s; diff --git a/include/block/qapi.h b/include/block/qapi.h index e92c00daf6..03745464d6 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -39,7 +39,6 @@ void bdrv_query_image_info(BlockDriverState *bs, void bdrv_query_info(BlockDriverState *bs, BlockInfo **p_info, Error **errp); -BlockStats *bdrv_query_stats(const BlockDriverState *bs); void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f, QEMUSnapshotInfo *sn); -- cgit v1.2.3-55-g7522 From bf4bd461b43d90c5af30f61f740c1bb675849ab9 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 17 Jun 2014 14:32:06 +0800 Subject: virtio-blk: Make request completion function virtual virtio_blk_req_complete will call VirtIOBlock.complete_request() to push data and notify guest. No functional change. Later, this will allow dataplane to provide it's own (vring_) version. Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 9 ++++++++- include/hw/virtio/virtio-blk.h | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 4b1aeab525..9975ad0324 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -44,7 +44,8 @@ static void virtio_blk_free_request(VirtIOBlockReq *req) } } -static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) +static void virtio_blk_complete_request(VirtIOBlockReq *req, + unsigned char status) { VirtIOBlock *s = req->dev; VirtIODevice *vdev = VIRTIO_DEVICE(s); @@ -56,6 +57,11 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) virtio_notify(vdev, s->vq); } +static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) +{ + req->dev->complete_request(req, status); +} + static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, bool is_read) { @@ -740,6 +746,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output); + s->complete_request = virtio_blk_complete_request; #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE virtio_blk_data_plane_create(vdev, blk, &s->dataplane, &err); if (err != NULL) { diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 2571e961ec..0398f4c46d 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -117,6 +117,7 @@ struct VirtIOBlkConf struct VirtIOBlockDataPlane; +struct VirtIOBlockReq; typedef struct VirtIOBlock { VirtIODevice parent_obj; BlockDriverState *bs; @@ -128,6 +129,8 @@ typedef struct VirtIOBlock { unsigned short sector_mask; bool original_wce; VMChangeStateEntry *change; + /* Function to push to vq and notify guest */ + void (*complete_request)(struct VirtIOBlockReq *req, unsigned char status); #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE Notifier migration_state_notifier; struct VirtIOBlockDataPlane *dataplane; -- cgit v1.2.3-55-g7522 From fee65db77181e6697745b313906bc4fdb30d2ff9 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 17 Jun 2014 14:32:07 +0800 Subject: virtio-blk: Export request handling functions to dataplane So that dataplane can use virtio_blk_handle_request and virtio_submit_multiwrite. Signed-off-by: Fam Zheng Tested-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/block/virtio-blk.c | 10 ++-------- include/hw/virtio/virtio-blk.h | 9 +++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) (limited to 'include') diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 9975ad0324..77fb4477c6 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -253,12 +253,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) virtio_blk_free_request(req); } -typedef struct MultiReqBuffer { - BlockRequest blkreq[32]; - unsigned int num_writes; -} MultiReqBuffer; - -static void virtio_submit_multiwrite(BlockDriverState *bs, MultiReqBuffer *mrb) +void virtio_submit_multiwrite(BlockDriverState *bs, MultiReqBuffer *mrb) { int i, ret; @@ -347,8 +342,7 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req) virtio_blk_rw_complete, req); } -static void virtio_blk_handle_request(VirtIOBlockReq *req, - MultiReqBuffer *mrb) +void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; struct iovec *in_iov = req->elem->in_sg; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 0398f4c46d..d0fb26f963 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -137,6 +137,11 @@ typedef struct VirtIOBlock { #endif } VirtIOBlock; +typedef struct MultiReqBuffer { + BlockRequest blkreq[32]; + unsigned int num_writes; +} MultiReqBuffer; + typedef struct VirtIOBlockReq { VirtIOBlock *dev; VirtQueueElement *elem; @@ -172,4 +177,8 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); int virtio_blk_handle_scsi_req(VirtIOBlock *blk, VirtQueueElement *elem); +void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb); + +void virtio_submit_multiwrite(BlockDriverState *bs, MultiReqBuffer *mrb); + #endif -- cgit v1.2.3-55-g7522 From 09158f00e0fdb506dcbf36f67c615b7f6c604c5a Mon Sep 17 00:00:00 2001 From: Benoît Canet Date: Fri, 27 Jun 2014 18:25:25 +0200 Subject: block: Add replaces argument to drive-mirror drive-mirror will bdrv_swap the new BDS named node-name with the one pointed by replaces when the mirroring is finished. Signed-off-by: Benoit Canet Signed-off-by: Kevin Wolf --- block.c | 25 ++++++++++++++++++++ block/mirror.c | 60 +++++++++++++++++++++++++++++++++++++---------- blockdev.c | 31 +++++++++++++++++++++++- hmp.c | 2 +- include/block/block.h | 4 ++++ include/block/block_int.h | 3 +++ qapi/block-core.json | 6 ++++- qmp-commands.hx | 4 +++- 8 files changed, 118 insertions(+), 17 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index 106238d0b7..9ecadf002f 100644 --- a/block.c +++ b/block.c @@ -5766,3 +5766,28 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate) return false; } + +BlockDriverState *check_to_replace_node(const char *node_name, Error **errp) +{ + BlockDriverState *to_replace_bs = bdrv_find_node(node_name); + if (!to_replace_bs) { + error_setg(errp, "Node name '%s' not found", node_name); + return NULL; + } + + if (bdrv_op_is_blocked(to_replace_bs, BLOCK_OP_TYPE_REPLACE, errp)) { + return NULL; + } + + /* We don't want arbitrary node of the BDS chain to be replaced only the top + * most non filter in order to prevent data corruption. + * Another benefit is that this tests exclude backing files which are + * blocked by the backing blockers. + */ + if (!bdrv_is_first_non_filter(to_replace_bs)) { + error_setg(errp, "Only top most non filter can be replaced"); + return NULL; + } + + return to_replace_bs; +} diff --git a/block/mirror.c b/block/mirror.c index 7c9f898089..6c3ee7041c 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -32,6 +32,12 @@ typedef struct MirrorBlockJob { RateLimit limit; BlockDriverState *target; BlockDriverState *base; + /* The name of the graph node to replace */ + char *replaces; + /* The BDS to replace */ + BlockDriverState *to_replace; + /* Used to block operations on the drive-mirror-replace target */ + Error *replace_blocker; bool is_none_mode; BlockdevOnError on_source_error, on_target_error; bool synced; @@ -500,10 +506,14 @@ immediate_exit: bdrv_release_dirty_bitmap(bs, s->dirty_bitmap); bdrv_iostatus_disable(s->target); if (s->should_complete && ret == 0) { - if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) { - bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL); + BlockDriverState *to_replace = s->common.bs; + if (s->to_replace) { + to_replace = s->to_replace; } - bdrv_swap(s->target, s->common.bs); + if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) { + bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL); + } + bdrv_swap(s->target, to_replace); if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) { /* drop the bs loop chain formed by the swap: break the loop then * trigger the unref from the top one */ @@ -512,6 +522,12 @@ immediate_exit: bdrv_unref(p); } } + if (s->to_replace) { + bdrv_op_unblock_all(s->to_replace, s->replace_blocker); + error_free(s->replace_blocker); + bdrv_unref(s->to_replace); + } + g_free(s->replaces); bdrv_unref(s->target); block_job_completed(&s->common, ret); } @@ -550,6 +566,20 @@ static void mirror_complete(BlockJob *job, Error **errp) return; } + /* check the target bs is not blocked and block all operations on it */ + if (s->replaces) { + s->to_replace = check_to_replace_node(s->replaces, &local_err); + if (!s->to_replace) { + error_propagate(errp, local_err); + return; + } + + error_setg(&s->replace_blocker, + "block device is in use by block-job-complete"); + bdrv_op_block_all(s->to_replace, s->replace_blocker); + bdrv_ref(s->to_replace); + } + s->should_complete = true; block_job_resume(job); } @@ -572,14 +602,15 @@ static const BlockJobDriver commit_active_job_driver = { }; static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, - int64_t speed, int64_t granularity, - int64_t buf_size, - BlockdevOnError on_source_error, - BlockdevOnError on_target_error, - BlockDriverCompletionFunc *cb, - void *opaque, Error **errp, - const BlockJobDriver *driver, - bool is_none_mode, BlockDriverState *base) + const char *replaces, + int64_t speed, int64_t granularity, + int64_t buf_size, + BlockdevOnError on_source_error, + BlockdevOnError on_target_error, + BlockDriverCompletionFunc *cb, + void *opaque, Error **errp, + const BlockJobDriver *driver, + bool is_none_mode, BlockDriverState *base) { MirrorBlockJob *s; @@ -610,6 +641,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, return; } + s->replaces = g_strdup(replaces); s->on_source_error = on_source_error; s->on_target_error = on_target_error; s->target = target; @@ -631,6 +663,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, } void mirror_start(BlockDriverState *bs, BlockDriverState *target, + const char *replaces, int64_t speed, int64_t granularity, int64_t buf_size, MirrorSyncMode mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, @@ -642,7 +675,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, is_none_mode = mode == MIRROR_SYNC_MODE_NONE; base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL; - mirror_start_job(bs, target, speed, granularity, buf_size, + mirror_start_job(bs, target, replaces, + speed, granularity, buf_size, on_source_error, on_target_error, cb, opaque, errp, &mirror_job_driver, is_none_mode, base); } @@ -690,7 +724,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, } bdrv_ref(base); - mirror_start_job(bs, base, speed, 0, 0, + mirror_start_job(bs, base, NULL, speed, 0, 0, on_error, on_error, cb, opaque, &local_err, &commit_active_job_driver, false, base); if (local_err) { diff --git a/blockdev.c b/blockdev.c index 943301226d..69b7c2a8c5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2100,6 +2100,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp) void qmp_drive_mirror(const char *device, const char *target, bool has_format, const char *format, bool has_node_name, const char *node_name, + bool has_replaces, const char *replaces, enum MirrorSyncMode sync, bool has_mode, enum NewImageMode mode, bool has_speed, int64_t speed, @@ -2187,6 +2188,29 @@ void qmp_drive_mirror(const char *device, const char *target, return; } + if (has_replaces) { + BlockDriverState *to_replace_bs; + + if (!has_node_name) { + error_setg(errp, "a node-name must be provided when replacing a" + " named node of the graph"); + return; + } + + to_replace_bs = check_to_replace_node(replaces, &local_err); + + if (!to_replace_bs) { + error_propagate(errp, local_err); + return; + } + + if (size != bdrv_getlength(to_replace_bs)) { + error_setg(errp, "cannot replace image with a mirror image of " + "different size"); + return; + } + } + if ((sync == MIRROR_SYNC_MODE_FULL || !source) && mode != NEW_IMAGE_MODE_EXISTING) { @@ -2231,7 +2255,12 @@ void qmp_drive_mirror(const char *device, const char *target, return; } - mirror_start(bs, target_bs, speed, granularity, buf_size, sync, + /* pass the node name to replace to mirror start since it's loose coupling + * and will allow to check whether the node still exist at mirror completion + */ + mirror_start(bs, target_bs, + has_replaces ? replaces : NULL, + speed, granularity, buf_size, sync, on_source_error, on_target_error, block_job_cb, bs, &local_err); if (local_err != NULL) { diff --git a/hmp.c b/hmp.c index 73acc3283c..6429e6b447 100644 --- a/hmp.c +++ b/hmp.c @@ -933,7 +933,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) } qmp_drive_mirror(device, filename, !!format, format, - false, NULL, + false, NULL, false, NULL, full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, true, mode, false, 0, false, 0, false, 0, false, 0, false, 0, &err); diff --git a/include/block/block.h b/include/block/block.h index d0baf4fb83..b53833d1d6 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -175,6 +175,7 @@ typedef enum BlockOpType { BLOCK_OP_TYPE_MIRROR, BLOCK_OP_TYPE_RESIZE, BLOCK_OP_TYPE_STREAM, + BLOCK_OP_TYPE_REPLACE, BLOCK_OP_TYPE_MAX, } BlockOpType; @@ -314,6 +315,9 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs, BlockDriverState *candidate); bool bdrv_is_first_non_filter(BlockDriverState *candidate); +/* check if a named node can be replaced when doing drive-mirror */ +BlockDriverState *check_to_replace_node(const char *node_name, Error **errp); + /* async block I/O */ typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector, int sector_num); diff --git a/include/block/block_int.h b/include/block/block_int.h index 135c5dc0e9..53e77cf11e 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -489,6 +489,8 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, * mirror_start: * @bs: Block device to operate on. * @target: Block device to write to. + * @replaces: Block graph node name to replace once the mirror is done. Can + * only be used when full mirroring is selected. * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @granularity: The chosen granularity for the dirty bitmap. * @buf_size: The amount of data that can be in flight at one time. @@ -505,6 +507,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, * @bs will be switched to read from @target. */ void mirror_start(BlockDriverState *bs, BlockDriverState *target, + const char *replaces, int64_t speed, int64_t granularity, int64_t buf_size, MirrorSyncMode mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, diff --git a/qapi/block-core.json b/qapi/block-core.json index ff7224f647..406ce03951 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -768,6 +768,10 @@ # @node-name: #optional the new block driver state node name in the graph # (Since 2.1) # +# @replaces: #optional with sync=full graph node name to be replaced by the new +# image when a whole image copy is done. This can be used to repair +# broken Quorum files. (Since 2.1) +# # @mode: #optional whether and how QEMU should create a new image, default is # 'absolute-paths'. # @@ -800,7 +804,7 @@ ## { 'command': 'drive-mirror', 'data': { 'device': 'str', 'target': 'str', '*format': 'str', - '*node-name': 'str', + '*node-name': 'str', '*replaces': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', '*speed': 'int', '*granularity': 'uint32', '*buf-size': 'int', '*on-source-error': 'BlockdevOnError', diff --git a/qmp-commands.hx b/qmp-commands.hx index 5254938878..d342b8a067 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1293,7 +1293,7 @@ EQMP { .name = "drive-mirror", .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," - "node-name:s?," + "node-name:s?,replaces:s?," "on-source-error:s?,on-target-error:s?," "granularity:i?,buf-size:i?", .mhandler.cmd_new = qmp_marshal_input_drive_mirror, @@ -1317,6 +1317,8 @@ Arguments: - "format": format of new image (json-string, optional) - "node-name": the name of the new block driver state in the node graph (json-string, optional) +- "replaces": the block driver node name to replace when finished + (json-string, optional) - "mode": how an image file should be created into the target file/device (NewImageMode, optional, default 'absolute-paths') - "speed": maximum speed of the streaming job, in bytes per second -- cgit v1.2.3-55-g7522 From 6b8aeca574a15668c47296d8e0c4f96c72e63c36 Mon Sep 17 00:00:00 2001 From: Chen Gang Date: Mon, 23 Jun 2014 23:28:23 +0800 Subject: block.c: Don't return success for bdrv_append_temp_snapshot() failure When failure occurs, 'ret' need be set, or may return 0 to indicate success. Previously, an error was set in errp, but 0 was returned anyway. So let bdrv_append_temp_snapshot() return an error code and use that for the bdrv_open() return value. Also, error_propagate() need be called only one time within a function. Signed-off-by: Chen Gang Signed-off-by: Kevin Wolf --- block.c | 7 ++++--- include/block/block.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/block.c b/block.c index 9ecadf002f..6856c18aa8 100644 --- a/block.c +++ b/block.c @@ -1278,7 +1278,7 @@ done: return ret; } -void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) +int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) { /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */ char *tmp_filename = g_malloc0(PATH_MAX + 1); @@ -1296,6 +1296,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) /* Get the required size from the image */ total_size = bdrv_getlength(bs); if (total_size < 0) { + ret = total_size; error_setg_errno(errp, -total_size, "Could not get image size"); goto out; } @@ -1342,6 +1343,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) out: g_free(tmp_filename); + return ret; } /* @@ -1491,9 +1493,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, /* For snapshot=on, create a temporary qcow2 overlay. bs points to the * temporary snapshot afterwards. */ if (snapshot_flags) { - bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err); + ret = bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err); if (local_err) { - error_propagate(errp, local_err); goto close_and_fail; } } diff --git a/include/block/block.h b/include/block/block.h index b53833d1d6..7e92f549fb 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -214,7 +214,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, bool allow_none, Error **errp); void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd); int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp); -void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp); +int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp); int bdrv_open(BlockDriverState **pbs, const char *filename, const char *reference, QDict *options, int flags, BlockDriver *drv, Error **errp); -- cgit v1.2.3-55-g7522