From c2ebb05e25dcb3acbcdaa541234746151d0ff6b1 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Sun, 14 Sep 2014 20:29:59 +0100 Subject: block/vhdx.c: Mark parent_vhdx_guid variable as unused The parent_vhdx_guid variable is defined but never used, which provokes complaints from newer versions of clang. Since the variable definition is here acting as documentation of the image format, mark it with the 'unused' attribute to keep the compiler happy rather than simply deleting it. Signed-off-by: Peter Maydell Reviewed-by: Fam Zheng Reviewed-by: Jeff Cody Signed-off-by: Stefan Hajnoczi --- block/vhdx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'block') diff --git a/block/vhdx.c b/block/vhdx.c index 796b7bd..d9aa2c1 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -99,7 +99,8 @@ static const MSGUID logical_sector_guid = { .data1 = 0x8141bf1d, /* Each parent type must have a valid GUID; this is for parent images * of type 'VHDX'. If we were to allow e.g. a QCOW2 parent, we would * need to make up our own QCOW2 GUID type */ -static const MSGUID parent_vhdx_guid = { .data1 = 0xb04aefb7, +static const MSGUID parent_vhdx_guid __attribute__((unused)) + = { .data1 = 0xb04aefb7, .data2 = 0xd19e, .data3 = 0x4a81, .data4 = { 0xb7, 0x89, 0x25, 0xb8, -- cgit v1.1 From 771b64daf9c73be98d223d3ab101a61f02cac277 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:13 +0800 Subject: linux-aio: Convert laio_aiocb_info.cancel to .cancel_async Just call io_cancel (2), if it fails, it means the request is not canceled, so the event loop will eventually call qemu_laio_process_completion. In qemu_laio_process_completion, change to call the cb unconditionally. It is required by bdrv_aio_cancel_async. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/linux-aio.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) (limited to 'block') diff --git a/block/linux-aio.c b/block/linux-aio.c index 9aca758..b67f56c 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -85,9 +85,8 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s, ret = -EINVAL; } } - - laiocb->common.cb(laiocb->common.opaque, ret); } + laiocb->common.cb(laiocb->common.opaque, ret); qemu_aio_release(laiocb); } @@ -153,35 +152,22 @@ static void laio_cancel(BlockDriverAIOCB *blockacb) struct io_event event; int ret; - if (laiocb->ret != -EINPROGRESS) + if (laiocb->ret != -EINPROGRESS) { return; - - /* - * Note that as of Linux 2.6.31 neither the block device code nor any - * filesystem implements cancellation of AIO request. - * Thus the polling loop below is the normal code path. - */ + } ret = io_cancel(laiocb->ctx->ctx, &laiocb->iocb, &event); - if (ret == 0) { - laiocb->ret = -ECANCELED; + laiocb->ret = -ECANCELED; + if (ret != 0) { + /* iocb is not cancelled, cb will be called by the event loop later */ return; } - /* - * We have to wait for the iocb to finish. - * - * The only way to get the iocb status update is by polling the io context. - * We might be able to do this slightly more optimal by removing the - * O_NONBLOCK flag. - */ - while (laiocb->ret == -EINPROGRESS) { - qemu_laio_completion_cb(&laiocb->ctx->e); - } + laiocb->common.cb(laiocb->common.opaque, laiocb->ret); } static const AIOCBInfo laio_aiocb_info = { .aiocb_size = sizeof(struct qemu_laiocb), - .cancel = laio_cancel, + .cancel_async = laio_cancel, }; static void ioq_init(LaioQueue *io_q) -- cgit v1.1 From 722d93335f7cda3065efc17763fd44dd6b244ed1 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:15 +0800 Subject: iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async Also drop the unused field "canceled". Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/iscsi.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) (limited to 'block') diff --git a/block/iscsi.c b/block/iscsi.c index 84bcae8..f8328d6 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -88,7 +88,6 @@ typedef struct IscsiAIOCB { struct scsi_task *task; uint8_t *buf; int status; - int canceled; int64_t sector_num; int nb_sectors; #ifdef __linux__ @@ -120,9 +119,7 @@ iscsi_bh_cb(void *p) g_free(acb->buf); acb->buf = NULL; - if (acb->canceled == 0) { - acb->common.cb(acb->common.opaque, acb->status); - } + acb->common.cb(acb->common.opaque, acb->status); if (acb->task != NULL) { scsi_free_scsi_task(acb->task); @@ -240,20 +237,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb) return; } - acb->canceled = 1; - /* send a task mgmt call to the target to cancel the task on the target */ iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task, iscsi_abort_task_cb, acb); - while (acb->status == -EINPROGRESS) { - aio_poll(iscsilun->aio_context, true); - } } static const AIOCBInfo iscsi_aiocb_info = { .aiocb_size = sizeof(IscsiAIOCB), - .cancel = iscsi_aio_cancel, + .cancel_async = iscsi_aio_cancel, }; @@ -638,10 +630,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, g_free(acb->buf); acb->buf = NULL; - if (acb->canceled != 0) { - return; - } - acb->status = 0; if (status < 0) { error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s", @@ -683,7 +671,6 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque); acb->iscsilun = iscsilun; - acb->canceled = 0; acb->bh = NULL; acb->status = -EINPROGRESS; acb->buf = NULL; -- cgit v1.1 From 4743b42a1b1947d76ad21ba7119c5638fa28c798 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:16 +0800 Subject: archipelago: Drop archipelago_aiocb_info.cancel The cancelled flag is no longer useful. Later the request will complete as before, and cb will be called. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/archipelago.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) (limited to 'block') diff --git a/block/archipelago.c b/block/archipelago.c index 93fb7c0..435efa7 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -91,7 +91,6 @@ typedef struct ArchipelagoAIOCB { struct BDRVArchipelagoState *s; QEMUIOVector *qiov; ARCHIPCmd cmd; - bool cancelled; int status; int64_t size; int64_t ret; @@ -318,9 +317,7 @@ static void qemu_archipelago_complete_aio(void *opaque) aio_cb->common.cb(aio_cb->common.opaque, aio_cb->ret); aio_cb->status = 0; - if (!aio_cb->cancelled) { - qemu_aio_release(aio_cb); - } + qemu_aio_release(aio_cb); g_free(reqdata); } @@ -725,19 +722,8 @@ static int qemu_archipelago_create(const char *filename, return ret; } -static void qemu_archipelago_aio_cancel(BlockDriverAIOCB *blockacb) -{ - ArchipelagoAIOCB *aio_cb = (ArchipelagoAIOCB *) blockacb; - aio_cb->cancelled = true; - while (aio_cb->status == -EINPROGRESS) { - aio_poll(bdrv_get_aio_context(aio_cb->common.bs), true); - } - qemu_aio_release(aio_cb); -} - static const AIOCBInfo archipelago_aiocb_info = { .aiocb_size = sizeof(ArchipelagoAIOCB), - .cancel = qemu_archipelago_aio_cancel, }; static int archipelago_submit_request(BDRVArchipelagoState *s, @@ -889,7 +875,6 @@ static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs, aio_cb->ret = 0; aio_cb->s = s; - aio_cb->cancelled = false; aio_cb->status = -EINPROGRESS; off = sector_num * BDRV_SECTOR_SIZE; -- cgit v1.1 From 4c781717c53d606bfa666d606c2875f9952a3155 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:17 +0800 Subject: blkdebug: Drop blkdebug_aiocb_info.cancel Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/blkdebug.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) (limited to 'block') diff --git a/block/blkdebug.c b/block/blkdebug.c index 69b330e..08131b3 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -52,11 +52,8 @@ typedef struct BlkdebugSuspendedReq { QLIST_ENTRY(BlkdebugSuspendedReq) next; } BlkdebugSuspendedReq; -static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb); - static const AIOCBInfo blkdebug_aiocb_info = { - .aiocb_size = sizeof(BlkdebugAIOCB), - .cancel = blkdebug_aio_cancel, + .aiocb_size = sizeof(BlkdebugAIOCB), }; enum { @@ -450,16 +447,6 @@ static void error_callback_bh(void *opaque) qemu_aio_release(acb); } -static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb) -{ - BlkdebugAIOCB *acb = container_of(blockacb, BlkdebugAIOCB, common); - if (acb->bh) { - qemu_bh_delete(acb->bh); - acb->bh = NULL; - } - qemu_aio_release(acb); -} - static BlockDriverAIOCB *inject_error(BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque, BlkdebugRule *rule) { -- cgit v1.1 From 9784e70f3d6b9805b5291f71f2543e346fe08433 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:18 +0800 Subject: blkverify: Drop blkverify_aiocb_info.cancel Also the finished pointer is not used any more. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/blkverify.c | 19 ------------------- 1 file changed, 19 deletions(-) (limited to 'block') diff --git a/block/blkverify.c b/block/blkverify.c index 163064c..460393f 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -29,7 +29,6 @@ struct BlkverifyAIOCB { int ret; /* first completed request's result */ unsigned int done; /* completion counter */ - bool *finished; /* completion signal for cancel */ QEMUIOVector *qiov; /* user I/O vector */ QEMUIOVector raw_qiov; /* cloned I/O vector for raw file */ @@ -38,22 +37,8 @@ struct BlkverifyAIOCB { void (*verify)(BlkverifyAIOCB *acb); }; -static void blkverify_aio_cancel(BlockDriverAIOCB *blockacb) -{ - BlkverifyAIOCB *acb = (BlkverifyAIOCB *)blockacb; - AioContext *aio_context = bdrv_get_aio_context(blockacb->bs); - bool finished = false; - - /* Wait until request completes, invokes its callback, and frees itself */ - acb->finished = &finished; - while (!finished) { - aio_poll(aio_context, true); - } -} - static const AIOCBInfo blkverify_aiocb_info = { .aiocb_size = sizeof(BlkverifyAIOCB), - .cancel = blkverify_aio_cancel, }; static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb, @@ -194,7 +179,6 @@ static BlkverifyAIOCB *blkverify_aio_get(BlockDriverState *bs, bool is_write, acb->qiov = qiov; acb->buf = NULL; acb->verify = NULL; - acb->finished = NULL; return acb; } @@ -208,9 +192,6 @@ static void blkverify_aio_bh(void *opaque) qemu_vfree(acb->buf); } acb->common.cb(acb->common.opaque, acb->ret); - if (acb->finished) { - *acb->finished = true; - } qemu_aio_release(acb); } -- cgit v1.1 From facb5539d6662e8208fc9a763719a5367d4e65d4 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:19 +0800 Subject: curl: Drop curl_aiocb_info.cancel Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/curl.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'block') diff --git a/block/curl.c b/block/curl.c index 938f9d9..6f5d6ae 100644 --- a/block/curl.c +++ b/block/curl.c @@ -613,14 +613,8 @@ out_noclean: return -EINVAL; } -static void curl_aio_cancel(BlockDriverAIOCB *blockacb) -{ - // Do we have to implement canceling? Seems to work without... -} - static const AIOCBInfo curl_aiocb_info = { .aiocb_size = sizeof(CURLAIOCB), - .cancel = curl_aio_cancel, }; -- cgit v1.1 From 533ffb17a5b023fe531477732b71d97267012863 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:20 +0800 Subject: qed: Drop qed_aiocb_info.cancel Also drop the now unused ->finished field. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/qed.c | 21 --------------------- 1 file changed, 21 deletions(-) (limited to 'block') diff --git a/block/qed.c b/block/qed.c index f8d9e12..7a15d44 100644 --- a/block/qed.c +++ b/block/qed.c @@ -18,22 +18,8 @@ #include "qapi/qmp/qerror.h" #include "migration/migration.h" -static void qed_aio_cancel(BlockDriverAIOCB *blockacb) -{ - QEDAIOCB *acb = (QEDAIOCB *)blockacb; - AioContext *aio_context = bdrv_get_aio_context(blockacb->bs); - bool finished = false; - - /* Wait for the request to finish */ - acb->finished = &finished; - while (!finished) { - aio_poll(aio_context, true); - } -} - static const AIOCBInfo qed_aiocb_info = { .aiocb_size = sizeof(QEDAIOCB), - .cancel = qed_aio_cancel, }; static int bdrv_qed_probe(const uint8_t *buf, int buf_size, @@ -919,18 +905,12 @@ static void qed_aio_complete_bh(void *opaque) BlockDriverCompletionFunc *cb = acb->common.cb; void *user_opaque = acb->common.opaque; int ret = acb->bh_ret; - bool *finished = acb->finished; qemu_bh_delete(acb->bh); qemu_aio_release(acb); /* Invoke callback */ cb(user_opaque, ret); - - /* Signal cancel completion */ - if (finished) { - *finished = true; - } } static void qed_aio_complete(QEDAIOCB *acb, int ret) @@ -1397,7 +1377,6 @@ static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs, opaque, flags); acb->flags = flags; - acb->finished = NULL; acb->qiov = qiov; acb->qiov_offset = 0; acb->cur_pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE; -- cgit v1.1 From 997dd8df3e95b2fdbd1f30b3deefaad4e9efd14a Mon Sep 17 00:00:00 2001 From: Liu Yuan Date: Thu, 11 Sep 2014 13:41:21 +0800 Subject: quorum: fix quorum_aio_cancel() For a fifo read pattern, we only have one running aio (possible other cases that has less number than num_children in the future), so we need to check if .acb is NULL against bdrv_aio_cancel() to avoid segfault. Cc: Eric Blake Cc: Benoit Canet Cc: Kevin Wolf Cc: Stefan Hajnoczi Signed-off-by: Liu Yuan Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/quorum.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 093382e..41c4249 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -138,7 +138,9 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) /* cancel all callbacks */ for (i = 0; i < s->num_children; i++) { - bdrv_aio_cancel(acb->qcrs[i].aiocb); + if (acb->qcrs[i].aiocb) { + bdrv_aio_cancel(acb->qcrs[i].aiocb); + } } g_free(acb->qcrs); -- cgit v1.1 From 7940e5056bfdb5d2861774e294d5459952cd0aee Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:22 +0800 Subject: quorum: Convert quorum_aiocb_info.cancel to .cancel_async Before, we cancel all the child requests with bdrv_aio_cancel, then free the acb.. Now we just kick off asynchronous cancellation of child requests and return, we know quorum_aio_cb will be called later, so in the end quorum_aio_finalize will take care of calling the caller's cb. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/quorum.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'block') diff --git a/block/quorum.c b/block/quorum.c index 41c4249..f343c04 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -139,17 +139,14 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) /* cancel all callbacks */ for (i = 0; i < s->num_children; i++) { if (acb->qcrs[i].aiocb) { - bdrv_aio_cancel(acb->qcrs[i].aiocb); + bdrv_aio_cancel_async(acb->qcrs[i].aiocb); } } - - g_free(acb->qcrs); - qemu_aio_release(acb); } static AIOCBInfo quorum_aiocb_info = { .aiocb_size = sizeof(QuorumAIOCB), - .cancel = quorum_aio_cancel, + .cancel_async = quorum_aio_cancel, }; static void quorum_aio_finalize(QuorumAIOCB *acb) -- cgit v1.1 From 7691e24dbebb46658e89b3f950fda6ec78bbb823 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:23 +0800 Subject: rbd: Drop rbd_aiocb_info.cancel And also drop the now unused "cancelled" field. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/rbd.c | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) (limited to 'block') diff --git a/block/rbd.c b/block/rbd.c index b7f7d5f..e5341fc 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -77,7 +77,6 @@ typedef struct RBDAIOCB { int64_t sector_num; int error; struct BDRVRBDState *s; - int cancelled; int status; } RBDAIOCB; @@ -408,9 +407,7 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret)); acb->status = 0; - if (!acb->cancelled) { - qemu_aio_release(acb); - } + qemu_aio_release(acb); } /* TODO Convert to fine grained options */ @@ -539,25 +536,8 @@ static void qemu_rbd_close(BlockDriverState *bs) rados_shutdown(s->cluster); } -/* - * Cancel aio. Since we don't reference acb in a non qemu threads, - * it is safe to access it here. - */ -static void qemu_rbd_aio_cancel(BlockDriverAIOCB *blockacb) -{ - RBDAIOCB *acb = (RBDAIOCB *) blockacb; - acb->cancelled = 1; - - while (acb->status == -EINPROGRESS) { - aio_poll(bdrv_get_aio_context(acb->common.bs), true); - } - - qemu_aio_release(acb); -} - static const AIOCBInfo rbd_aiocb_info = { .aiocb_size = sizeof(RBDAIOCB), - .cancel = qemu_rbd_aio_cancel, }; static void rbd_finish_bh(void *opaque) @@ -640,7 +620,6 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs, acb->ret = 0; acb->error = 0; acb->s = s; - acb->cancelled = 0; acb->bh = NULL; acb->status = -EINPROGRESS; -- cgit v1.1 From 6d24b4df38b0e1b2a6cdeeaa4a0e3dcdee643364 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:24 +0800 Subject: sheepdog: Convert sd_aiocb_info.cancel to .cancel_async Also drop the now unused SheepdogAIOCB.finished field. Note that this aio is internal to sheepdog driver and has NULL cb and opaque, and should be unused at all. Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/sheepdog.c | 46 +++++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 27 deletions(-) (limited to 'block') diff --git a/block/sheepdog.c b/block/sheepdog.c index 7da36e1..f538606 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -315,7 +315,6 @@ struct SheepdogAIOCB { void (*aio_done_func)(SheepdogAIOCB *); bool cancelable; - bool *finished; int nr_pending; }; @@ -446,9 +445,6 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req) static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb) { qemu_coroutine_enter(acb->coroutine, NULL); - if (acb->finished) { - *acb->finished = true; - } qemu_aio_release(acb); } @@ -482,36 +478,33 @@ static void sd_aio_cancel(BlockDriverAIOCB *blockacb) SheepdogAIOCB *acb = (SheepdogAIOCB *)blockacb; BDRVSheepdogState *s = acb->common.bs->opaque; AIOReq *aioreq, *next; - bool finished = false; - - acb->finished = &finished; - while (!finished) { - if (sd_acb_cancelable(acb)) { - /* Remove outstanding requests from pending and failed queues. */ - QLIST_FOREACH_SAFE(aioreq, &s->pending_aio_head, aio_siblings, - next) { - if (aioreq->aiocb == acb) { - free_aio_req(s, aioreq); - } + + if (sd_acb_cancelable(acb)) { + /* Remove outstanding requests from pending and failed queues. */ + QLIST_FOREACH_SAFE(aioreq, &s->pending_aio_head, aio_siblings, + next) { + if (aioreq->aiocb == acb) { + free_aio_req(s, aioreq); } - QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings, - next) { - if (aioreq->aiocb == acb) { - free_aio_req(s, aioreq); - } + } + QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings, + next) { + if (aioreq->aiocb == acb) { + free_aio_req(s, aioreq); } + } - assert(acb->nr_pending == 0); - sd_finish_aiocb(acb); - return; + assert(acb->nr_pending == 0); + if (acb->common.cb) { + acb->common.cb(acb->common.opaque, -ECANCELED); } - aio_poll(s->aio_context, true); + sd_finish_aiocb(acb); } } static const AIOCBInfo sd_aiocb_info = { - .aiocb_size = sizeof(SheepdogAIOCB), - .cancel = sd_aio_cancel, + .aiocb_size = sizeof(SheepdogAIOCB), + .cancel_async = sd_aio_cancel, }; static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov, @@ -528,7 +521,6 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov, acb->aio_done_func = NULL; acb->cancelable = true; - acb->finished = NULL; acb->coroutine = qemu_coroutine_self(); acb->ret = 0; acb->nr_pending = 0; -- cgit v1.1 From 5da91e4ef46fe85694c5c43a8f3a186a7a83cda7 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:25 +0800 Subject: win32-aio: Drop win32_aiocb_info.cancel Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/win32-aio.c | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'block') diff --git a/block/win32-aio.c b/block/win32-aio.c index 5030e32..eed86f7 100644 --- a/block/win32-aio.c +++ b/block/win32-aio.c @@ -106,22 +106,8 @@ static void win32_aio_completion_cb(EventNotifier *e) } } -static void win32_aio_cancel(BlockDriverAIOCB *blockacb) -{ - QEMUWin32AIOCB *waiocb = (QEMUWin32AIOCB *)blockacb; - - /* - * CancelIoEx is only supported in Vista and newer. For now, just - * wait for completion. - */ - while (!HasOverlappedIoCompleted(&waiocb->ov)) { - aio_poll(bdrv_get_aio_context(blockacb->bs), true); - } -} - static const AIOCBInfo win32_aiocb_info = { .aiocb_size = sizeof(QEMUWin32AIOCB), - .cancel = win32_aio_cancel, }; BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs, -- cgit v1.1 From 8007429a99d6ea8480ba0a7a5fb5ae92473f798c Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 13:41:28 +0800 Subject: block: Rename qemu_aio_release -> qemu_aio_unref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Suggested-by: Benoît Canet Signed-off-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- block/archipelago.c | 4 ++-- block/blkdebug.c | 2 +- block/blkverify.c | 2 +- block/curl.c | 10 +++++----- block/iscsi.c | 6 +++--- block/linux-aio.c | 4 ++-- block/qed.c | 2 +- block/quorum.c | 2 +- block/rbd.c | 4 ++-- block/sheepdog.c | 8 ++++---- block/win32-aio.c | 4 ++-- 11 files changed, 24 insertions(+), 24 deletions(-) (limited to 'block') diff --git a/block/archipelago.c b/block/archipelago.c index 435efa7..3c86d0b 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -317,7 +317,7 @@ static void qemu_archipelago_complete_aio(void *opaque) aio_cb->common.cb(aio_cb->common.opaque, aio_cb->ret); aio_cb->status = 0; - qemu_aio_release(aio_cb); + qemu_aio_unref(aio_cb); g_free(reqdata); } @@ -890,7 +890,7 @@ static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs, err_exit: error_report("qemu_archipelago_aio_rw(): I/O Error\n"); - qemu_aio_release(aio_cb); + qemu_aio_unref(aio_cb); return NULL; } diff --git a/block/blkdebug.c b/block/blkdebug.c index 08131b3..ced0b60 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -444,7 +444,7 @@ static void error_callback_bh(void *opaque) struct BlkdebugAIOCB *acb = opaque; qemu_bh_delete(acb->bh); acb->common.cb(acb->common.opaque, acb->ret); - qemu_aio_release(acb); + qemu_aio_unref(acb); } static BlockDriverAIOCB *inject_error(BlockDriverState *bs, diff --git a/block/blkverify.c b/block/blkverify.c index 460393f..7d64a23 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -192,7 +192,7 @@ static void blkverify_aio_bh(void *opaque) qemu_vfree(acb->buf); } acb->common.cb(acb->common.opaque, acb->ret); - qemu_aio_release(acb); + qemu_aio_unref(acb); } static void blkverify_aio_cb(void *opaque, int ret) diff --git a/block/curl.c b/block/curl.c index 6f5d6ae..225407c 100644 --- a/block/curl.c +++ b/block/curl.c @@ -212,7 +212,7 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque) qemu_iovec_from_buf(acb->qiov, 0, s->orig_buf + acb->start, acb->end - acb->start); acb->common.cb(acb->common.opaque, 0); - qemu_aio_release(acb); + qemu_aio_unref(acb); s->acb[i] = NULL; } } @@ -304,7 +304,7 @@ static void curl_multi_check_completion(BDRVCURLState *s) } acb->common.cb(acb->common.opaque, -EIO); - qemu_aio_release(acb); + qemu_aio_unref(acb); state->acb[i] = NULL; } } @@ -636,7 +636,7 @@ static void curl_readv_bh_cb(void *p) // we can just call the callback and be done. switch (curl_find_buf(s, start, acb->nb_sectors * SECTOR_SIZE, acb)) { case FIND_RET_OK: - qemu_aio_release(acb); + qemu_aio_unref(acb); // fall through case FIND_RET_WAIT: return; @@ -648,7 +648,7 @@ static void curl_readv_bh_cb(void *p) state = curl_init_state(acb->common.bs, s); if (!state) { acb->common.cb(acb->common.opaque, -EIO); - qemu_aio_release(acb); + qemu_aio_unref(acb); return; } @@ -664,7 +664,7 @@ static void curl_readv_bh_cb(void *p) if (state->buf_len && state->orig_buf == NULL) { curl_clean_state(state); acb->common.cb(acb->common.opaque, -ENOMEM); - qemu_aio_release(acb); + qemu_aio_unref(acb); return; } state->acb[0] = acb; diff --git a/block/iscsi.c b/block/iscsi.c index f8328d6..3c1b416 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -126,7 +126,7 @@ iscsi_bh_cb(void *p) acb->task = NULL; } - qemu_aio_release(acb); + qemu_aio_unref(acb); } static void @@ -680,7 +680,7 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, if (acb->task == NULL) { error_report("iSCSI: Failed to allocate task for scsi command. %s", iscsi_get_error(iscsi)); - qemu_aio_release(acb); + qemu_aio_unref(acb); return NULL; } memset(acb->task, 0, sizeof(struct scsi_task)); @@ -718,7 +718,7 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, (data.size > 0) ? &data : NULL, acb) != 0) { scsi_free_scsi_task(acb->task); - qemu_aio_release(acb); + qemu_aio_unref(acb); return NULL; } diff --git a/block/linux-aio.c b/block/linux-aio.c index b67f56c..f4baba2 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -88,7 +88,7 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s, } laiocb->common.cb(laiocb->common.opaque, ret); - qemu_aio_release(laiocb); + qemu_aio_unref(laiocb); } /* The completion BH fetches completed I/O requests and invokes their @@ -286,7 +286,7 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd, return &laiocb->common; out_free_aiocb: - qemu_aio_release(laiocb); + qemu_aio_unref(laiocb); return NULL; } diff --git a/block/qed.c b/block/qed.c index 7a15d44..e2316d7 100644 --- a/block/qed.c +++ b/block/qed.c @@ -907,7 +907,7 @@ static void qed_aio_complete_bh(void *opaque) int ret = acb->bh_ret; qemu_bh_delete(acb->bh); - qemu_aio_release(acb); + qemu_aio_unref(acb); /* Invoke callback */ cb(user_opaque, ret); diff --git a/block/quorum.c b/block/quorum.c index f343c04..7687466 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -168,7 +168,7 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) } g_free(acb->qcrs); - qemu_aio_release(acb); + qemu_aio_unref(acb); } static bool quorum_sha256_compare(QuorumVoteValue *a, QuorumVoteValue *b) diff --git a/block/rbd.c b/block/rbd.c index e5341fc..96947e3 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -407,7 +407,7 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret)); acb->status = 0; - qemu_aio_release(acb); + qemu_aio_unref(acb); } /* TODO Convert to fine grained options */ @@ -671,7 +671,7 @@ failed_completion: failed: g_free(rcb); qemu_vfree(acb->bounce); - qemu_aio_release(acb); + qemu_aio_unref(acb); return NULL; } diff --git a/block/sheepdog.c b/block/sheepdog.c index f538606..2d78ef9 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -445,7 +445,7 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req) static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb) { qemu_coroutine_enter(acb->coroutine, NULL); - qemu_aio_release(acb); + qemu_aio_unref(acb); } /* @@ -2130,7 +2130,7 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num, ret = sd_co_rw_vector(acb); if (ret <= 0) { - qemu_aio_release(acb); + qemu_aio_unref(acb); return ret; } @@ -2151,7 +2151,7 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, ret = sd_co_rw_vector(acb); if (ret <= 0) { - qemu_aio_release(acb); + qemu_aio_unref(acb); return ret; } @@ -2510,7 +2510,7 @@ static coroutine_fn int sd_co_discard(BlockDriverState *bs, int64_t sector_num, ret = sd_co_rw_vector(acb); if (ret <= 0) { - qemu_aio_release(acb); + qemu_aio_unref(acb); return ret; } diff --git a/block/win32-aio.c b/block/win32-aio.c index eed86f7..9323c1a 100644 --- a/block/win32-aio.c +++ b/block/win32-aio.c @@ -88,7 +88,7 @@ static void win32_aio_process_completion(QEMUWin32AIOState *s, waiocb->common.cb(waiocb->common.opaque, ret); - qemu_aio_release(waiocb); + qemu_aio_unref(waiocb); } static void win32_aio_completion_cb(EventNotifier *e) @@ -158,7 +158,7 @@ BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs, out_dec_count: aio->count--; out: - qemu_aio_release(waiocb); + qemu_aio_unref(waiocb); return NULL; } -- cgit v1.1 From e819ab22777fd455d5b130afe48c808d35ef7e93 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 11 Sep 2014 14:09:56 +0800 Subject: block: Introduce "null" drivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is an analogue to Linux null_blk. It can be used for testing or benchmarking block device emulation and general block layer functionalities such as coroutines and throttling, where disk IO is not necessary or wanted. Use null-aio:// for AIO version, and null-co:// for coroutine version. [Resolved conflict with Fam's async bdrv_aio_cancel() series: 1. Drop .bdrv_aio_cancel() since it is now done by block.c 2. Rename qemu_aio_release() to qemu_aio_unref() --Stefan] Signed-off-by: Fam Zheng Reviewed-by: Benoît Canet Reviewed-by: Eric Blake Message-id: 1410415798-20673-2-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/Makefile.objs | 1 + block/null.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 block/null.c (limited to 'block') diff --git a/block/Makefile.objs b/block/Makefile.objs index c9c8bbb..d6e23a2 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -9,6 +9,7 @@ block-obj-y += snapshot.o qapi.o block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o block-obj-$(CONFIG_POSIX) += raw-posix.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o +block-obj-y += null.o block-obj-y += nbd.o nbd-client.o sheepdog.o block-obj-$(CONFIG_LIBISCSI) += iscsi.o diff --git a/block/null.c b/block/null.c new file mode 100644 index 0000000..8284419 --- /dev/null +++ b/block/null.c @@ -0,0 +1,168 @@ +/* + * Null block driver + * + * Authors: + * Fam Zheng + * + * Copyright (C) 2014 Red Hat, Inc. + * + * 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 "block/block_int.h" + +typedef struct { + int64_t length; +} BDRVNullState; + +static QemuOptsList runtime_opts = { + .name = "null", + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), + .desc = { + { + .name = "filename", + .type = QEMU_OPT_STRING, + .help = "", + }, + { + .name = BLOCK_OPT_SIZE, + .type = QEMU_OPT_SIZE, + .help = "size of the null block", + }, + { /* end of list */ } + }, +}; + +static int null_file_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) +{ + QemuOpts *opts; + BDRVNullState *s = bs->opaque; + + opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); + qemu_opts_absorb_qdict(opts, options, &error_abort); + s->length = + qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 1 << 30); + qemu_opts_del(opts); + return 0; +} + +static void null_close(BlockDriverState *bs) +{ +} + +static int64_t null_getlength(BlockDriverState *bs) +{ + BDRVNullState *s = bs->opaque; + return s->length; +} + +static coroutine_fn int null_co_readv(BlockDriverState *bs, + int64_t sector_num, int nb_sectors, + QEMUIOVector *qiov) +{ + return 0; +} + +static coroutine_fn int null_co_writev(BlockDriverState *bs, + int64_t sector_num, int nb_sectors, + QEMUIOVector *qiov) +{ + return 0; +} + +static coroutine_fn int null_co_flush(BlockDriverState *bs) +{ + return 0; +} + +typedef struct { + BlockDriverAIOCB common; + QEMUBH *bh; +} NullAIOCB; + +static const AIOCBInfo null_aiocb_info = { + .aiocb_size = sizeof(NullAIOCB), +}; + +static void null_bh_cb(void *opaque) +{ + NullAIOCB *acb = opaque; + acb->common.cb(acb->common.opaque, 0); + qemu_bh_delete(acb->bh); + qemu_aio_unref(acb); +} + +static inline BlockDriverAIOCB *null_aio_common(BlockDriverState *bs, + BlockDriverCompletionFunc *cb, + void *opaque) +{ + NullAIOCB *acb; + + acb = qemu_aio_get(&null_aiocb_info, bs, cb, opaque); + acb->bh = aio_bh_new(bdrv_get_aio_context(bs), null_bh_cb, acb); + qemu_bh_schedule(acb->bh); + return &acb->common; +} + +static BlockDriverAIOCB *null_aio_readv(BlockDriverState *bs, + int64_t sector_num, QEMUIOVector *qiov, + int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque) +{ + return null_aio_common(bs, cb, opaque); +} + +static BlockDriverAIOCB *null_aio_writev(BlockDriverState *bs, + int64_t sector_num, QEMUIOVector *qiov, + int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque) +{ + return null_aio_common(bs, cb, opaque); +} + +static BlockDriverAIOCB *null_aio_flush(BlockDriverState *bs, + BlockDriverCompletionFunc *cb, + void *opaque) +{ + return null_aio_common(bs, cb, opaque); +} + +static BlockDriver bdrv_null_co = { + .format_name = "null-co", + .protocol_name = "null-co", + .instance_size = sizeof(BDRVNullState), + + .bdrv_file_open = null_file_open, + .bdrv_close = null_close, + .bdrv_getlength = null_getlength, + + .bdrv_co_readv = null_co_readv, + .bdrv_co_writev = null_co_writev, + .bdrv_co_flush_to_disk = null_co_flush, +}; + +static BlockDriver bdrv_null_aio = { + .format_name = "null-aio", + .protocol_name = "null-aio", + .instance_size = sizeof(BDRVNullState), + + .bdrv_file_open = null_file_open, + .bdrv_close = null_close, + .bdrv_getlength = null_getlength, + + .bdrv_aio_readv = null_aio_readv, + .bdrv_aio_writev = null_aio_writev, + .bdrv_aio_flush = null_aio_flush, +}; + +static void bdrv_null_init(void) +{ + bdrv_register(&bdrv_null_co); + bdrv_register(&bdrv_null_aio); +} + +block_init(bdrv_null_init); -- cgit v1.1 From 9bf040b962f90aa2e1cef6543dfee6c96f73ef7e Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 5 Sep 2014 16:07:15 +0200 Subject: qapi/block: Add "fatal" to BLOCK_IMAGE_CORRUPTED Not every BLOCK_IMAGE_CORRUPTED event must be fatal; for example, when reading from an image, they should generally not be. Nonetheless, even an image only read from may of course be corrupted and this can be detected during normal operation. In this case, a non-fatal event should be emitted, but the image should not be marked corrupt (in accordance to "fatal" set to false). Signed-off-by: Max Reitz Reviewed-by: Eric Blake Message-id: 1409926039-29044-2-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/qcow2-refcount.c | 1 + 1 file changed, 1 insertion(+) (limited to 'block') diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 43665b8..0bd75d2 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1853,6 +1853,7 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, offset, true, size, + true, &error_abort); g_free(message); -- cgit v1.1 From 85186ebdac7e183242deaa55d5049988de832be1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 5 Sep 2014 16:07:16 +0200 Subject: qcow2: Add qcow2_signal_corruption() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a helper function for easily marking an image corrupt (on fatal corruptions) while outputting an informative message to stderr and via QAPI. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Benoît Canet Message-id: 1409926039-29044-3-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/qcow2.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ block/qcow2.h | 5 +++++ 2 files changed, 53 insertions(+) (limited to 'block') diff --git a/block/qcow2.c b/block/qcow2.c index 0daf25c..f4bbe8b 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -31,6 +31,8 @@ #include "qapi/qmp/qerror.h" #include "qapi/qmp/qbool.h" #include "qapi/util.h" +#include "qapi/qmp/types.h" +#include "qapi-event.h" #include "trace.h" #include "qemu/option_int.h" @@ -2529,6 +2531,52 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts) return 0; } +/* + * If offset or size are negative, respectively, they will not be included in + * the BLOCK_IMAGE_CORRUPTED event emitted. + * fatal will be ignored for read-only BDS; corruptions found there will always + * be considered non-fatal. + */ +void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset, + int64_t size, const char *message_format, ...) +{ + BDRVQcowState *s = bs->opaque; + char *message; + va_list ap; + + fatal = fatal && !bs->read_only; + + if (s->signaled_corruption && + (!fatal || (s->incompatible_features & QCOW2_INCOMPAT_CORRUPT))) + { + return; + } + + va_start(ap, message_format); + message = g_strdup_vprintf(message_format, ap); + va_end(ap); + + if (fatal) { + fprintf(stderr, "qcow2: Marking image as corrupt: %s; further " + "corruption events will be suppressed\n", message); + } else { + fprintf(stderr, "qcow2: Image is corrupt: %s; further non-fatal " + "corruption events will be suppressed\n", message); + } + + qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs), message, + offset >= 0, offset, size >= 0, size, + fatal, &error_abort); + g_free(message); + + if (fatal) { + qcow2_mark_corrupt(bs); + bs->drv = NULL; /* make BDS unusable */ + } + + s->signaled_corruption = true; +} + static QemuOptsList qcow2_create_opts = { .name = "qcow2-create-opts", .head = QTAILQ_HEAD_INITIALIZER(qcow2_create_opts.head), diff --git a/block/qcow2.h b/block/qcow2.h index 6aeb7ea..7b7b6a6 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -261,6 +261,7 @@ typedef struct BDRVQcowState { bool discard_passthrough[QCOW2_DISCARD_MAX]; int overlap_check; /* bitmask of Qcow2MetadataOverlap values */ + bool signaled_corruption; uint64_t incompatible_features; uint64_t compatible_features; @@ -477,6 +478,10 @@ int qcow2_mark_corrupt(BlockDriverState *bs); int qcow2_mark_consistent(BlockDriverState *bs); int qcow2_update_header(BlockDriverState *bs); +void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset, + int64_t size, const char *message_format, ...) + GCC_FMT_ATTR(5, 6); + /* qcow2-refcount.c functions */ int qcow2_refcount_init(BlockDriverState *bs); void qcow2_refcount_close(BlockDriverState *bs); -- cgit v1.1 From adb435522b86b3fca2324cb8c94e17b55ae071f1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 5 Sep 2014 16:07:17 +0200 Subject: qcow2: Use qcow2_signal_corruption() for overlaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the new function in case of a failed overlap check. This changes output in case of corruption, so adapt iotest 060's reference output accordingly. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Benoît Canet Message-id: 1409926039-29044-4-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/qcow2-refcount.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) (limited to 'block') diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 0bd75d2..b9d421e 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -26,8 +26,6 @@ #include "block/block_int.h" #include "block/qcow2.h" #include "qemu/range.h" -#include "qapi/qmp/types.h" -#include "qapi-event.h" static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size); static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, @@ -1838,27 +1836,11 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, return ret; } else if (ret > 0) { int metadata_ol_bitnr = ffs(ret) - 1; - char *message; - assert(metadata_ol_bitnr < QCOW2_OL_MAX_BITNR); - fprintf(stderr, "qcow2: Preventing invalid write on metadata (overlaps " - "with %s); image marked as corrupt.\n", - metadata_ol_names[metadata_ol_bitnr]); - message = g_strdup_printf("Prevented %s overwrite", - metadata_ol_names[metadata_ol_bitnr]); - qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs), - message, - true, - offset, - true, - size, - true, - &error_abort); - g_free(message); - - qcow2_mark_corrupt(bs); - bs->drv = NULL; /* make BDS unusable */ + qcow2_signal_corruption(bs, true, offset, size, "Preventing invalid " + "write on metadata (overlaps with %s)", + metadata_ol_names[metadata_ol_bitnr]); return -EIO; } -- cgit v1.1 From a97c67ee6c1546b985c1048c7a1f9e4fc13d9ee1 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 5 Sep 2014 16:07:18 +0200 Subject: qcow2: Check L1/L2/reftable entries for alignment Offsets taken from the L1, L2 and refcount tables are generally assumed to be correctly aligned. However, this cannot be guaranteed if the image has been written to by something different than qemu, thus check all offsets taken from these tables for correct cluster alignment. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Message-id: 1409926039-29044-5-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/qcow2-cluster.c | 43 ++++++++++++++++++++++++++++++++++++++++--- block/qcow2-refcount.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 5 deletions(-) (limited to 'block') diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 735f687..f7dd8c0 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -486,6 +486,13 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, goto out; } + if (offset_into_cluster(s, l2_offset)) { + qcow2_signal_corruption(bs, true, -1, -1, "L2 table offset %#" PRIx64 + " unaligned (L1 index: %#" PRIx64 ")", + l2_offset, l1_index); + return -EIO; + } + /* load the l2 table in memory */ ret = l2_load(bs, l2_offset, &l2_table); @@ -508,8 +515,11 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, break; case QCOW2_CLUSTER_ZERO: if (s->qcow_version < 3) { - qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table); - return -EIO; + qcow2_signal_corruption(bs, true, -1, -1, "Zero cluster entry found" + " in pre-v3 image (L2 offset: %#" PRIx64 + ", L2 index: %#x)", l2_offset, l2_index); + ret = -EIO; + goto fail; } c = count_contiguous_clusters(nb_clusters, s->cluster_size, &l2_table[l2_index], QCOW_OFLAG_ZERO); @@ -525,6 +535,14 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, c = count_contiguous_clusters(nb_clusters, s->cluster_size, &l2_table[l2_index], QCOW_OFLAG_ZERO); *cluster_offset &= L2E_OFFSET_MASK; + if (offset_into_cluster(s, *cluster_offset)) { + qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset %#" + PRIx64 " unaligned (L2 offset: %#" PRIx64 + ", L2 index: %#x)", *cluster_offset, + l2_offset, l2_index); + ret = -EIO; + goto fail; + } break; default: abort(); @@ -541,6 +559,10 @@ out: *num = nb_available - index_in_cluster; return ret; + +fail: + qcow2_cache_put(bs, s->l2_table_cache, (void **)&l2_table); + return ret; } /* @@ -576,6 +598,12 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset, assert(l1_index < s->l1_size); l2_offset = s->l1_table[l1_index] & L1E_OFFSET_MASK; + if (offset_into_cluster(s, l2_offset)) { + qcow2_signal_corruption(bs, true, -1, -1, "L2 table offset %#" PRIx64 + " unaligned (L1 index: %#" PRIx64 ")", + l2_offset, l1_index); + return -EIO; + } /* seek the l2 table of the given l2 offset */ @@ -948,6 +976,15 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset, bool offset_matches = (cluster_offset & L2E_OFFSET_MASK) == *host_offset; + if (offset_into_cluster(s, cluster_offset & L2E_OFFSET_MASK)) { + qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset " + "%#llx unaligned (guest offset: %#" PRIx64 + ")", cluster_offset & L2E_OFFSET_MASK, + guest_offset); + ret = -EIO; + goto out; + } + if (*host_offset != 0 && !offset_matches) { *bytes = 0; ret = 0; @@ -979,7 +1016,7 @@ out: /* Only return a host offset if we actually made progress. Otherwise we * would make requirements for handle_alloc() that it can't fulfill */ - if (ret) { + if (ret > 0) { *host_offset = (cluster_offset & L2E_OFFSET_MASK) + offset_into_cluster(s, guest_offset); } diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index b9d421e..2bcaaf9 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -108,6 +108,13 @@ static int get_refcount(BlockDriverState *bs, int64_t cluster_index) if (!refcount_block_offset) return 0; + if (offset_into_cluster(s, refcount_block_offset)) { + qcow2_signal_corruption(bs, true, -1, -1, "Refblock offset %#" PRIx64 + " unaligned (reftable index: %#" PRIx64 ")", + refcount_block_offset, refcount_table_index); + return -EIO; + } + ret = qcow2_cache_get(bs, s->refcount_block_cache, refcount_block_offset, (void**) &refcount_block); if (ret < 0) { @@ -181,6 +188,14 @@ static int alloc_refcount_block(BlockDriverState *bs, /* If it's already there, we're done */ if (refcount_block_offset) { + if (offset_into_cluster(s, refcount_block_offset)) { + qcow2_signal_corruption(bs, true, -1, -1, "Refblock offset %#" + PRIx64 " unaligned (reftable index: " + "%#x)", refcount_block_offset, + refcount_table_index); + return -EIO; + } + return load_refcount_block(bs, refcount_block_offset, (void**) refcount_block); } @@ -836,8 +851,14 @@ void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry, case QCOW2_CLUSTER_NORMAL: case QCOW2_CLUSTER_ZERO: if (l2_entry & L2E_OFFSET_MASK) { - qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK, - nb_clusters << s->cluster_bits, type); + if (offset_into_cluster(s, l2_entry & L2E_OFFSET_MASK)) { + qcow2_signal_corruption(bs, false, -1, -1, + "Cannot free unaligned cluster %#llx", + l2_entry & L2E_OFFSET_MASK); + } else { + qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK, + nb_clusters << s->cluster_bits, type); + } } break; case QCOW2_CLUSTER_UNALLOCATED: @@ -901,6 +922,14 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs, old_l2_offset = l2_offset; l2_offset &= L1E_OFFSET_MASK; + if (offset_into_cluster(s, l2_offset)) { + qcow2_signal_corruption(bs, true, -1, -1, "L2 table offset %#" + PRIx64 " unaligned (L1 index: %#x)", + l2_offset, i); + ret = -EIO; + goto fail; + } + ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset, (void**) &l2_table); if (ret < 0) { @@ -933,6 +962,17 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs, case QCOW2_CLUSTER_NORMAL: case QCOW2_CLUSTER_ZERO: + if (offset_into_cluster(s, offset & L2E_OFFSET_MASK)) { + qcow2_signal_corruption(bs, true, -1, -1, "Data " + "cluster offset %#llx " + "unaligned (L2 offset: %#" + PRIx64 ", L2 index: %#x)", + offset & L2E_OFFSET_MASK, + l2_offset, j); + ret = -EIO; + goto fail; + } + cluster_index = (offset & L2E_OFFSET_MASK) >> s->cluster_bits; if (!cluster_index) { /* unallocated */ -- cgit v1.1 From 7b17ce60cc58b3c20b3e708a2d69f6bbe2b4edfa Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 20 Aug 2014 19:59:33 +0200 Subject: qcow2: Fix leak of QemuOpts in qcow2_open() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, the QemuOpts object opts is leaked if anything fails from its creation up to and including the image repair block. Fix this by freeing that object in the fail path. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Benoît Canet Message-id: 1408557576-14574-2-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/qcow2.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/qcow2.c b/block/qcow2.c index f4bbe8b..744e0db 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -538,7 +538,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, unsigned int len, i; int ret = 0; QCowHeader header; - QemuOpts *opts; + QemuOpts *opts = NULL; Error *local_err = NULL; uint64_t ext_end; uint64_t l1_vm_state_index; @@ -935,7 +935,6 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, error_setg(errp, "Unsupported value '%s' for qcow2 option " "'overlap-check'. Allowed are either of the following: " "none, constant, cached, all", opt_overlap_check); - qemu_opts_del(opts); ret = -EINVAL; goto fail; } @@ -950,6 +949,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, } qemu_opts_del(opts); + opts = NULL; if (s->use_lazy_refcounts && s->qcow_version < 3) { error_setg(errp, "Lazy refcounts require a qcow2 image with at least " @@ -967,6 +967,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, return ret; fail: + qemu_opts_del(opts); g_free(s->unknown_header_fields); cleanup_unknown_header_ext(bs); qcow2_free_snapshots(bs); -- cgit v1.1 From ee42b5ce0b33986970f5c2fd4b676cddc0d6306c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 20 Aug 2014 19:59:35 +0200 Subject: qcow2: Add overlap-check.template option Being able to set the overlap-check option to a string and then refine it via the overlap-check.* options is a nice idea for the command line but does not work so well for non-flattened dicts. In that case, one can only specify either but not both, so add a field to overlap-check.* which does the same as directly specifying overlap-check but can be used in conjunction with the other fields in non-flattened dicts. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Message-id: 1408557576-14574-4-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/qcow2.c | 24 ++++++++++++++++++++++-- block/qcow2.h | 1 + 2 files changed, 23 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/qcow2.c b/block/qcow2.c index 744e0db..778fc1e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -406,6 +406,12 @@ static QemuOptsList qcow2_runtime_opts = { "templates (none, constant, cached, all)", }, { + .name = QCOW2_OPT_OVERLAP_TEMPLATE, + .type = QEMU_OPT_STRING, + .help = "Selects which overlap checks to perform from a range of " + "templates (none, constant, cached, all)", + }, + { .name = QCOW2_OPT_OVERLAP_MAIN_HEADER, .type = QEMU_OPT_BOOL, .help = "Check for unintended writes into the main qcow2 header", @@ -542,7 +548,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, Error *local_err = NULL; uint64_t ext_end; uint64_t l1_vm_state_index; - const char *opt_overlap_check; + const char *opt_overlap_check, *opt_overlap_check_template; int overlap_check_template = 0; uint64_t l2_cache_size, refcount_cache_size; @@ -922,7 +928,21 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, s->discard_passthrough[QCOW2_DISCARD_OTHER] = qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false); - opt_overlap_check = qemu_opt_get(opts, "overlap-check") ?: "cached"; + opt_overlap_check = qemu_opt_get(opts, QCOW2_OPT_OVERLAP); + opt_overlap_check_template = qemu_opt_get(opts, QCOW2_OPT_OVERLAP_TEMPLATE); + if (opt_overlap_check_template && opt_overlap_check && + strcmp(opt_overlap_check_template, opt_overlap_check)) + { + error_setg(errp, "Conflicting values for qcow2 options '" + QCOW2_OPT_OVERLAP "' ('%s') and '" QCOW2_OPT_OVERLAP_TEMPLATE + "' ('%s')", opt_overlap_check, opt_overlap_check_template); + ret = -EINVAL; + goto fail; + } + if (!opt_overlap_check) { + opt_overlap_check = opt_overlap_check_template ?: "cached"; + } + if (!strcmp(opt_overlap_check, "none")) { overlap_check_template = 0; } else if (!strcmp(opt_overlap_check, "constant")) { diff --git a/block/qcow2.h b/block/qcow2.h index 7b7b6a6..7d61e61 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -83,6 +83,7 @@ #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot" #define QCOW2_OPT_DISCARD_OTHER "pass-discard-other" #define QCOW2_OPT_OVERLAP "overlap-check" +#define QCOW2_OPT_OVERLAP_TEMPLATE "overlap-check.template" #define QCOW2_OPT_OVERLAP_MAIN_HEADER "overlap-check.main-header" #define QCOW2_OPT_OVERLAP_ACTIVE_L1 "overlap-check.active-l1" #define QCOW2_OPT_OVERLAP_ACTIVE_L2 "overlap-check.active-l2" -- cgit v1.1 From 3e7e6f186f6180c5900dbc8db04abbbda1275dad Mon Sep 17 00:00:00 2001 From: Chrysostomos Nanakos Date: Tue, 16 Sep 2014 12:17:11 +0300 Subject: block/archipelago: Fix typo in qemu_archipelago_truncate() Fix a typo introduced by 94c80a438c85f2c19698547fbb115ea46d80c5f1 Signed-off-by: Chrysostomos Nanakos Signed-off-by: Stefan Hajnoczi --- block/archipelago.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'block') diff --git a/block/archipelago.c b/block/archipelago.c index 3c86d0b..73d91a4 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -993,7 +993,7 @@ static int qemu_archipelago_truncate(BlockDriverState *bs, int64_t offset) req = xseg_get_request(s->xseg, s->srcport, s->mportno, X_ALLOC); if (!req) { archipelagolog("Cannot get XSEG request\n"); - return err_exit2; + goto err_exit2; } ret = xseg_prep_request(s->xseg, req, targetlen, 0); -- cgit v1.1 From 550830f9351291c585c963204ad9127998b1c1ce Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 16 Sep 2014 15:24:24 +0100 Subject: block: delete cow block driver This patch removes support for the cow file format. Normally we do not break backwards compatibility but in this case there is no impact and it is the most logical option. Extraordinary claims require extraordinary evidence so I will show why removing the cow block driver is the right thing to do. The cow file format is the disk image format for Usermode Linux, a way of running a Linux system in userspace. The performance of UML was never great and it was hacky, but it enjoyed some popularity before hardware virtualization support became mainstream. QEMU's block/cow.c is supposed to read this image file format. Unfortunately the file format was underspecified: 1. Earlier Linux versions used the MAXPATHLEN constant for the backing filename field. The value of MAXPATHLEN can change, so Linux switched to a 4096 literal but QEMU has a 1024 literal. 2. Padding was not used on the header struct (both in the Linux kernel and in QEMU) so the struct layout varied across architectures. In particular, i386 and x86_64 were different due to int64_t alignment differences. Linux now uses __attribute__((packed)), QEMU does not. Therefore: 1. QEMU cow images do not conform to the Linux cow image file format. 2. cow images cannot be shared between different host architectures. This means QEMU cow images are useless and QEMU has not had bug reports from users actually hitting these issues. Let's get rid of this thing, it serves no purpose and no one will be affected. Signed-off-by: Stefan Hajnoczi Reviewed-by: Markus Armbruster Message-id: 1410877464-20481-1-git-send-email-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- block/Makefile.objs | 2 +- block/cow.c | 433 ---------------------------------------------------- 2 files changed, 1 insertion(+), 434 deletions(-) delete mode 100644 block/cow.c (limited to 'block') diff --git a/block/Makefile.objs b/block/Makefile.objs index d6e23a2..a833ed5 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -1,4 +1,4 @@ -block-obj-y += raw_bsd.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o +block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o block-obj-y += qed-check.o diff --git a/block/cow.c b/block/cow.c deleted file mode 100644 index c3769fe..0000000 --- a/block/cow.c +++ /dev/null @@ -1,433 +0,0 @@ -/* - * Block driver for the COW format - * - * Copyright (c) 2004 Fabrice Bellard - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ -#include "qemu-common.h" -#include "block/block_int.h" -#include "qemu/module.h" - -/**************************************************************/ -/* COW block driver using file system holes */ - -/* user mode linux compatible COW file */ -#define COW_MAGIC 0x4f4f4f4d /* MOOO */ -#define COW_VERSION 2 - -struct cow_header_v2 { - uint32_t magic; - uint32_t version; - char backing_file[1024]; - int32_t mtime; - uint64_t size; - uint32_t sectorsize; -}; - -typedef struct BDRVCowState { - CoMutex lock; - int64_t cow_sectors_offset; -} BDRVCowState; - -static int cow_probe(const uint8_t *buf, int buf_size, const char *filename) -{ - const struct cow_header_v2 *cow_header = (const void *)buf; - - if (buf_size >= sizeof(struct cow_header_v2) && - be32_to_cpu(cow_header->magic) == COW_MAGIC && - be32_to_cpu(cow_header->version) == COW_VERSION) - return 100; - else - return 0; -} - -static int cow_open(BlockDriverState *bs, QDict *options, int flags, - Error **errp) -{ - BDRVCowState *s = bs->opaque; - struct cow_header_v2 cow_header; - int bitmap_size; - int64_t size; - int ret; - - /* see if it is a cow image */ - ret = bdrv_pread(bs->file, 0, &cow_header, sizeof(cow_header)); - if (ret < 0) { - goto fail; - } - - if (be32_to_cpu(cow_header.magic) != COW_MAGIC) { - error_setg(errp, "Image not in COW format"); - ret = -EINVAL; - goto fail; - } - - if (be32_to_cpu(cow_header.version) != COW_VERSION) { - char version[64]; - snprintf(version, sizeof(version), - "COW version %" PRIu32, cow_header.version); - error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, - bs->device_name, "cow", version); - ret = -ENOTSUP; - goto fail; - } - - /* cow image found */ - size = be64_to_cpu(cow_header.size); - bs->total_sectors = size / 512; - - pstrcpy(bs->backing_file, sizeof(bs->backing_file), - cow_header.backing_file); - - bitmap_size = ((bs->total_sectors + 7) >> 3) + sizeof(cow_header); - s->cow_sectors_offset = (bitmap_size + 511) & ~511; - qemu_co_mutex_init(&s->lock); - return 0; - fail: - return ret; -} - -static inline void cow_set_bits(uint8_t *bitmap, int start, int64_t nb_sectors) -{ - int64_t bitnum = start, last = start + nb_sectors; - while (bitnum < last) { - if ((bitnum & 7) == 0 && bitnum + 8 <= last) { - bitmap[bitnum / 8] = 0xFF; - bitnum += 8; - continue; - } - bitmap[bitnum/8] |= (1 << (bitnum % 8)); - bitnum++; - } -} - -#define BITS_PER_BITMAP_SECTOR (512 * 8) - -/* Cannot use bitmap.c on big-endian machines. */ -static int cow_test_bit(int64_t bitnum, const uint8_t *bitmap) -{ - return (bitmap[bitnum / 8] & (1 << (bitnum & 7))) != 0; -} - -static int cow_find_streak(const uint8_t *bitmap, int value, int start, int nb_sectors) -{ - int streak_value = value ? 0xFF : 0; - int last = MIN(start + nb_sectors, BITS_PER_BITMAP_SECTOR); - int bitnum = start; - while (bitnum < last) { - if ((bitnum & 7) == 0 && bitmap[bitnum / 8] == streak_value) { - bitnum += 8; - continue; - } - if (cow_test_bit(bitnum, bitmap) == value) { - bitnum++; - continue; - } - break; - } - return MIN(bitnum, last) - start; -} - -/* Return true if first block has been changed (ie. current version is - * in COW file). Set the number of continuous blocks for which that - * is true. */ -static int coroutine_fn cow_co_is_allocated(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *num_same) -{ - int64_t bitnum = sector_num + sizeof(struct cow_header_v2) * 8; - uint64_t offset = (bitnum / 8) & -BDRV_SECTOR_SIZE; - bool first = true; - int changed = 0, same = 0; - - do { - int ret; - uint8_t bitmap[BDRV_SECTOR_SIZE]; - - bitnum &= BITS_PER_BITMAP_SECTOR - 1; - int sector_bits = MIN(nb_sectors, BITS_PER_BITMAP_SECTOR - bitnum); - - ret = bdrv_pread(bs->file, offset, &bitmap, sizeof(bitmap)); - if (ret < 0) { - return ret; - } - - if (first) { - changed = cow_test_bit(bitnum, bitmap); - first = false; - } - - same += cow_find_streak(bitmap, changed, bitnum, nb_sectors); - - bitnum += sector_bits; - nb_sectors -= sector_bits; - offset += BDRV_SECTOR_SIZE; - } while (nb_sectors); - - *num_same = same; - return changed; -} - -static int64_t coroutine_fn cow_co_get_block_status(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *num_same) -{ - BDRVCowState *s = bs->opaque; - int ret = cow_co_is_allocated(bs, sector_num, nb_sectors, num_same); - int64_t offset = s->cow_sectors_offset + (sector_num << BDRV_SECTOR_BITS); - if (ret < 0) { - return ret; - } - return (ret ? BDRV_BLOCK_DATA : 0) | offset | BDRV_BLOCK_OFFSET_VALID; -} - -static int cow_update_bitmap(BlockDriverState *bs, int64_t sector_num, - int nb_sectors) -{ - int64_t bitnum = sector_num + sizeof(struct cow_header_v2) * 8; - uint64_t offset = (bitnum / 8) & -BDRV_SECTOR_SIZE; - bool first = true; - int sector_bits; - - for ( ; nb_sectors; - bitnum += sector_bits, - nb_sectors -= sector_bits, - offset += BDRV_SECTOR_SIZE) { - int ret, set; - uint8_t bitmap[BDRV_SECTOR_SIZE]; - - bitnum &= BITS_PER_BITMAP_SECTOR - 1; - sector_bits = MIN(nb_sectors, BITS_PER_BITMAP_SECTOR - bitnum); - - ret = bdrv_pread(bs->file, offset, &bitmap, sizeof(bitmap)); - if (ret < 0) { - return ret; - } - - /* Skip over any already set bits */ - set = cow_find_streak(bitmap, 1, bitnum, sector_bits); - bitnum += set; - sector_bits -= set; - nb_sectors -= set; - if (!sector_bits) { - continue; - } - - if (first) { - ret = bdrv_flush(bs->file); - if (ret < 0) { - return ret; - } - first = false; - } - - cow_set_bits(bitmap, bitnum, sector_bits); - - ret = bdrv_pwrite(bs->file, offset, &bitmap, sizeof(bitmap)); - if (ret < 0) { - return ret; - } - } - - return 0; -} - -static int coroutine_fn cow_read(BlockDriverState *bs, int64_t sector_num, - uint8_t *buf, int nb_sectors) -{ - BDRVCowState *s = bs->opaque; - int ret, n; - - while (nb_sectors > 0) { - ret = cow_co_is_allocated(bs, sector_num, nb_sectors, &n); - if (ret < 0) { - return ret; - } - if (ret) { - ret = bdrv_pread(bs->file, - s->cow_sectors_offset + sector_num * 512, - buf, n * 512); - if (ret < 0) { - return ret; - } - } else { - if (bs->backing_hd) { - /* read from the base image */ - ret = bdrv_read(bs->backing_hd, sector_num, buf, n); - if (ret < 0) { - return ret; - } - } else { - memset(buf, 0, n * 512); - } - } - nb_sectors -= n; - sector_num += n; - buf += n * 512; - } - return 0; -} - -static coroutine_fn int cow_co_read(BlockDriverState *bs, int64_t sector_num, - uint8_t *buf, int nb_sectors) -{ - int ret; - BDRVCowState *s = bs->opaque; - qemu_co_mutex_lock(&s->lock); - ret = cow_read(bs, sector_num, buf, nb_sectors); - qemu_co_mutex_unlock(&s->lock); - return ret; -} - -static int cow_write(BlockDriverState *bs, int64_t sector_num, - const uint8_t *buf, int nb_sectors) -{ - BDRVCowState *s = bs->opaque; - int ret; - - ret = bdrv_pwrite(bs->file, s->cow_sectors_offset + sector_num * 512, - buf, nb_sectors * 512); - if (ret < 0) { - return ret; - } - - return cow_update_bitmap(bs, sector_num, nb_sectors); -} - -static coroutine_fn int cow_co_write(BlockDriverState *bs, int64_t sector_num, - const uint8_t *buf, int nb_sectors) -{ - int ret; - BDRVCowState *s = bs->opaque; - qemu_co_mutex_lock(&s->lock); - ret = cow_write(bs, sector_num, buf, nb_sectors); - qemu_co_mutex_unlock(&s->lock); - return ret; -} - -static void cow_close(BlockDriverState *bs) -{ -} - -static int cow_create(const char *filename, QemuOpts *opts, Error **errp) -{ - struct cow_header_v2 cow_header; - struct stat st; - int64_t image_sectors = 0; - char *image_filename = NULL; - Error *local_err = NULL; - int ret; - BlockDriverState *cow_bs = NULL; - - /* Read out options */ - image_sectors = DIV_ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), - BDRV_SECTOR_SIZE); - image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); - - ret = bdrv_create_file(filename, opts, &local_err); - if (ret < 0) { - error_propagate(errp, local_err); - goto exit; - } - - ret = bdrv_open(&cow_bs, filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err); - if (ret < 0) { - error_propagate(errp, local_err); - goto exit; - } - - memset(&cow_header, 0, sizeof(cow_header)); - cow_header.magic = cpu_to_be32(COW_MAGIC); - cow_header.version = cpu_to_be32(COW_VERSION); - if (image_filename) { - /* Note: if no file, we put a dummy mtime */ - cow_header.mtime = cpu_to_be32(0); - - if (stat(image_filename, &st) != 0) { - goto mtime_fail; - } - cow_header.mtime = cpu_to_be32(st.st_mtime); - mtime_fail: - pstrcpy(cow_header.backing_file, sizeof(cow_header.backing_file), - image_filename); - } - cow_header.sectorsize = cpu_to_be32(512); - cow_header.size = cpu_to_be64(image_sectors * 512); - ret = bdrv_pwrite(cow_bs, 0, &cow_header, sizeof(cow_header)); - if (ret < 0) { - goto exit; - } - - /* resize to include at least all the bitmap */ - ret = bdrv_truncate(cow_bs, - sizeof(cow_header) + ((image_sectors + 7) >> 3)); - if (ret < 0) { - goto exit; - } - -exit: - g_free(image_filename); - if (cow_bs) { - bdrv_unref(cow_bs); - } - return ret; -} - -static QemuOptsList cow_create_opts = { - .name = "cow-create-opts", - .head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head), - .desc = { - { - .name = BLOCK_OPT_SIZE, - .type = QEMU_OPT_SIZE, - .help = "Virtual disk size" - }, - { - .name = BLOCK_OPT_BACKING_FILE, - .type = QEMU_OPT_STRING, - .help = "File name of a base image" - }, - { /* end of list */ } - } -}; - -static BlockDriver bdrv_cow = { - .format_name = "cow", - .instance_size = sizeof(BDRVCowState), - - .bdrv_probe = cow_probe, - .bdrv_open = cow_open, - .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, - .bdrv_co_get_block_status = cow_co_get_block_status, - - .create_opts = &cow_create_opts, -}; - -static void bdrv_cow_init(void) -{ - bdrv_register(&bdrv_cow); -} - -block_init(bdrv_cow_init); -- cgit v1.1 From e91a8b2fef92e7d31290e785a35f85a503fa1356 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Tue, 16 Sep 2014 15:12:06 -0400 Subject: block: vhdx - fix reading beyond pointer during image creation In vhdx_create_metadata(), we allocate 40 bytes to entry_buffer for the various metadata table entries. However, we write out 64kB from that buffer into the new file. Only write out the correct 40 bytes. Signed-off-by: Jeff Cody Reviewed-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- block/vhdx.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'block') diff --git a/block/vhdx.c b/block/vhdx.c index d9aa2c1..22a4fb8 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1408,6 +1408,12 @@ exit: return ret; } +#define VHDX_METADATA_ENTRY_BUFFER_SIZE \ + (sizeof(VHDXFileParameters) +\ + sizeof(VHDXVirtualDiskSize) +\ + sizeof(VHDXPage83Data) +\ + sizeof(VHDXVirtualDiskLogicalSectorSize) +\ + sizeof(VHDXVirtualDiskPhysicalSectorSize)) /* * Create the Metadata entries. @@ -1446,11 +1452,7 @@ static int vhdx_create_new_metadata(BlockDriverState *bs, VHDXVirtualDiskLogicalSectorSize *mt_log_sector_size; VHDXVirtualDiskPhysicalSectorSize *mt_phys_sector_size; - entry_buffer = g_malloc0(sizeof(VHDXFileParameters) + - sizeof(VHDXVirtualDiskSize) + - sizeof(VHDXPage83Data) + - sizeof(VHDXVirtualDiskLogicalSectorSize) + - sizeof(VHDXVirtualDiskPhysicalSectorSize)); + entry_buffer = g_malloc0(VHDX_METADATA_ENTRY_BUFFER_SIZE); mt_file_params = entry_buffer; offset += sizeof(VHDXFileParameters); @@ -1531,7 +1533,7 @@ static int vhdx_create_new_metadata(BlockDriverState *bs, } ret = bdrv_pwrite(bs, metadata_offset + (64 * KiB), entry_buffer, - VHDX_HEADER_BLOCK_SIZE); + VHDX_METADATA_ENTRY_BUFFER_SIZE); if (ret < 0) { goto exit; } @@ -1726,7 +1728,6 @@ static int vhdx_create_new_region_table(BlockDriverState *bs, goto exit; } - exit: g_free(s); g_free(buffer); @@ -1877,7 +1878,6 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp) } - delete_and_exit: bdrv_unref(bs); exit: -- cgit v1.1