From b35278f75450e57c134a153e6da9744c1db8382f Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Fri, 15 Jun 2012 16:41:07 +0100 Subject: [PATCH 01/24] qcow2: fix #ifdef'd qcow2_check_refcounts() callers The DEBUG_ALLOC qcow2.h macro enables additional consistency checks throughout the code. This makes it easier to spot corruptions that are introduced during development. Since consistency check is an expensive operation the DEBUG_ALLOC macro is used to compile checks out in normal builds and qcow2_check_refcounts() calls missed the addition of a new function argument. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/qcow2-snapshot.c | 6 +++--- block/qcow2.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 4561a2abf9..4e7c93b8b3 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -405,7 +405,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) #ifdef DEBUG_ALLOC { BdrvCheckResult result = {0}; - qcow2_check_refcounts(bs, &result); + qcow2_check_refcounts(bs, &result, 0); } #endif return 0; @@ -522,7 +522,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) #ifdef DEBUG_ALLOC { BdrvCheckResult result = {0}; - qcow2_check_refcounts(bs, &result); + qcow2_check_refcounts(bs, &result, 0); } #endif return 0; @@ -582,7 +582,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id) #ifdef DEBUG_ALLOC { BdrvCheckResult result = {0}; - qcow2_check_refcounts(bs, &result); + qcow2_check_refcounts(bs, &result, 0); } #endif return 0; diff --git a/block/qcow2.c b/block/qcow2.c index 2c1cd0a446..5be5ace694 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -415,7 +415,7 @@ static int qcow2_open(BlockDriverState *bs, int flags) #ifdef DEBUG_ALLOC { BdrvCheckResult result = {0}; - qcow2_check_refcounts(bs, &result); + qcow2_check_refcounts(bs, &result, 0); } #endif return ret; From 206e6d8551839008b6858cf8f500d2e644d2b561 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 18 Jun 2012 14:00:57 +0100 Subject: [PATCH 02/24] qcow2: preserve free_byte_offset when qcow2_alloc_bytes() fails When qcow2_alloc_clusters() error handling code was introduced in commit 5d757b563d59142ca81e1073a8e8396750a0ad1a, the value of free_byte_offset was clobbered in the error case. This patch keeps free_byte_offset at 0 so we will try to allocate clusters again next time this function is called. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 66f391597c..5e3f9153fb 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -627,10 +627,11 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size) BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_BYTES); assert(size > 0 && size <= s->cluster_size); if (s->free_byte_offset == 0) { - s->free_byte_offset = qcow2_alloc_clusters(bs, s->cluster_size); - if (s->free_byte_offset < 0) { - return s->free_byte_offset; + offset = qcow2_alloc_clusters(bs, s->cluster_size); + if (offset < 0) { + return offset; } + s->free_byte_offset = offset; } redo: free_in_cluster = s->cluster_size - From 04d4abe96ca07b851fb74125390b1b626126aa86 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 27 Jun 2012 18:03:13 +0100 Subject: [PATCH 03/24] blockdev: warn when copy_on_read=on and readonly=on If the image is read-only then it's not possible to copy read data into it. Therefore copy-on-read is automatically disabled for read-only images. Up until now this behavior was silent, add a warning so the user knows why copy-on-read is not working. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- blockdev.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/blockdev.c b/blockdev.c index 9e0a72a269..a85a429aef 100644 --- a/blockdev.c +++ b/blockdev.c @@ -609,6 +609,10 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) bdrv_flags |= ro ? 0 : BDRV_O_RDWR; + if (ro && copy_on_read) { + error_report("warning: disabling copy_on_read on readonly drive"); + } + ret = bdrv_open(dinfo->bdrv, file, bdrv_flags, drv); if (ret < 0) { error_report("could not open disk image %s: %s", From 1b6ac9985aba27142d7dd6460bf20fc6ade92c00 Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Wed, 27 Jun 2012 07:26:18 +0900 Subject: [PATCH 04/24] sheepdog: fix dprintf format strings This fixes warnings about dprintf format in debug mode. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 8877f4528d..afd06aa5d0 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1571,11 +1571,11 @@ static int coroutine_fn sd_co_rw_vector(void *p) } if (create) { - dprintf("update ino (%" PRIu32") %" PRIu64 " %" PRIu64 - " %" PRIu64 "\n", inode->vdi_id, oid, + dprintf("update ino (%" PRIu32 ") %" PRIu64 " %" PRIu64 " %ld\n", + inode->vdi_id, oid, vid_to_data_oid(inode->data_vdi_id[idx], idx), idx); oid = vid_to_data_oid(inode->vdi_id, idx); - dprintf("new oid %lx\n", oid); + dprintf("new oid %" PRIx64 "\n", oid); } aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, old_oid, done); @@ -1726,7 +1726,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) SheepdogInode *inode; unsigned int datalen; - dprintf("sn_info: name %s id_str %s s: name %s vm_state_size %d " + dprintf("sn_info: name %s id_str %s s: name %s vm_state_size %" PRId64 " " "is_snapshot %d\n", sn_info->name, sn_info->id_str, s->name, sn_info->vm_state_size, s->is_snapshot); From 2dfcca3b6828052cadd30c66a1a840bf0fc6670c Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Wed, 27 Jun 2012 07:26:19 +0900 Subject: [PATCH 05/24] sheepdog: restart I/O when socket becomes ready in do_co_req() Currently, no one reenters the yielded coroutine. This fixes it. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/block/sheepdog.c b/block/sheepdog.c index afd06aa5d0..0b49c6d660 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -577,10 +577,21 @@ out: return ret; } +static void restart_co_req(void *opaque) +{ + Coroutine *co = opaque; + + qemu_coroutine_enter(co, NULL); +} + static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data, unsigned int *wlen, unsigned int *rlen) { int ret; + Coroutine *co; + + co = qemu_coroutine_self(); + qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, NULL, co); socket_set_block(sockfd); ret = send_co_req(sockfd, hdr, data, wlen); @@ -588,6 +599,8 @@ static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data, goto out; } + qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, NULL, co); + ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr)); if (ret < sizeof(*hdr)) { error_report("failed to get a rsp, %s", strerror(errno)); @@ -609,6 +622,7 @@ static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data, } ret = 0; out: + qemu_aio_set_fd_handler(sockfd, NULL, NULL, NULL, NULL); socket_set_nonblock(sockfd); return ret; } From b97564f4c5457b218c7dd56dde0554ce4765cae9 Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Wed, 27 Jun 2012 07:26:20 +0900 Subject: [PATCH 06/24] sheepdog: use coroutine based socket functions in coroutine context This removes blocking network I/Os in coroutine context. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 0b49c6d660..5dc1d7a348 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -541,11 +541,18 @@ static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data, return ret; } +static coroutine_fn int do_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen, unsigned int *rlen); + static int do_req(int sockfd, SheepdogReq *hdr, void *data, unsigned int *wlen, unsigned int *rlen) { int ret; + if (qemu_in_coroutine()) { + return do_co_req(sockfd, hdr, data, wlen, rlen); + } + socket_set_block(sockfd); ret = send_req(sockfd, hdr, data, wlen); if (ret < 0) { @@ -1642,7 +1649,6 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num, int ret; if (bs->growable && sector_num + nb_sectors > bs->total_sectors) { - /* TODO: shouldn't block here */ ret = sd_truncate(bs, (sector_num + nb_sectors) * SECTOR_SIZE); if (ret < 0) { return ret; @@ -1710,7 +1716,7 @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) hdr.opcode = SD_OP_FLUSH_VDI; hdr.oid = vid_to_vdi_oid(inode->vdi_id); - ret = do_co_req(s->flush_fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); + ret = do_req(s->flush_fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); if (ret) { error_report("failed to send a request to the sheep"); return ret; From 1d732d7d7ca1731b6b77e791ca57743449620fe3 Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Wed, 27 Jun 2012 07:26:21 +0900 Subject: [PATCH 07/24] sheepdog: make sure we don't free aiocb before sending all requests This patch increments the pending counter before sending requests, and make sures that aiocb is not freed while sending them. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 5dc1d7a348..d4e5e3aa9a 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -260,7 +260,6 @@ typedef struct AIOReq { uint32_t id; QLIST_ENTRY(AIOReq) outstanding_aio_siblings; - QLIST_ENTRY(AIOReq) aioreq_siblings; } AIOReq; enum AIOCBState { @@ -283,8 +282,7 @@ struct SheepdogAIOCB { void (*aio_done_func)(SheepdogAIOCB *); int canceled; - - QLIST_HEAD(aioreq_head, AIOReq) aioreq_head; + int nr_pending; }; typedef struct BDRVSheepdogState { @@ -388,19 +386,19 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb, QLIST_INSERT_HEAD(&s->outstanding_aio_head, aio_req, outstanding_aio_siblings); - QLIST_INSERT_HEAD(&acb->aioreq_head, aio_req, aioreq_siblings); + acb->nr_pending++; return aio_req; } -static inline int free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req) +static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req) { SheepdogAIOCB *acb = aio_req->aiocb; + QLIST_REMOVE(aio_req, outstanding_aio_siblings); - QLIST_REMOVE(aio_req, aioreq_siblings); g_free(aio_req); - return !QLIST_EMPTY(&acb->aioreq_head); + acb->nr_pending--; } static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb) @@ -446,7 +444,7 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov, acb->canceled = 0; acb->coroutine = qemu_coroutine_self(); acb->ret = 0; - QLIST_INIT(&acb->aioreq_head); + acb->nr_pending = 0; return acb; } @@ -663,7 +661,7 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid, ui if (ret < 0) { error_report("add_aio_request is failed"); free_aio_req(s, aio_req); - if (QLIST_EMPTY(&acb->aioreq_head)) { + if (!acb->nr_pending) { sd_finish_aiocb(acb); } } @@ -684,7 +682,6 @@ static void coroutine_fn aio_read_response(void *opaque) int ret; AIOReq *aio_req = NULL; SheepdogAIOCB *acb; - int rest; unsigned long idx; if (QLIST_EMPTY(&s->outstanding_aio_head)) { @@ -755,8 +752,8 @@ static void coroutine_fn aio_read_response(void *opaque) error_report("%s", sd_strerror(rsp.result)); } - rest = free_aio_req(s, aio_req); - if (!rest) { + free_aio_req(s, aio_req); + if (!acb->nr_pending) { /* * We've finished all requests which belong to the AIOCB, so * we can switch back to sd_co_readv/writev now. @@ -1568,6 +1565,12 @@ static int coroutine_fn sd_co_rw_vector(void *p) } } + /* + * Make sure we don't free the aiocb before we are done with all requests. + * This additional reference is dropped at the end of this function. + */ + acb->nr_pending++; + while (done != total) { uint8_t flags = 0; uint64_t old_oid = 0; @@ -1636,7 +1639,7 @@ static int coroutine_fn sd_co_rw_vector(void *p) done += len; } out: - if (QLIST_EMPTY(&acb->aioreq_head)) { + if (!--acb->nr_pending) { return acb->ret; } return 1; From c292ee6a67924061345975c02cd25b18f7054c4d Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Wed, 27 Jun 2012 07:26:22 +0900 Subject: [PATCH 08/24] sheepdog: split outstanding list into inflight and pending outstanding_list_head is used for both pending and inflight requests. This patch splits it and improves readability. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 49 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index d4e5e3aa9a..f6cd5172a0 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -259,7 +259,7 @@ typedef struct AIOReq { uint8_t flags; uint32_t id; - QLIST_ENTRY(AIOReq) outstanding_aio_siblings; + QLIST_ENTRY(AIOReq) aio_siblings; } AIOReq; enum AIOCBState { @@ -305,7 +305,8 @@ typedef struct BDRVSheepdogState { Coroutine *co_recv; uint32_t aioreq_seq_num; - QLIST_HEAD(outstanding_aio_head, AIOReq) outstanding_aio_head; + QLIST_HEAD(inflight_aio_head, AIOReq) inflight_aio_head; + QLIST_HEAD(pending_aio_head, AIOReq) pending_aio_head; } BDRVSheepdogState; static const char * sd_strerror(int err) @@ -356,7 +357,7 @@ static const char * sd_strerror(int err) * Sheepdog I/O handling: * * 1. In sd_co_rw_vector, we send the I/O requests to the server and - * link the requests to the outstanding_list in the + * link the requests to the inflight_list in the * BDRVSheepdogState. The function exits without waiting for * receiving the response. * @@ -384,9 +385,6 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb, aio_req->flags = flags; aio_req->id = s->aioreq_seq_num++; - QLIST_INSERT_HEAD(&s->outstanding_aio_head, aio_req, - outstanding_aio_siblings); - acb->nr_pending++; return aio_req; } @@ -395,7 +393,7 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req) { SheepdogAIOCB *acb = aio_req->aiocb; - QLIST_REMOVE(aio_req, outstanding_aio_siblings); + QLIST_REMOVE(aio_req, aio_siblings); g_free(aio_req); acb->nr_pending--; @@ -640,22 +638,21 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, * This function searchs pending requests to the object `oid', and * sends them. */ -static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid, uint32_t id) +static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid) { AIOReq *aio_req, *next; SheepdogAIOCB *acb; int ret; - QLIST_FOREACH_SAFE(aio_req, &s->outstanding_aio_head, - outstanding_aio_siblings, next) { - if (id == aio_req->id) { - continue; - } + QLIST_FOREACH_SAFE(aio_req, &s->pending_aio_head, aio_siblings, next) { if (aio_req->oid != oid) { continue; } acb = aio_req->aiocb; + /* move aio_req from pending list to inflight one */ + QLIST_REMOVE(aio_req, aio_siblings); + QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings); ret = add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, 0, acb->aiocb_type); if (ret < 0) { @@ -684,7 +681,7 @@ static void coroutine_fn aio_read_response(void *opaque) SheepdogAIOCB *acb; unsigned long idx; - if (QLIST_EMPTY(&s->outstanding_aio_head)) { + if (QLIST_EMPTY(&s->inflight_aio_head)) { goto out; } @@ -695,8 +692,8 @@ static void coroutine_fn aio_read_response(void *opaque) goto out; } - /* find the right aio_req from the outstanding_aio list */ - QLIST_FOREACH(aio_req, &s->outstanding_aio_head, outstanding_aio_siblings) { + /* find the right aio_req from the inflight aio list */ + QLIST_FOREACH(aio_req, &s->inflight_aio_head, aio_siblings) { if (aio_req->id == rsp.id) { break; } @@ -734,7 +731,7 @@ static void coroutine_fn aio_read_response(void *opaque) * create requests are not allowed, so we search the * pending requests here. */ - send_pending_req(s, vid_to_data_oid(s->inode.vdi_id, idx), rsp.id); + send_pending_req(s, vid_to_data_oid(s->inode.vdi_id, idx)); } break; case AIOCB_READ_UDATA: @@ -786,7 +783,8 @@ static int aio_flush_request(void *opaque) { BDRVSheepdogState *s = opaque; - return !QLIST_EMPTY(&s->outstanding_aio_head); + return !QLIST_EMPTY(&s->inflight_aio_head) || + !QLIST_EMPTY(&s->pending_aio_head); } static int set_nodelay(int fd) @@ -1103,7 +1101,8 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) strstart(filename, "sheepdog:", (const char **)&filename); - QLIST_INIT(&s->outstanding_aio_head); + QLIST_INIT(&s->inflight_aio_head); + QLIST_INIT(&s->pending_aio_head); s->fd = -1; memset(vdi, 0, sizeof(vdi)); @@ -1465,6 +1464,7 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb) iov.iov_len = sizeof(s->inode); aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id), data_len, offset, 0, 0, offset); + QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings); ret = add_aio_request(s, aio_req, &iov, 1, 0, AIOCB_WRITE_UDATA); if (ret) { free_aio_req(s, aio_req); @@ -1533,7 +1533,7 @@ out: * Send I/O requests to the server. * * This function sends requests to the server, links the requests to - * the outstanding_list in BDRVSheepdogState, and exits without + * the inflight_list in BDRVSheepdogState, and exits without * waiting the response. The responses are received in the * `aio_read_response' function which is called from the main loop as * a fd handler. @@ -1606,11 +1606,7 @@ static int coroutine_fn sd_co_rw_vector(void *p) if (create) { AIOReq *areq; - QLIST_FOREACH(areq, &s->outstanding_aio_head, - outstanding_aio_siblings) { - if (areq == aio_req) { - continue; - } + QLIST_FOREACH(areq, &s->inflight_aio_head, aio_siblings) { if (areq->oid == oid) { /* * Sheepdog cannot handle simultaneous create @@ -1620,11 +1616,14 @@ static int coroutine_fn sd_co_rw_vector(void *p) */ aio_req->flags = 0; aio_req->base_oid = 0; + QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, + aio_siblings); goto done; } } } + QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings); ret = add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create, acb->aiocb_type); if (ret < 0) { From 7dc1cde05bd8c63789edc03fedb71d2d68da1d4f Mon Sep 17 00:00:00 2001 From: MORITA Kazutaka Date: Wed, 27 Jun 2012 07:26:23 +0900 Subject: [PATCH 09/24] sheepdog: traverse pending_list from the first for each time The pending list can be modified in other coroutine context sd_co_rw_vector, so we need to traverse the list from the first again after we send the pending request. Signed-off-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index f6cd5172a0..6e73efbad1 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -634,21 +634,31 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, struct iovec *iov, int niov, int create, enum AIOCBState aiocb_type); + +static AIOReq *find_pending_req(BDRVSheepdogState *s, uint64_t oid) +{ + AIOReq *aio_req; + + QLIST_FOREACH(aio_req, &s->pending_aio_head, aio_siblings) { + if (aio_req->oid == oid) { + return aio_req; + } + } + + return NULL; +} + /* * This function searchs pending requests to the object `oid', and * sends them. */ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid) { - AIOReq *aio_req, *next; + AIOReq *aio_req; SheepdogAIOCB *acb; int ret; - QLIST_FOREACH_SAFE(aio_req, &s->pending_aio_head, aio_siblings, next) { - if (aio_req->oid != oid) { - continue; - } - + while ((aio_req = find_pending_req(s, oid)) != NULL) { acb = aio_req->aiocb; /* move aio_req from pending list to inflight one */ QLIST_REMOVE(aio_req, aio_siblings); From 820100fd15b66880df75415c6086a7ffeee7bf14 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 6 Jun 2012 08:10:39 +0200 Subject: [PATCH 10/24] blkdebug: remove sync i/o events These are unused, except (by mistake more or less) in QED. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block.h | 2 -- block/blkdebug.c | 2 -- block/qed.c | 2 +- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/block.h b/block.h index d135652902..42e30d6a7c 100644 --- a/block.h +++ b/block.h @@ -395,9 +395,7 @@ typedef enum { BLKDBG_L2_ALLOC_COW_READ, BLKDBG_L2_ALLOC_WRITE, - BLKDBG_READ, BLKDBG_READ_AIO, - BLKDBG_READ_BACKING, BLKDBG_READ_BACKING_AIO, BLKDBG_READ_COMPRESSED, diff --git a/block/blkdebug.c b/block/blkdebug.c index e56e37da51..1eff9404ba 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -147,9 +147,7 @@ static const char *event_names[BLKDBG_EVENT_MAX] = { [BLKDBG_L2_ALLOC_COW_READ] = "l2_alloc.cow_read", [BLKDBG_L2_ALLOC_WRITE] = "l2_alloc.write", - [BLKDBG_READ] = "read", [BLKDBG_READ_AIO] = "read_aio", - [BLKDBG_READ_BACKING] = "read_backing", [BLKDBG_READ_BACKING_AIO] = "read_backing_aio", [BLKDBG_READ_COMPRESSED] = "read_compressed", diff --git a/block/qed.c b/block/qed.c index ab5972466c..dd2832a93b 100644 --- a/block/qed.c +++ b/block/qed.c @@ -748,7 +748,7 @@ static void qed_read_backing_file(BDRVQEDState *s, uint64_t pos, /* If the read straddles the end of the backing file, shorten it */ size = MIN((uint64_t)backing_length - pos, qiov->size); - BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING); + BLKDBG_EVENT(s->bs->file, BLKDBG_READ_BACKING_AIO); bdrv_aio_readv(s->bs->backing_hd, pos / BDRV_SECTOR_SIZE, qiov, size / BDRV_SECTOR_SIZE, cb, opaque); } From 368e8dd10a24a529612f37ba7370262f5ea5a814 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 6 Jun 2012 08:10:40 +0200 Subject: [PATCH 11/24] blkdebug: tiny cleanup Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/blkdebug.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 1eff9404ba..1f79ef2a00 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -361,9 +361,7 @@ static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs, return inject_error(bs, cb, opaque); } - BlockDriverAIOCB *acb = - bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque); - return acb; + return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque); } static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs, @@ -376,9 +374,7 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs, return inject_error(bs, cb, opaque); } - BlockDriverAIOCB *acb = - bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque); - return acb; + return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque); } static void blkdebug_close(BlockDriverState *bs) From e130225587cb0d48b2c0b7c04b6bf9c95fe75ac9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 6 Jun 2012 08:10:41 +0200 Subject: [PATCH 12/24] blkdebug: pass getlength to underlying file This is required when using blkdebug with raw format. Unlike qcow2/QED, raw asks blkdebug for the length of the file, it doesn't get it from a header. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/blkdebug.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/block/blkdebug.c b/block/blkdebug.c index 1f79ef2a00..b084a232cf 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -429,6 +429,11 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkDebugEvent event) } } +static int64_t blkdebug_getlength(BlockDriverState *bs) +{ + return bdrv_getlength(bs->file); +} + static BlockDriver bdrv_blkdebug = { .format_name = "blkdebug", .protocol_name = "blkdebug", @@ -437,6 +442,7 @@ static BlockDriver bdrv_blkdebug = { .bdrv_file_open = blkdebug_open, .bdrv_close = blkdebug_close, + .bdrv_getlength = blkdebug_getlength, .bdrv_aio_readv = blkdebug_aio_readv, .bdrv_aio_writev = blkdebug_aio_writev, From 571cd43e57beb1f8fd42c60bd3c69777f8ecdc51 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 6 Jun 2012 08:10:42 +0200 Subject: [PATCH 13/24] blkdebug: store list of active rules This prepares for the next patch, where some active rules may actually not trigger depending on input to readv/writev. Store the active rules in a SIMPLEQ (so that it can be emptied easily with QSIMPLEQ_INIT), and fetch the errno/once/immediately arguments from there. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/blkdebug.c | 69 ++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index b084a232cf..d12ebbf9f9 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -26,24 +26,10 @@ #include "block_int.h" #include "module.h" -typedef struct BlkdebugVars { - int state; - - /* If inject_errno != 0, an error is injected for requests */ - int inject_errno; - - /* Decides if all future requests fail (false) or only the next one and - * after the next request inject_errno is reset to 0 (true) */ - bool inject_once; - - /* Decides if aio_readv/writev fails right away (true) or returns an error - * return value only in the callback (false) */ - bool inject_immediately; -} BlkdebugVars; - typedef struct BDRVBlkdebugState { - BlkdebugVars vars; - QLIST_HEAD(list, BlkdebugRule) rules[BLKDBG_EVENT_MAX]; + int state; + QLIST_HEAD(, BlkdebugRule) rules[BLKDBG_EVENT_MAX]; + QSIMPLEQ_HEAD(, BlkdebugRule) active_rules; } BDRVBlkdebugState; typedef struct BlkdebugAIOCB { @@ -79,6 +65,7 @@ typedef struct BlkdebugRule { } set_state; } options; QLIST_ENTRY(BlkdebugRule) next; + QSIMPLEQ_ENTRY(BlkdebugRule) active_next; } BlkdebugRule; static QemuOptsList inject_error_opts = { @@ -300,7 +287,7 @@ static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags) filename = c + 1; /* Set initial state */ - s->vars.state = 1; + s->state = 1; /* Open the backing file */ ret = bdrv_file_open(&bs->file, filename, flags); @@ -326,18 +313,18 @@ static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb) } static BlockDriverAIOCB *inject_error(BlockDriverState *bs, - BlockDriverCompletionFunc *cb, void *opaque) + BlockDriverCompletionFunc *cb, void *opaque, BlkdebugRule *rule) { BDRVBlkdebugState *s = bs->opaque; - int error = s->vars.inject_errno; + int error = rule->options.inject.error; struct BlkdebugAIOCB *acb; QEMUBH *bh; - if (s->vars.inject_once) { - s->vars.inject_errno = 0; + if (rule->options.inject.once) { + QSIMPLEQ_INIT(&s->active_rules); } - if (s->vars.inject_immediately) { + if (rule->options.inject.immediately) { return NULL; } @@ -356,9 +343,10 @@ static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque) { BDRVBlkdebugState *s = bs->opaque; + BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules); - if (s->vars.inject_errno) { - return inject_error(bs, cb, opaque); + if (rule && rule->options.inject.error) { + return inject_error(bs, cb, opaque, rule); } return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque); @@ -369,9 +357,10 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque) { BDRVBlkdebugState *s = bs->opaque; + BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules); - if (s->vars.inject_errno) { - return inject_error(bs, cb, opaque); + if (rule && rule->options.inject.error) { + return inject_error(bs, cb, opaque, rule); } return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque); @@ -391,41 +380,45 @@ static void blkdebug_close(BlockDriverState *bs) } } -static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, - BlkdebugVars *old_vars) +static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule, + int old_state, bool injected) { BDRVBlkdebugState *s = bs->opaque; - BlkdebugVars *vars = &s->vars; /* Only process rules for the current state */ - if (rule->state && rule->state != old_vars->state) { - return; + if (rule->state && rule->state != old_state) { + return injected; } /* Take the action */ switch (rule->action) { case ACTION_INJECT_ERROR: - vars->inject_errno = rule->options.inject.error; - vars->inject_once = rule->options.inject.once; - vars->inject_immediately = rule->options.inject.immediately; + if (!injected) { + QSIMPLEQ_INIT(&s->active_rules); + injected = true; + } + QSIMPLEQ_INSERT_HEAD(&s->active_rules, rule, active_next); break; case ACTION_SET_STATE: - vars->state = rule->options.set_state.new_state; + s->state = rule->options.set_state.new_state; break; } + return injected; } static void blkdebug_debug_event(BlockDriverState *bs, BlkDebugEvent event) { BDRVBlkdebugState *s = bs->opaque; struct BlkdebugRule *rule; - BlkdebugVars old_vars = s->vars; + int old_state = s->state; + bool injected; assert((int)event >= 0 && event < BLKDBG_EVENT_MAX); + injected = false; QLIST_FOREACH(rule, &s->rules[event], next) { - process_rule(bs, rule, &old_vars); + injected = process_rule(bs, rule, old_state, injected); } } From e4780db4293ed4380ce93ad77881b46f5ae59786 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 6 Jun 2012 08:10:43 +0200 Subject: [PATCH 14/24] blkdebug: optionally tie errors to a specific sector This makes blkdebug scripts more powerful, and independent of the exact sequence of operations performed by streaming. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/blkdebug.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index d12ebbf9f9..59dcea0650 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -59,6 +59,7 @@ typedef struct BlkdebugRule { int error; int immediately; int once; + int64_t sector; } inject; struct { int new_state; @@ -84,6 +85,10 @@ static QemuOptsList inject_error_opts = { .name = "errno", .type = QEMU_OPT_NUMBER, }, + { + .name = "sector", + .type = QEMU_OPT_NUMBER, + }, { .name = "once", .type = QEMU_OPT_BOOL, @@ -213,6 +218,7 @@ static int add_rule(QemuOpts *opts, void *opaque) rule->options.inject.once = qemu_opt_get_bool(opts, "once", 0); rule->options.inject.immediately = qemu_opt_get_bool(opts, "immediately", 0); + rule->options.inject.sector = qemu_opt_get_number(opts, "sector", -1); break; case ACTION_SET_STATE: @@ -343,7 +349,15 @@ static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque) { BDRVBlkdebugState *s = bs->opaque; - BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules); + BlkdebugRule *rule = NULL; + + QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) { + if (rule->options.inject.sector == -1 || + (rule->options.inject.sector >= sector_num && + rule->options.inject.sector < sector_num + nb_sectors)) { + break; + } + } if (rule && rule->options.inject.error) { return inject_error(bs, cb, opaque, rule); @@ -357,7 +371,15 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque) { BDRVBlkdebugState *s = bs->opaque; - BlkdebugRule *rule = QSIMPLEQ_FIRST(&s->active_rules); + BlkdebugRule *rule = NULL; + + QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) { + if (rule->options.inject.sector == -1 || + (rule->options.inject.sector >= sector_num && + rule->options.inject.sector < sector_num + nb_sectors)) { + break; + } + } if (rule && rule->options.inject.error) { return inject_error(bs, cb, opaque, rule); From 5c171afa4cff41101ac3e5b0cd703fd211aaa253 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 6 Jun 2012 08:10:44 +0200 Subject: [PATCH 15/24] raw: hook into blkdebug Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/raw.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/raw.c b/block/raw.c index 09d9b4878b..ff34ea41e7 100644 --- a/block/raw.c +++ b/block/raw.c @@ -12,12 +12,14 @@ static int raw_open(BlockDriverState *bs, int flags) static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) { + BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO); return bdrv_co_readv(bs->file, sector_num, nb_sectors, qiov); } static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) { + BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO); return bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov); } From a9fc4408e3511a073583a18b98a26765ff1e21d7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 14 Jun 2012 16:55:01 +0200 Subject: [PATCH 16/24] block: copy over job and dirty bitmap fields in bdrv_append While these should not be in use at the time a transaction is started, a command in the prepare phase of a transaction might have added them, so they need to be brought over. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/block.c b/block.c index 0acdcac158..702821dbab 100644 --- a/block.c +++ b/block.c @@ -1027,6 +1027,16 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) tmp.iostatus_enabled = bs_top->iostatus_enabled; tmp.iostatus = bs_top->iostatus; + /* dirty bitmap */ + tmp.dirty_count = bs_top->dirty_count; + tmp.dirty_bitmap = bs_top->dirty_bitmap; + assert(bs_new->dirty_bitmap == NULL); + + /* job */ + tmp.in_use = bs_top->in_use; + tmp.job = bs_top->job; + assert(bs_new->job == NULL); + /* keep the same entry in bdrv_states */ pstrcpy(tmp.device_name, sizeof(tmp.device_name), bs_top->device_name); tmp.list = bs_top->list; @@ -1051,6 +1061,11 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) /* clear the copied fields in the new backing file */ bdrv_detach_dev(bs_new, bs_new->dev); + bs_new->job = NULL; + bs_new->in_use = 0; + bs_new->dirty_bitmap = NULL; + bs_new->dirty_count = 0; + qemu_co_queue_init(&bs_new->throttled_reqs); memset(&bs_new->io_base, 0, sizeof(bs_new->io_base)); memset(&bs_new->io_limits, 0, sizeof(bs_new->io_limits)); From 4ddc07cac2bd794e4ff17717551e66589e71e714 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 14 Jun 2012 16:55:02 +0200 Subject: [PATCH 17/24] block: introduce bdrv_swap, implement bdrv_append on top of it The new function can be made a bit nicer than bdrv_append. It swaps the whole contents, and then swaps back (using the usual t=a;a=b;b=t idiom) the fields that need to stay on top. Thus, it does not need explicit bdrv_detach_dev, bdrv_iostatus_disable, etc. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block.c | 202 ++++++++++++++++++++++++++++++-------------------------- block.h | 1 + 2 files changed, 109 insertions(+), 94 deletions(-) diff --git a/block.c b/block.c index 702821dbab..29857dbe6d 100644 --- a/block.c +++ b/block.c @@ -971,6 +971,107 @@ static void bdrv_rebind(BlockDriverState *bs) } } +static void bdrv_move_feature_fields(BlockDriverState *bs_dest, + BlockDriverState *bs_src) +{ + /* move some fields that need to stay attached to the device */ + bs_dest->open_flags = bs_src->open_flags; + + /* dev info */ + bs_dest->dev_ops = bs_src->dev_ops; + bs_dest->dev_opaque = bs_src->dev_opaque; + bs_dest->dev = bs_src->dev; + bs_dest->buffer_alignment = bs_src->buffer_alignment; + bs_dest->copy_on_read = bs_src->copy_on_read; + + bs_dest->enable_write_cache = bs_src->enable_write_cache; + + /* i/o timing parameters */ + bs_dest->slice_time = bs_src->slice_time; + bs_dest->slice_start = bs_src->slice_start; + bs_dest->slice_end = bs_src->slice_end; + bs_dest->io_limits = bs_src->io_limits; + bs_dest->io_base = bs_src->io_base; + bs_dest->throttled_reqs = bs_src->throttled_reqs; + bs_dest->block_timer = bs_src->block_timer; + bs_dest->io_limits_enabled = bs_src->io_limits_enabled; + + /* geometry */ + bs_dest->cyls = bs_src->cyls; + bs_dest->heads = bs_src->heads; + bs_dest->secs = bs_src->secs; + bs_dest->translation = bs_src->translation; + + /* r/w error */ + bs_dest->on_read_error = bs_src->on_read_error; + bs_dest->on_write_error = bs_src->on_write_error; + + /* i/o status */ + bs_dest->iostatus_enabled = bs_src->iostatus_enabled; + bs_dest->iostatus = bs_src->iostatus; + + /* dirty bitmap */ + bs_dest->dirty_count = bs_src->dirty_count; + bs_dest->dirty_bitmap = bs_src->dirty_bitmap; + + /* job */ + bs_dest->in_use = bs_src->in_use; + bs_dest->job = bs_src->job; + + /* keep the same entry in bdrv_states */ + pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name), + bs_src->device_name); + bs_dest->list = bs_src->list; +} + +/* + * Swap bs contents for two image chains while they are live, + * while keeping required fields on the BlockDriverState that is + * actually attached to a device. + * + * This will modify the BlockDriverState fields, and swap contents + * between bs_new and bs_old. Both bs_new and bs_old are modified. + * + * bs_new is required to be anonymous. + * + * This function does not create any image files. + */ +void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) +{ + BlockDriverState tmp; + + /* bs_new must be anonymous and shouldn't have anything fancy enabled */ + assert(bs_new->device_name[0] == '\0'); + assert(bs_new->dirty_bitmap == NULL); + assert(bs_new->job == NULL); + assert(bs_new->dev == NULL); + assert(bs_new->in_use == 0); + assert(bs_new->io_limits_enabled == false); + assert(bs_new->block_timer == NULL); + + tmp = *bs_new; + *bs_new = *bs_old; + *bs_old = tmp; + + /* there are some fields that should not be swapped, move them back */ + bdrv_move_feature_fields(&tmp, bs_old); + bdrv_move_feature_fields(bs_old, bs_new); + bdrv_move_feature_fields(bs_new, &tmp); + + /* bs_new shouldn't be in bdrv_states even after the swap! */ + assert(bs_new->device_name[0] == '\0'); + + /* Check a few fields that should remain attached to the device */ + assert(bs_new->dev == NULL); + assert(bs_new->job == NULL); + assert(bs_new->in_use == 0); + assert(bs_new->io_limits_enabled == false); + assert(bs_new->block_timer == NULL); + + bdrv_rebind(bs_new); + bdrv_rebind(bs_old); +} + /* * Add new bs contents at the top of an image chain while the chain is * live, while keeping required fields on the top layer. @@ -984,103 +1085,16 @@ static void bdrv_rebind(BlockDriverState *bs) */ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) { - BlockDriverState tmp; - - /* bs_new must be anonymous */ - assert(bs_new->device_name[0] == '\0'); - - tmp = *bs_new; - - /* there are some fields that need to stay on the top layer: */ - tmp.open_flags = bs_top->open_flags; - - /* dev info */ - tmp.dev_ops = bs_top->dev_ops; - tmp.dev_opaque = bs_top->dev_opaque; - tmp.dev = bs_top->dev; - tmp.buffer_alignment = bs_top->buffer_alignment; - tmp.copy_on_read = bs_top->copy_on_read; - - tmp.enable_write_cache = bs_top->enable_write_cache; - - /* i/o timing parameters */ - tmp.slice_time = bs_top->slice_time; - tmp.slice_start = bs_top->slice_start; - tmp.slice_end = bs_top->slice_end; - tmp.io_limits = bs_top->io_limits; - tmp.io_base = bs_top->io_base; - tmp.throttled_reqs = bs_top->throttled_reqs; - tmp.block_timer = bs_top->block_timer; - tmp.io_limits_enabled = bs_top->io_limits_enabled; - - /* geometry */ - tmp.cyls = bs_top->cyls; - tmp.heads = bs_top->heads; - tmp.secs = bs_top->secs; - tmp.translation = bs_top->translation; - - /* r/w error */ - tmp.on_read_error = bs_top->on_read_error; - tmp.on_write_error = bs_top->on_write_error; - - /* i/o status */ - tmp.iostatus_enabled = bs_top->iostatus_enabled; - tmp.iostatus = bs_top->iostatus; - - /* dirty bitmap */ - tmp.dirty_count = bs_top->dirty_count; - tmp.dirty_bitmap = bs_top->dirty_bitmap; - assert(bs_new->dirty_bitmap == NULL); - - /* job */ - tmp.in_use = bs_top->in_use; - tmp.job = bs_top->job; - assert(bs_new->job == NULL); - - /* keep the same entry in bdrv_states */ - pstrcpy(tmp.device_name, sizeof(tmp.device_name), bs_top->device_name); - tmp.list = bs_top->list; + bdrv_swap(bs_new, bs_top); /* The contents of 'tmp' will become bs_top, as we are * swapping bs_new and bs_top contents. */ - tmp.backing_hd = bs_new; - pstrcpy(tmp.backing_file, sizeof(tmp.backing_file), bs_top->filename); - pstrcpy(tmp.backing_format, sizeof(tmp.backing_format), - bs_top->drv ? bs_top->drv->format_name : ""); - - /* swap contents of the fixed new bs and the current top */ - *bs_new = *bs_top; - *bs_top = tmp; - - /* device_name[] was carried over from the old bs_top. bs_new - * shouldn't be in bdrv_states, so we need to make device_name[] - * reflect the anonymity of bs_new - */ - bs_new->device_name[0] = '\0'; - - /* clear the copied fields in the new backing file */ - bdrv_detach_dev(bs_new, bs_new->dev); - - bs_new->job = NULL; - bs_new->in_use = 0; - bs_new->dirty_bitmap = NULL; - bs_new->dirty_count = 0; - - qemu_co_queue_init(&bs_new->throttled_reqs); - memset(&bs_new->io_base, 0, sizeof(bs_new->io_base)); - memset(&bs_new->io_limits, 0, sizeof(bs_new->io_limits)); - bdrv_iostatus_disable(bs_new); - - /* we don't use bdrv_io_limits_disable() for this, because we don't want - * to affect or delete the block_timer, as it has been moved to bs_top */ - bs_new->io_limits_enabled = false; - bs_new->block_timer = NULL; - bs_new->slice_time = 0; - bs_new->slice_start = 0; - bs_new->slice_end = 0; - - bdrv_rebind(bs_new); - bdrv_rebind(bs_top); + bs_top->backing_hd = bs_new; + bs_top->open_flags &= ~BDRV_O_NO_BACKING; + pstrcpy(bs_top->backing_file, sizeof(bs_top->backing_file), + bs_new->filename); + pstrcpy(bs_top->backing_format, sizeof(bs_top->backing_format), + bs_new->drv ? bs_new->drv->format_name : ""); } void bdrv_delete(BlockDriverState *bs) diff --git a/block.h b/block.h index 42e30d6a7c..3af93c6266 100644 --- a/block.h +++ b/block.h @@ -122,6 +122,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, int bdrv_create_file(const char* filename, QEMUOptionParameter *options); BlockDriverState *bdrv_new(const char *device_name); void bdrv_make_anon(BlockDriverState *bs); +void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old); void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top); void bdrv_delete(BlockDriverState *bs); int bdrv_parse_cache_flags(const char *mode, int *flags); From 6be01b1e0b14ad4809c9aec273c6109b91d2df1c Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Fri, 22 Jun 2012 12:33:54 +0200 Subject: [PATCH 18/24] fdc: rewrite seek and DSKCHG bit handling This bit is cleared on every successful seek to a different track (cylinder). The seek is also called on revalidate or on read/write/format commands which also clear the DSKCHG bit. Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- hw/fdc.c | 79 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index 5b3224b39b..0270264030 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -153,8 +153,12 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, } #endif drv->head = head; - if (drv->track != track) + if (drv->track != track) { + if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) { + drv->media_changed = 0; + } ret = 1; + } drv->track = track; drv->sect = sect; } @@ -170,9 +174,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, static void fd_recalibrate(FDrive *drv) { FLOPPY_DPRINTF("recalibrate\n"); - drv->head = 0; - drv->track = 0; - drv->sect = 1; + fd_seek(drv, 0, 0, 1, 1); } /* Revalidate a disk drive after a disk change */ @@ -711,14 +713,6 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0) qemu_set_irq(fdctrl->irq, 1); fdctrl->sra |= FD_SRA_INTPEND; } - if (status0 & FD_SR0_SEEK) { - FDrive *cur_drv; - /* A seek clears the disk change line (if a disk is inserted) */ - cur_drv = get_cur_drv(fdctrl); - if (cur_drv->bs != NULL && bdrv_is_inserted(cur_drv->bs)) { - cur_drv->media_changed = 0; - } - } fdctrl->reset_sensei = 0; fdctrl->status0 = status0; @@ -997,7 +991,10 @@ static void fdctrl_unimplemented(FDCtrl *fdctrl, int direction) fdctrl_set_fifo(fdctrl, 1, 0); } -/* Seek to next sector */ +/* Seek to next sector + * returns 0 when end of track reached (for DBL_SIDES on head 1) + * otherwise returns 1 + */ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv) { FLOPPY_DPRINTF("seek to next sector (%d %02x %02x => %d)\n", @@ -1005,30 +1002,39 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv) fd_sector(cur_drv)); /* XXX: cur_drv->sect >= cur_drv->last_sect should be an error in fact */ - if (cur_drv->sect >= cur_drv->last_sect || - cur_drv->sect == fdctrl->eot) { - cur_drv->sect = 1; + uint8_t new_head = cur_drv->head; + uint8_t new_track = cur_drv->track; + uint8_t new_sect = cur_drv->sect; + + int ret = 1; + + if (new_sect >= cur_drv->last_sect || + new_sect == fdctrl->eot) { + new_sect = 1; if (FD_MULTI_TRACK(fdctrl->data_state)) { - if (cur_drv->head == 0 && + if (new_head == 0 && (cur_drv->flags & FDISK_DBL_SIDES) != 0) { - cur_drv->head = 1; + new_head = 1; } else { - cur_drv->head = 0; - cur_drv->track++; - if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) - return 0; + new_head = 0; + new_track++; + if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) { + ret = 0; + } } } else { - cur_drv->track++; - return 0; + new_track++; + ret = 0; + } + if (ret == 1) { + FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n", + new_head, new_track, new_sect, fd_sector(cur_drv)); } - FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n", - cur_drv->head, cur_drv->track, - cur_drv->sect, fd_sector(cur_drv)); } else { - cur_drv->sect++; + new_sect++; } - return 1; + fd_seek(cur_drv, new_head, new_track, new_sect, 1); + return ret; } /* Callback for transfer end (stop or abort) */ @@ -1626,11 +1632,7 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction) /* The seek command just sends step pulses to the drive and doesn't care if * there is a medium inserted of if it's banging the head against the drive. */ - if (fdctrl->fifo[2] > cur_drv->max_track) { - cur_drv->track = cur_drv->max_track; - } else { - cur_drv->track = fdctrl->fifo[2]; - } + fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1); /* Raise Interrupt */ fdctrl_raise_irq(fdctrl, FD_SR0_SEEK); } @@ -1695,9 +1697,10 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK); cur_drv = get_cur_drv(fdctrl); if (fdctrl->fifo[2] + cur_drv->track >= cur_drv->max_track) { - cur_drv->track = cur_drv->max_track - 1; + fd_seek(cur_drv, cur_drv->head, cur_drv->max_track - 1, + cur_drv->sect, 1); } else { - cur_drv->track += fdctrl->fifo[2]; + fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ @@ -1711,9 +1714,9 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK); cur_drv = get_cur_drv(fdctrl); if (fdctrl->fifo[2] > cur_drv->track) { - cur_drv->track = 0; + fd_seek(cur_drv, cur_drv->head, 0, cur_drv->sect, 1); } else { - cur_drv->track -= fdctrl->fifo[2]; + fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ From 2fee00885a9ea4db69bbfc1ba8ccf95f2ae9aec6 Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Fri, 22 Jun 2012 12:33:55 +0200 Subject: [PATCH 19/24] fdc: fix interrupt handling If you call the SENSE INTERRUPT STATUS command while there is no interrupt waiting you get as result unknown command. Fixed status0 register handling for read/write/format commands. Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- hw/fdc.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index 0270264030..e28841c170 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -307,6 +307,9 @@ enum { }; enum { + FD_SR0_DS0 = 0x01, + FD_SR0_DS1 = 0x02, + FD_SR0_HEAD = 0x04, FD_SR0_EQPMT = 0x10, FD_SR0_SEEK = 0x20, FD_SR0_ABNTERM = 0x40, @@ -972,14 +975,15 @@ static void fdctrl_reset_fifo(FDCtrl *fdctrl) } /* Set FIFO status for the host to read */ -static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len, int do_irq) +static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len, uint8_t status0) { fdctrl->data_dir = FD_DIR_READ; fdctrl->data_len = fifo_len; fdctrl->data_pos = 0; fdctrl->msr |= FD_MSR_CMDBUSY | FD_MSR_RQM | FD_MSR_DIO; - if (do_irq) - fdctrl_raise_irq(fdctrl, 0x00); + if (status0) { + fdctrl_raise_irq(fdctrl, status0); + } } /* Set an error: unimplemented/unknown command */ @@ -1044,10 +1048,12 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0, FDrive *cur_drv; cur_drv = get_cur_drv(fdctrl); + fdctrl->status0 = status0 | FD_SR0_SEEK | (cur_drv->head << 2) | + GET_CUR_DRV(fdctrl); + FLOPPY_DPRINTF("transfer status: %02x %02x %02x (%02x)\n", - status0, status1, status2, - status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl)); - fdctrl->fifo[0] = status0 | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl); + status0, status1, status2, fdctrl->status0); + fdctrl->fifo[0] = fdctrl->status0; fdctrl->fifo[1] = status1; fdctrl->fifo[2] = status2; fdctrl->fifo[3] = cur_drv->track; @@ -1060,7 +1066,7 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0, } fdctrl->msr |= FD_MSR_RQM | FD_MSR_DIO; fdctrl->msr &= ~FD_MSR_NONDMA; - fdctrl_set_fifo(fdctrl, 7, 1); + fdctrl_set_fifo(fdctrl, 7, fdctrl->status0); } /* Prepare a data transfer (either DMA or FIFO) */ @@ -1175,7 +1181,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction) if (direction != FD_DIR_WRITE) fdctrl->msr |= FD_MSR_DIO; /* IO based transfer: calculate len */ - fdctrl_raise_irq(fdctrl, 0x00); + fdctrl_raise_irq(fdctrl, FD_SR0_SEEK); return; } @@ -1604,16 +1610,18 @@ static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction) { FDrive *cur_drv = get_cur_drv(fdctrl); - if(fdctrl->reset_sensei > 0) { + if (fdctrl->reset_sensei > 0) { fdctrl->fifo[0] = FD_SR0_RDYCHG + FD_RESET_SENSEI_COUNT - fdctrl->reset_sensei; fdctrl->reset_sensei--; + } else if (!(fdctrl->sra & FD_SRA_INTPEND)) { + fdctrl->fifo[0] = FD_SR0_INVCMD; + fdctrl_set_fifo(fdctrl, 1, 0); + return; } else { - /* XXX: status0 handling is broken for read/write - commands, so we do this hack. It should be suppressed - ASAP */ fdctrl->fifo[0] = - FD_SR0_SEEK | (cur_drv->head << 2) | GET_CUR_DRV(fdctrl); + (fdctrl->status0 & ~(FD_SR0_HEAD | FD_SR0_DS1 | FD_SR0_DS0)) + | GET_CUR_DRV(fdctrl); } fdctrl->fifo[1] = cur_drv->track; From 59240c349c64448cc79cb6400aae2186e3f9243e Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Wed, 4 Jul 2012 16:26:04 +0200 Subject: [PATCH 20/24] fdc_test: update media_change test After rewrite DSKCHG bit handling the test has to be updated. Now is needed to seek to different track to clear DSKCHG bit. Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- tests/fdc-test.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 610e2f1e26..5f52f6c826 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -156,19 +156,16 @@ static uint8_t send_read_command(void) return ret; } -static void send_step_pulse(void) +static void send_step_pulse(int cyl) { int drive = 0; int head = 0; - static int cyl = 0; floppy_send(CMD_SEEK); floppy_send(head << 2 | drive); g_assert(!get_irq(FLOPPY_IRQ)); floppy_send(cyl); ack_irq(); - - cyl = (cyl + 1) % 4; } static uint8_t cmos_read(uint8_t reg) @@ -195,8 +192,7 @@ static void test_no_media_on_start(void) assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); - send_step_pulse(); - send_step_pulse(); + send_step_pulse(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); @@ -227,7 +223,14 @@ static void test_media_change(void) dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); - send_step_pulse(); + send_step_pulse(0); + dir = inb(FLOPPY_BASE + reg_dir); + assert_bit_set(dir, DSKCHG); + dir = inb(FLOPPY_BASE + reg_dir); + assert_bit_set(dir, DSKCHG); + + /* Step to next track should clear DSKCHG bit. */ + send_step_pulse(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_clear(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); @@ -243,7 +246,13 @@ static void test_media_change(void) dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); - send_step_pulse(); + send_step_pulse(0); + dir = inb(FLOPPY_BASE + reg_dir); + assert_bit_set(dir, DSKCHG); + dir = inb(FLOPPY_BASE + reg_dir); + assert_bit_set(dir, DSKCHG); + + send_step_pulse(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); From b3ce604eeaa77970fa53838e7df2bc85344f2554 Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Wed, 4 Jul 2012 11:18:35 +0200 Subject: [PATCH 21/24] fdc_test: introduce test_sense_interrupt Calling sense interrupt status while there is no interrupt should return invalid command (0x80). Read command should always returns in st0 seek_end bit set to 1. Signed-off-by: Pavel Hrdina Signed-off-by: Kevin Wolf --- tests/fdc-test.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 5f52f6c826..585fb0e343 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -142,7 +142,7 @@ static uint8_t send_read_command(void) } st0 = floppy_recv(); - if (st0 != 0x40) { + if (st0 != 0x60) { ret = 1; } @@ -259,6 +259,28 @@ static void test_media_change(void) assert_bit_set(dir, DSKCHG); } +static void test_sense_interrupt(void) +{ + int drive = 0; + int head = 0; + int cyl = 0; + int ret = 0; + + floppy_send(CMD_SENSE_INT); + ret = floppy_recv(); + g_assert(ret == 0x80); + + floppy_send(CMD_SEEK); + floppy_send(head << 2 | drive); + g_assert(!get_irq(FLOPPY_IRQ)); + floppy_send(cyl); + + floppy_send(CMD_SENSE_INT); + ret = floppy_recv(); + g_assert(ret == 0x20); + floppy_recv(); +} + /* success if no crash or abort */ static void fuzz_registers(void) { @@ -306,6 +328,7 @@ int main(int argc, char **argv) qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start); qtest_add_func("/fdc/read_without_media", test_read_without_media); qtest_add_func("/fdc/media_change", test_media_change); + qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt); qtest_add_func("/fdc/fuzz-registers", fuzz_registers); ret = g_test_run(); From 1f69c2b022710222ff0379678e49f8bfb6c91233 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 29 Jun 2012 17:34:23 +0200 Subject: [PATCH 22/24] fdc: Drop broken code for user-defined floppy geometry bdrv_get_floppy_geometry_hint() fails to store through its parameter drive when bs has a geometry hint. Makes fd_revalidate() assign random crap to drv->drive. Has been broken that way for ages. Harmless, because: * The only way to set a geometry hint is -drive if=none,cyls=... Since commit c219331e, probably unintentional. * The only use of drv->drive is as argument to another bdrv_get_floppy_geometry_hint(). Which doesn't use it, since the geometry hint is still there. Drop the broken code, ignore -drive parameter cyls, heads and secs for floppies even with if=none, just like before commit c219331e. Matches -help, which explains cyls, heads, secs as "hard disk physical geometry". Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block.c | 62 +++++++++++++++++++++++++------------------------------- hw/fdc.c | 3 --- 2 files changed, 28 insertions(+), 37 deletions(-) diff --git a/block.c b/block.c index 29857dbe6d..f2540b9e0c 100644 --- a/block.c +++ b/block.c @@ -2337,46 +2337,40 @@ void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads, uint64_t nb_sectors, size; int i, first_match, match; - bdrv_get_geometry_hint(bs, nb_heads, max_track, last_sect); - if (*nb_heads != 0 && *max_track != 0 && *last_sect != 0) { - /* User defined disk */ - *rate = FDRIVE_RATE_500K; - } else { - bdrv_get_geometry(bs, &nb_sectors); - match = -1; - first_match = -1; - for (i = 0; ; i++) { - parse = &fd_formats[i]; - if (parse->drive == FDRIVE_DRV_NONE) { + bdrv_get_geometry(bs, &nb_sectors); + match = -1; + first_match = -1; + for (i = 0; ; i++) { + parse = &fd_formats[i]; + if (parse->drive == FDRIVE_DRV_NONE) { + break; + } + if (drive_in == parse->drive || + drive_in == FDRIVE_DRV_NONE) { + size = (parse->max_head + 1) * parse->max_track * + parse->last_sect; + if (nb_sectors == size) { + match = i; break; } - if (drive_in == parse->drive || - drive_in == FDRIVE_DRV_NONE) { - size = (parse->max_head + 1) * parse->max_track * - parse->last_sect; - if (nb_sectors == size) { - match = i; - break; - } - if (first_match == -1) { - first_match = i; - } - } - } - if (match == -1) { if (first_match == -1) { - match = 1; - } else { - match = first_match; + first_match = i; } - parse = &fd_formats[match]; } - *nb_heads = parse->max_head + 1; - *max_track = parse->max_track; - *last_sect = parse->last_sect; - *drive = parse->drive; - *rate = parse->rate; } + if (match == -1) { + if (first_match == -1) { + match = 1; + } else { + match = first_match; + } + parse = &fd_formats[match]; + } + *nb_heads = parse->max_head + 1; + *max_track = parse->max_track; + *last_sect = parse->last_sect; + *drive = parse->drive; + *rate = parse->rate; } int bdrv_get_translation_hint(BlockDriverState *bs) diff --git a/hw/fdc.c b/hw/fdc.c index e28841c170..edf07063b2 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -191,9 +191,6 @@ static void fd_revalidate(FDrive *drv) &last_sect, drv->drive, &drive, &rate); if (!bdrv_is_inserted(drv->bs)) { FLOPPY_DPRINTF("No disk in drive\n"); - } else if (nb_heads != 0 && max_track != 0 && last_sect != 0) { - FLOPPY_DPRINTF("User defined disk (%d %d %d)\n", - nb_heads - 1, max_track, last_sect); } else { FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads, max_track, last_sect, ro ? "ro" : "rw"); From bb494a505e17dd06a07a662b8c800f255ac387c4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 29 Jun 2012 17:34:27 +0200 Subject: [PATCH 23/24] qtest: Tidy up temporary files properly Each test litters /tmp with several files: a pid file and two sockets. Tidy up. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- tests/libqtest.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index 071b6be521..02d039218d 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -40,6 +40,7 @@ struct QTestState bool irq_level[MAX_IRQ]; GString *rx; gchar *pid_file; + char *socket_path, *qmp_socket_path; }; #define g_assert_no_errno(ret) do { \ @@ -88,8 +89,6 @@ QTestState *qtest_init(const char *extra_args) { QTestState *s; int sock, qmpsock, ret, i; - gchar *socket_path; - gchar *qmp_socket_path; gchar *pid_file; gchar *command; const char *qemu_binary; @@ -98,14 +97,14 @@ QTestState *qtest_init(const char *extra_args) qemu_binary = getenv("QTEST_QEMU_BINARY"); g_assert(qemu_binary != NULL); - socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid()); - qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid()); - pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid()); - s = g_malloc(sizeof(*s)); - sock = init_socket(socket_path); - qmpsock = init_socket(qmp_socket_path); + s->socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid()); + s->qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid()); + pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid()); + + sock = init_socket(s->socket_path); + qmpsock = init_socket(s->qmp_socket_path); pid = fork(); if (pid == 0) { @@ -115,8 +114,8 @@ QTestState *qtest_init(const char *extra_args) "-qmp unix:%s,nowait " "-pidfile %s " "-machine accel=qtest " - "%s", qemu_binary, socket_path, - qmp_socket_path, pid_file, + "%s", qemu_binary, s->socket_path, + s->qmp_socket_path, pid_file, extra_args ?: ""); ret = system(command); @@ -133,9 +132,6 @@ QTestState *qtest_init(const char *extra_args) s->irq_level[i] = false; } - g_free(socket_path); - g_free(qmp_socket_path); - /* Read the QMP greeting and then do the handshake */ qtest_qmp(s, ""); qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"); @@ -160,6 +156,13 @@ void qtest_quit(QTestState *s) fclose(f); } + + unlink(s->pid_file); + unlink(s->socket_path); + unlink(s->qmp_socket_path); + g_free(s->pid_file); + g_free(s->socket_path); + g_free(s->qmp_socket_path); } static void socket_sendf(int fd, const char *fmt, va_list ap) From 07d27a442e01870ea6fc2d6ffbe1df61dccaafc4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 29 Jun 2012 17:34:29 +0200 Subject: [PATCH 24/24] block: Factor bdrv_read_unthrottled() out of guess_disk_lchs() To prepare move of guess_disk_lchs() into hw/, where it poking BlockDriverState member io_limits_enabled directly would be unclean. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block.c | 24 +++++++++++++++++------- block.h | 2 ++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index f2540b9e0c..b410ed6467 100644 --- a/block.c +++ b/block.c @@ -1639,6 +1639,20 @@ int bdrv_read(BlockDriverState *bs, int64_t sector_num, return bdrv_rw_co(bs, sector_num, buf, nb_sectors, false); } +/* Just like bdrv_read(), but with I/O throttling temporarily disabled */ +int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num, + uint8_t *buf, int nb_sectors) +{ + bool enabled; + int ret; + + enabled = bs->io_limits_enabled; + bs->io_limits_enabled = false; + ret = bdrv_read(bs, 0, buf, 1); + bs->io_limits_enabled = enabled; + return ret; +} + #define BITS_PER_LONG (sizeof(unsigned long) * 8) static void set_dirty_bitmap(BlockDriverState *bs, int64_t sector_num, @@ -2136,11 +2150,10 @@ static int guess_disk_lchs(BlockDriverState *bs, int *pcylinders, int *pheads, int *psectors) { uint8_t buf[BDRV_SECTOR_SIZE]; - int ret, i, heads, sectors, cylinders; + int i, heads, sectors, cylinders; struct partition *p; uint32_t nr_sects; uint64_t nb_sectors; - bool enabled; bdrv_get_geometry(bs, &nb_sectors); @@ -2149,12 +2162,9 @@ static int guess_disk_lchs(BlockDriverState *bs, * but also in async I/O mode. So the I/O throttling function has to * be disabled temporarily here, not permanently. */ - enabled = bs->io_limits_enabled; - bs->io_limits_enabled = false; - ret = bdrv_read(bs, 0, buf, 1); - bs->io_limits_enabled = enabled; - if (ret < 0) + if (bdrv_read_unthrottled(bs, 0, buf, 1) < 0) { return -1; + } /* test msdos magic */ if (buf[510] != 0x55 || buf[511] != 0xaa) return -1; diff --git a/block.h b/block.h index 3af93c6266..e34d94210d 100644 --- a/block.h +++ b/block.h @@ -142,6 +142,8 @@ bool bdrv_dev_is_tray_open(BlockDriverState *bs); bool bdrv_dev_is_medium_locked(BlockDriverState *bs); int bdrv_read(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, int nb_sectors); +int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num, + uint8_t *buf, int nb_sectors); int bdrv_write(BlockDriverState *bs, int64_t sector_num, const uint8_t *buf, int nb_sectors); int bdrv_pread(BlockDriverState *bs, int64_t offset,