* [Qemu-devel] [PATCH 1/7] vdi: basic conversion to coroutines
2012-03-05 17:40 [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Paolo Bonzini
@ 2012-03-05 17:40 ` Paolo Bonzini
2012-03-05 17:40 ` [Qemu-devel] [PATCH 2/7] vdi: move end-of-I/O handling at the end Paolo Bonzini
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-03-05 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, sw
Even a basic conversion changing the bdrv_aio_readv/bdrv_aio_writev calls
to bdrv_co_readv/bdrv_co_writev, and callbacks to goto statements can
eliminate a lot of code. This is because error handling is simplified
and indirections through bottom halves can go away.
After this patch, I/O to the underlying file already happens via
coroutines, but the code still looks a lot like if asynchronous I/O was
being used.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/vdi.c | 158 ++++++++++++++---------------------------------------------
1 files changed, 37 insertions(+), 121 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 6a0011f..35edc90 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -160,7 +160,6 @@ typedef struct {
void *orig_buf;
bool is_write;
int header_modified;
- BlockDriverAIOCB *hd_aiocb;
struct iovec hd_iov;
QEMUIOVector hd_qiov;
QEMUBH *bh;
@@ -489,33 +488,19 @@ static int coroutine_fn vdi_co_is_allocated(BlockDriverState *bs,
return VDI_IS_ALLOCATED(bmap_entry);
}
-static void vdi_aio_cancel(BlockDriverAIOCB *blockacb)
-{
- /* TODO: This code is untested. How can I get it executed? */
- VdiAIOCB *acb = container_of(blockacb, VdiAIOCB, common);
- logout("\n");
- if (acb->hd_aiocb) {
- bdrv_aio_cancel(acb->hd_aiocb);
- }
- qemu_aio_release(acb);
-}
-
static AIOPool vdi_aio_pool = {
.aiocb_size = sizeof(VdiAIOCB),
- .cancel = vdi_aio_cancel,
};
static VdiAIOCB *vdi_aio_setup(BlockDriverState *bs, int64_t sector_num,
- QEMUIOVector *qiov, int nb_sectors,
- BlockDriverCompletionFunc *cb, void *opaque, int is_write)
+ QEMUIOVector *qiov, int nb_sectors, int is_write)
{
VdiAIOCB *acb;
logout("%p, %" PRId64 ", %p, %d, %p, %p, %d\n",
- bs, sector_num, qiov, nb_sectors, cb, opaque, is_write);
+ bs, sector_num, qiov, nb_sectors, is_write);
- acb = qemu_aio_get(&vdi_aio_pool, bs, cb, opaque);
- acb->hd_aiocb = NULL;
+ acb = qemu_aio_get(&vdi_aio_pool, bs, NULL, NULL);
acb->sector_num = sector_num;
acb->qiov = qiov;
acb->is_write = is_write;
@@ -538,42 +523,7 @@ static VdiAIOCB *vdi_aio_setup(BlockDriverState *bs, int64_t sector_num,
return acb;
}
-static int vdi_schedule_bh(QEMUBHFunc *cb, VdiAIOCB *acb)
-{
- logout("\n");
-
- if (acb->bh) {
- return -EIO;
- }
-
- acb->bh = qemu_bh_new(cb, acb);
- if (!acb->bh) {
- return -EIO;
- }
-
- qemu_bh_schedule(acb->bh);
-
- return 0;
-}
-
-static void vdi_aio_read_cb(void *opaque, int ret);
-static void vdi_aio_write_cb(void *opaque, int ret);
-
-static void vdi_aio_rw_bh(void *opaque)
-{
- VdiAIOCB *acb = opaque;
- logout("\n");
- qemu_bh_delete(acb->bh);
- acb->bh = NULL;
-
- if (acb->is_write) {
- vdi_aio_write_cb(opaque, 0);
- } else {
- vdi_aio_read_cb(opaque, 0);
- }
-}
-
-static void vdi_aio_read_cb(void *opaque, int ret)
+static int vdi_aio_read_cb(void *opaque, int ret)
{
VdiAIOCB *acb = opaque;
BlockDriverState *bs = acb->common.bs;
@@ -585,12 +535,7 @@ static void vdi_aio_read_cb(void *opaque, int ret)
logout("%u sectors read\n", acb->n_sectors);
- acb->hd_aiocb = NULL;
-
- if (ret < 0) {
- goto done;
- }
-
+restart:
acb->nb_sectors -= acb->n_sectors;
if (acb->nb_sectors == 0) {
@@ -618,10 +563,7 @@ static void vdi_aio_read_cb(void *opaque, int ret)
if (!VDI_IS_ALLOCATED(bmap_entry)) {
/* Block not allocated, return zeros, no need to wait. */
memset(acb->buf, 0, n_sectors * SECTOR_SIZE);
- ret = vdi_schedule_bh(vdi_aio_rw_bh, acb);
- if (ret < 0) {
- goto done;
- }
+ ret = 0;
} else {
uint64_t offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors +
@@ -629,41 +571,34 @@ static void vdi_aio_read_cb(void *opaque, int ret)
acb->hd_iov.iov_base = (void *)acb->buf;
acb->hd_iov.iov_len = n_sectors * SECTOR_SIZE;
qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- acb->hd_aiocb = bdrv_aio_readv(bs->file, offset, &acb->hd_qiov,
- n_sectors, vdi_aio_read_cb, acb);
+ ret = bdrv_co_readv(bs->file, offset, n_sectors, &acb->hd_qiov);
+ }
+ if (ret >= 0) {
+ goto restart;
}
- return;
+
done:
if (acb->qiov->niov > 1) {
qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size);
qemu_vfree(acb->orig_buf);
}
- acb->common.cb(acb->common.opaque, ret);
qemu_aio_release(acb);
+ return ret;
}
-static BlockDriverAIOCB *vdi_aio_readv(BlockDriverState *bs,
- int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
- BlockDriverCompletionFunc *cb, void *opaque)
+static int vdi_co_readv(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
VdiAIOCB *acb;
int ret;
logout("\n");
- acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, 0);
- ret = vdi_schedule_bh(vdi_aio_rw_bh, acb);
- if (ret < 0) {
- if (acb->qiov->niov > 1) {
- qemu_vfree(acb->orig_buf);
- }
- qemu_aio_release(acb);
- return NULL;
- }
-
- return &acb->common;
+ acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, 0);
+ ret = vdi_aio_read_cb(acb, 0);
+ return ret;
}
-static void vdi_aio_write_cb(void *opaque, int ret)
+static int vdi_aio_write_cb(void *opaque, int ret)
{
VdiAIOCB *acb = opaque;
BlockDriverState *bs = acb->common.bs;
@@ -673,12 +608,7 @@ static void vdi_aio_write_cb(void *opaque, int ret)
uint32_t sector_in_block;
uint32_t n_sectors;
- acb->hd_aiocb = NULL;
-
- if (ret < 0) {
- goto done;
- }
-
+restart:
acb->nb_sectors -= acb->n_sectors;
acb->sector_num += acb->n_sectors;
acb->buf += acb->n_sectors * SECTOR_SIZE;
@@ -686,6 +616,7 @@ static void vdi_aio_write_cb(void *opaque, int ret)
if (acb->nb_sectors == 0) {
logout("finished data write\n");
acb->n_sectors = 0;
+ ret = 0;
if (acb->header_modified) {
VdiHeader *header = acb->block_buffer;
logout("now writing modified header\n");
@@ -696,10 +627,9 @@ static void vdi_aio_write_cb(void *opaque, int ret)
acb->hd_iov.iov_base = acb->block_buffer;
acb->hd_iov.iov_len = SECTOR_SIZE;
qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- acb->hd_aiocb = bdrv_aio_writev(bs->file, 0, &acb->hd_qiov, 1,
- vdi_aio_write_cb, acb);
- return;
- } else if (VDI_IS_ALLOCATED(acb->bmap_first)) {
+ ret = bdrv_co_writev(bs->file, 0, 1, &acb->hd_qiov);
+ }
+ if (ret >= 0 && VDI_IS_ALLOCATED(acb->bmap_first)) {
/* One or more new blocks were allocated. */
uint64_t offset;
uint32_t bmap_first;
@@ -722,11 +652,8 @@ static void vdi_aio_write_cb(void *opaque, int ret)
qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
logout("will write %u block map sectors starting from entry %u\n",
n_sectors, bmap_first);
- acb->hd_aiocb = bdrv_aio_writev(bs->file, offset, &acb->hd_qiov,
- n_sectors, vdi_aio_write_cb, acb);
- return;
+ ret = bdrv_co_writev(bs->file, offset, n_sectors, &acb->hd_qiov);
}
- ret = 0;
goto done;
}
@@ -772,9 +699,7 @@ static void vdi_aio_write_cb(void *opaque, int ret)
acb->hd_iov.iov_base = (void *)block;
acb->hd_iov.iov_len = s->block_size;
qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- acb->hd_aiocb = bdrv_aio_writev(bs->file, offset,
- &acb->hd_qiov, s->block_sectors,
- vdi_aio_write_cb, acb);
+ ret = bdrv_co_writev(bs->file, offset, s->block_sectors, &acb->hd_qiov);
} else {
uint64_t offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors +
@@ -782,39 +707,30 @@ static void vdi_aio_write_cb(void *opaque, int ret)
acb->hd_iov.iov_base = (void *)acb->buf;
acb->hd_iov.iov_len = n_sectors * SECTOR_SIZE;
qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- acb->hd_aiocb = bdrv_aio_writev(bs->file, offset, &acb->hd_qiov,
- n_sectors, vdi_aio_write_cb, acb);
+ ret = bdrv_co_writev(bs->file, offset, n_sectors, &acb->hd_qiov);
+ }
+ if (ret >= 0) {
+ goto restart;
}
-
- return;
done:
if (acb->qiov->niov > 1) {
qemu_vfree(acb->orig_buf);
}
- acb->common.cb(acb->common.opaque, ret);
qemu_aio_release(acb);
+ return ret;
}
-static BlockDriverAIOCB *vdi_aio_writev(BlockDriverState *bs,
- int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
- BlockDriverCompletionFunc *cb, void *opaque)
+static int vdi_co_writev(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
VdiAIOCB *acb;
int ret;
logout("\n");
- acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, 1);
- ret = vdi_schedule_bh(vdi_aio_rw_bh, acb);
- if (ret < 0) {
- if (acb->qiov->niov > 1) {
- qemu_vfree(acb->orig_buf);
- }
- qemu_aio_release(acb);
- return NULL;
- }
-
- return &acb->common;
+ acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, 1);
+ ret = vdi_aio_write_cb(acb, 0);
+ return ret;
}
static int vdi_create(const char *filename, QEMUOptionParameter *options)
@@ -973,9 +889,9 @@ static BlockDriver bdrv_vdi = {
.bdrv_co_is_allocated = vdi_co_is_allocated,
.bdrv_make_empty = vdi_make_empty,
- .bdrv_aio_readv = vdi_aio_readv,
+ .bdrv_co_readv = vdi_co_readv,
#if defined(CONFIG_VDI_WRITE)
- .bdrv_aio_writev = vdi_aio_writev,
+ .bdrv_co_writev = vdi_co_writev,
#endif
.bdrv_get_info = vdi_get_info,
--
1.7.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 2/7] vdi: move end-of-I/O handling at the end
2012-03-05 17:40 [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Paolo Bonzini
2012-03-05 17:40 ` [Qemu-devel] [PATCH 1/7] vdi: basic conversion " Paolo Bonzini
@ 2012-03-05 17:40 ` Paolo Bonzini
2012-03-05 17:40 ` [Qemu-devel] [PATCH 3/7] vdi: merge aio_read_cb and aio_write_cb into callers Paolo Bonzini
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-03-05 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, sw
The next step is to take code that only triggers after the first operation,
and move it at the end of vdi_aio_read_cb and vdi_aio_write_cb.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/vdi.c | 123 +++++++++++++++++++++++++++--------------------------------
1 files changed, 56 insertions(+), 67 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 35edc90..4b780db 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -533,20 +533,7 @@ static int vdi_aio_read_cb(void *opaque, int ret)
uint32_t sector_in_block;
uint32_t n_sectors;
- logout("%u sectors read\n", acb->n_sectors);
-
restart:
- acb->nb_sectors -= acb->n_sectors;
-
- if (acb->nb_sectors == 0) {
- /* request completed */
- ret = 0;
- goto done;
- }
-
- acb->sector_num += acb->n_sectors;
- acb->buf += acb->n_sectors * SECTOR_SIZE;
-
block_index = acb->sector_num / s->block_sectors;
sector_in_block = acb->sector_num % s->block_sectors;
n_sectors = s->block_sectors - sector_in_block;
@@ -573,11 +560,16 @@ restart:
qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
ret = bdrv_co_readv(bs->file, offset, n_sectors, &acb->hd_qiov);
}
- if (ret >= 0) {
+ logout("%u sectors read\n", acb->n_sectors);
+
+ acb->nb_sectors -= acb->n_sectors;
+ acb->sector_num += acb->n_sectors;
+ acb->buf += acb->n_sectors * SECTOR_SIZE;
+
+ if (ret >= 0 && acb->nb_sectors > 0) {
goto restart;
}
-done:
if (acb->qiov->niov > 1) {
qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size);
qemu_vfree(acb->orig_buf);
@@ -609,56 +601,6 @@ static int vdi_aio_write_cb(void *opaque, int ret)
uint32_t n_sectors;
restart:
- acb->nb_sectors -= acb->n_sectors;
- acb->sector_num += acb->n_sectors;
- acb->buf += acb->n_sectors * SECTOR_SIZE;
-
- if (acb->nb_sectors == 0) {
- logout("finished data write\n");
- acb->n_sectors = 0;
- ret = 0;
- if (acb->header_modified) {
- VdiHeader *header = acb->block_buffer;
- logout("now writing modified header\n");
- assert(VDI_IS_ALLOCATED(acb->bmap_first));
- *header = s->header;
- vdi_header_to_le(header);
- acb->header_modified = 0;
- acb->hd_iov.iov_base = acb->block_buffer;
- acb->hd_iov.iov_len = SECTOR_SIZE;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- ret = bdrv_co_writev(bs->file, 0, 1, &acb->hd_qiov);
- }
- if (ret >= 0 && VDI_IS_ALLOCATED(acb->bmap_first)) {
- /* One or more new blocks were allocated. */
- uint64_t offset;
- uint32_t bmap_first;
- uint32_t bmap_last;
- g_free(acb->block_buffer);
- acb->block_buffer = NULL;
- bmap_first = acb->bmap_first;
- bmap_last = acb->bmap_last;
- logout("now writing modified block map entry %u...%u\n",
- bmap_first, bmap_last);
- /* Write modified sectors from block map. */
- bmap_first /= (SECTOR_SIZE / sizeof(uint32_t));
- bmap_last /= (SECTOR_SIZE / sizeof(uint32_t));
- n_sectors = bmap_last - bmap_first + 1;
- offset = s->bmap_sector + bmap_first;
- acb->bmap_first = VDI_UNALLOCATED;
- acb->hd_iov.iov_base = (void *)((uint8_t *)&s->bmap[0] +
- bmap_first * SECTOR_SIZE);
- acb->hd_iov.iov_len = n_sectors * SECTOR_SIZE;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- logout("will write %u block map sectors starting from entry %u\n",
- n_sectors, bmap_first);
- ret = bdrv_co_writev(bs->file, offset, n_sectors, &acb->hd_qiov);
- }
- goto done;
- }
-
- logout("%u sectors written\n", acb->n_sectors);
-
block_index = acb->sector_num / s->block_sectors;
sector_in_block = acb->sector_num % s->block_sectors;
n_sectors = s->block_sectors - sector_in_block;
@@ -709,11 +651,58 @@ restart:
qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
ret = bdrv_co_writev(bs->file, offset, n_sectors, &acb->hd_qiov);
}
- if (ret >= 0) {
+
+ acb->nb_sectors -= acb->n_sectors;
+ acb->sector_num += acb->n_sectors;
+ acb->buf += acb->n_sectors * SECTOR_SIZE;
+
+ logout("%u sectors written\n", acb->n_sectors);
+ if (ret >= 0 && acb->nb_sectors > 0) {
goto restart;
}
-done:
+ logout("finished data write\n");
+ if (ret >= 0) {
+ ret = 0;
+ if (acb->header_modified) {
+ VdiHeader *header = acb->block_buffer;
+ logout("now writing modified header\n");
+ assert(VDI_IS_ALLOCATED(acb->bmap_first));
+ *header = s->header;
+ vdi_header_to_le(header);
+ acb->header_modified = 0;
+ acb->hd_iov.iov_base = acb->block_buffer;
+ acb->hd_iov.iov_len = SECTOR_SIZE;
+ qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+ ret = bdrv_co_writev(bs->file, 0, 1, &acb->hd_qiov);
+ }
+ if (ret >= 0 && VDI_IS_ALLOCATED(acb->bmap_first)) {
+ /* One or more new blocks were allocated. */
+ uint64_t offset;
+ uint32_t bmap_first;
+ uint32_t bmap_last;
+ g_free(acb->block_buffer);
+ acb->block_buffer = NULL;
+ bmap_first = acb->bmap_first;
+ bmap_last = acb->bmap_last;
+ logout("now writing modified block map entry %u...%u\n",
+ bmap_first, bmap_last);
+ /* Write modified sectors from block map. */
+ bmap_first /= (SECTOR_SIZE / sizeof(uint32_t));
+ bmap_last /= (SECTOR_SIZE / sizeof(uint32_t));
+ n_sectors = bmap_last - bmap_first + 1;
+ offset = s->bmap_sector + bmap_first;
+ acb->bmap_first = VDI_UNALLOCATED;
+ acb->hd_iov.iov_base = (void *)((uint8_t *)&s->bmap[0] +
+ bmap_first * SECTOR_SIZE);
+ acb->hd_iov.iov_len = n_sectors * SECTOR_SIZE;
+ qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+ logout("will write %u block map sectors starting from entry %u\n",
+ n_sectors, bmap_first);
+ ret = bdrv_co_writev(bs->file, offset, n_sectors, &acb->hd_qiov);
+ }
+ }
+
if (acb->qiov->niov > 1) {
qemu_vfree(acb->orig_buf);
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 3/7] vdi: merge aio_read_cb and aio_write_cb into callers
2012-03-05 17:40 [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Paolo Bonzini
2012-03-05 17:40 ` [Qemu-devel] [PATCH 1/7] vdi: basic conversion " Paolo Bonzini
2012-03-05 17:40 ` [Qemu-devel] [PATCH 2/7] vdi: move end-of-I/O handling at the end Paolo Bonzini
@ 2012-03-05 17:40 ` Paolo Bonzini
2012-03-05 17:40 ` [Qemu-devel] [PATCH 4/7] vdi: move aiocb fields to locals Paolo Bonzini
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-03-05 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, sw
Now inline the former AIO callbacks into vdi_co_readv and vdi_co_writev.
While many cleanups are possible, the code now really looks synchronous.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/vdi.c | 40 ++++++++++++----------------------------
1 files changed, 12 insertions(+), 28 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 4b780db..9e5b169 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -523,15 +523,19 @@ static VdiAIOCB *vdi_aio_setup(BlockDriverState *bs, int64_t sector_num,
return acb;
}
-static int vdi_aio_read_cb(void *opaque, int ret)
+static int vdi_co_readv(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
- VdiAIOCB *acb = opaque;
- BlockDriverState *bs = acb->common.bs;
+ VdiAIOCB *acb;
BDRVVdiState *s = bs->opaque;
uint32_t bmap_entry;
uint32_t block_index;
uint32_t sector_in_block;
uint32_t n_sectors;
+ int ret;
+
+ logout("\n");
+ acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, 0);
restart:
block_index = acb->sector_num / s->block_sectors;
@@ -578,27 +582,19 @@ restart:
return ret;
}
-static int vdi_co_readv(BlockDriverState *bs,
+static int vdi_co_writev(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
VdiAIOCB *acb;
- int ret;
-
- logout("\n");
- acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, 0);
- ret = vdi_aio_read_cb(acb, 0);
- return ret;
-}
-
-static int vdi_aio_write_cb(void *opaque, int ret)
-{
- VdiAIOCB *acb = opaque;
- BlockDriverState *bs = acb->common.bs;
BDRVVdiState *s = bs->opaque;
uint32_t bmap_entry;
uint32_t block_index;
uint32_t sector_in_block;
uint32_t n_sectors;
+ int ret;
+
+ logout("\n");
+ acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, 1);
restart:
block_index = acb->sector_num / s->block_sectors;
@@ -710,18 +706,6 @@ restart:
return ret;
}
-static int vdi_co_writev(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
-{
- VdiAIOCB *acb;
- int ret;
-
- logout("\n");
- acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, 1);
- ret = vdi_aio_write_cb(acb, 0);
- return ret;
-}
-
static int vdi_create(const char *filename, QEMUOptionParameter *options)
{
int fd;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 4/7] vdi: move aiocb fields to locals
2012-03-05 17:40 [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Paolo Bonzini
` (2 preceding siblings ...)
2012-03-05 17:40 ` [Qemu-devel] [PATCH 3/7] vdi: merge aio_read_cb and aio_write_cb into callers Paolo Bonzini
@ 2012-03-05 17:40 ` Paolo Bonzini
2012-03-05 17:40 ` [Qemu-devel] [PATCH 5/7] vdi: leave bounce buffering to block layer Paolo Bonzini
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-03-05 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, sw
Most of the AIOCB really holds local variables that need to persist
across callback invocation. It can go away now.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/vdi.c | 162 ++++++++++++++++++++------------------------------
2 files changed, 65 insertions(+), 101 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 9e5b169..682583c 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -145,24 +145,8 @@ void uuid_unparse(const uuid_t uu, char *out)
typedef struct {
BlockDriverAIOCB common;
- int64_t sector_num;
- QEMUIOVector *qiov;
uint8_t *buf;
- /* Total number of sectors. */
- int nb_sectors;
- /* Number of sectors for current AIO. */
- int n_sectors;
- /* New allocated block map entry. */
- uint32_t bmap_first;
- uint32_t bmap_last;
- /* Buffer for new allocated block. */
- void *block_buffer;
void *orig_buf;
- bool is_write;
- int header_modified;
- struct iovec hd_iov;
- QEMUIOVector hd_qiov;
- QEMUBH *bh;
} VdiAIOCB;
typedef struct {
@@ -492,34 +476,20 @@ static AIOPool vdi_aio_pool = {
.aiocb_size = sizeof(VdiAIOCB),
};
-static VdiAIOCB *vdi_aio_setup(BlockDriverState *bs, int64_t sector_num,
- QEMUIOVector *qiov, int nb_sectors, int is_write)
+static VdiAIOCB *vdi_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov)
{
VdiAIOCB *acb;
- logout("%p, %" PRId64 ", %p, %d, %p, %p, %d\n",
- bs, sector_num, qiov, nb_sectors, is_write);
+ logout("%p, %p\n", bs, qiov);
acb = qemu_aio_get(&vdi_aio_pool, bs, NULL, NULL);
- acb->sector_num = sector_num;
- acb->qiov = qiov;
- acb->is_write = is_write;
if (qiov->niov > 1) {
acb->buf = qemu_blockalign(bs, qiov->size);
acb->orig_buf = acb->buf;
- if (is_write) {
- qemu_iovec_to_buffer(qiov, acb->buf);
- }
} else {
acb->buf = (uint8_t *)qiov->iov->iov_base;
}
- acb->nb_sectors = nb_sectors;
- acb->n_sectors = 0;
- acb->bmap_first = VDI_UNALLOCATED;
- acb->bmap_last = VDI_UNALLOCATED;
- acb->block_buffer = NULL;
- acb->header_modified = 0;
return acb;
}
@@ -532,24 +503,25 @@ static int vdi_co_readv(BlockDriverState *bs,
uint32_t block_index;
uint32_t sector_in_block;
uint32_t n_sectors;
+ struct iovec hd_iov;
+ QEMUIOVector hd_qiov;
int ret;
logout("\n");
- acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, 0);
+ acb = vdi_aio_setup(bs, qiov);
restart:
- block_index = acb->sector_num / s->block_sectors;
- sector_in_block = acb->sector_num % s->block_sectors;
+ block_index = sector_num / s->block_sectors;
+ sector_in_block = sector_num % s->block_sectors;
n_sectors = s->block_sectors - sector_in_block;
- if (n_sectors > acb->nb_sectors) {
- n_sectors = acb->nb_sectors;
+ if (n_sectors > nb_sectors) {
+ n_sectors = nb_sectors;
}
logout("will read %u sectors starting at sector %" PRIu64 "\n",
- n_sectors, acb->sector_num);
+ n_sectors, sector_num);
/* prepare next AIO request */
- acb->n_sectors = n_sectors;
bmap_entry = le32_to_cpu(s->bmap[block_index]);
if (!VDI_IS_ALLOCATED(bmap_entry)) {
/* Block not allocated, return zeros, no need to wait. */
@@ -559,23 +531,23 @@ restart:
uint64_t offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors +
sector_in_block;
- acb->hd_iov.iov_base = (void *)acb->buf;
- acb->hd_iov.iov_len = n_sectors * SECTOR_SIZE;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- ret = bdrv_co_readv(bs->file, offset, n_sectors, &acb->hd_qiov);
+ hd_iov.iov_base = (void *)acb->buf;
+ hd_iov.iov_len = n_sectors * SECTOR_SIZE;
+ qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
+ ret = bdrv_co_readv(bs->file, offset, n_sectors, &hd_qiov);
}
- logout("%u sectors read\n", acb->n_sectors);
+ logout("%u sectors read\n", n_sectors);
- acb->nb_sectors -= acb->n_sectors;
- acb->sector_num += acb->n_sectors;
- acb->buf += acb->n_sectors * SECTOR_SIZE;
+ nb_sectors -= n_sectors;
+ sector_num += n_sectors;
+ acb->buf += n_sectors * SECTOR_SIZE;
- if (ret >= 0 && acb->nb_sectors > 0) {
+ if (ret >= 0 && nb_sectors > 0) {
goto restart;
}
- if (acb->qiov->niov > 1) {
- qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size);
+ if (acb->orig_buf) {
+ qemu_iovec_from_buffer(qiov, acb->orig_buf, qiov->size);
qemu_vfree(acb->orig_buf);
}
qemu_aio_release(acb);
@@ -591,96 +563,93 @@ static int vdi_co_writev(BlockDriverState *bs,
uint32_t block_index;
uint32_t sector_in_block;
uint32_t n_sectors;
+ uint32_t bmap_first;
+ uint32_t bmap_last;
+ struct iovec hd_iov;
+ QEMUIOVector hd_qiov;
+ uint8_t *block = NULL;
int ret;
logout("\n");
- acb = vdi_aio_setup(bs, sector_num, qiov, nb_sectors, 1);
+ acb = vdi_aio_setup(bs, qiov);
+ if (acb->orig_buf) {
+ qemu_iovec_to_buffer(qiov, acb->buf);
+ }
restart:
- block_index = acb->sector_num / s->block_sectors;
- sector_in_block = acb->sector_num % s->block_sectors;
+ block_index = sector_num / s->block_sectors;
+ sector_in_block = sector_num % s->block_sectors;
n_sectors = s->block_sectors - sector_in_block;
- if (n_sectors > acb->nb_sectors) {
- n_sectors = acb->nb_sectors;
+ if (n_sectors > nb_sectors) {
+ n_sectors = nb_sectors;
}
logout("will write %u sectors starting at sector %" PRIu64 "\n",
- n_sectors, acb->sector_num);
+ n_sectors, sector_num);
/* prepare next AIO request */
- acb->n_sectors = n_sectors;
bmap_entry = le32_to_cpu(s->bmap[block_index]);
if (!VDI_IS_ALLOCATED(bmap_entry)) {
/* Allocate new block and write to it. */
uint64_t offset;
- uint8_t *block;
bmap_entry = s->header.blocks_allocated;
s->bmap[block_index] = cpu_to_le32(bmap_entry);
s->header.blocks_allocated++;
offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors;
- block = acb->block_buffer;
if (block == NULL) {
block = g_malloc(s->block_size);
- acb->block_buffer = block;
- acb->bmap_first = block_index;
- assert(!acb->header_modified);
- acb->header_modified = 1;
+ bmap_first = block_index;
}
- acb->bmap_last = block_index;
+ bmap_last = block_index;
/* Copy data to be written to new block and zero unused parts. */
memset(block, 0, sector_in_block * SECTOR_SIZE);
memcpy(block + sector_in_block * SECTOR_SIZE,
acb->buf, n_sectors * SECTOR_SIZE);
memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
(s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
- acb->hd_iov.iov_base = (void *)block;
- acb->hd_iov.iov_len = s->block_size;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- ret = bdrv_co_writev(bs->file, offset, s->block_sectors, &acb->hd_qiov);
+ hd_iov.iov_base = (void *)block;
+ hd_iov.iov_len = s->block_size;
+ qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
+ ret = bdrv_co_writev(bs->file, offset, s->block_sectors, &hd_qiov);
} else {
uint64_t offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors +
sector_in_block;
- acb->hd_iov.iov_base = (void *)acb->buf;
- acb->hd_iov.iov_len = n_sectors * SECTOR_SIZE;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- ret = bdrv_co_writev(bs->file, offset, n_sectors, &acb->hd_qiov);
+ hd_iov.iov_base = (void *)acb->buf;
+ hd_iov.iov_len = n_sectors * SECTOR_SIZE;
+ qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
+ ret = bdrv_co_writev(bs->file, offset, n_sectors, &hd_qiov);
}
- acb->nb_sectors -= acb->n_sectors;
- acb->sector_num += acb->n_sectors;
- acb->buf += acb->n_sectors * SECTOR_SIZE;
+ nb_sectors -= n_sectors;
+ sector_num += n_sectors;
+ acb->buf += n_sectors * SECTOR_SIZE;
- logout("%u sectors written\n", acb->n_sectors);
- if (ret >= 0 && acb->nb_sectors > 0) {
+ logout("%u sectors written\n", n_sectors);
+ if (ret >= 0 && nb_sectors > 0) {
goto restart;
}
logout("finished data write\n");
if (ret >= 0) {
ret = 0;
- if (acb->header_modified) {
- VdiHeader *header = acb->block_buffer;
+ if (block) {
+ VdiHeader *header = (VdiHeader *) block;
logout("now writing modified header\n");
- assert(VDI_IS_ALLOCATED(acb->bmap_first));
+ assert(VDI_IS_ALLOCATED(bmap_first));
*header = s->header;
vdi_header_to_le(header);
- acb->header_modified = 0;
- acb->hd_iov.iov_base = acb->block_buffer;
- acb->hd_iov.iov_len = SECTOR_SIZE;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
- ret = bdrv_co_writev(bs->file, 0, 1, &acb->hd_qiov);
+ hd_iov.iov_base = block;
+ hd_iov.iov_len = SECTOR_SIZE;
+ qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
+ ret = bdrv_co_writev(bs->file, 0, 1, &hd_qiov);
}
- if (ret >= 0 && VDI_IS_ALLOCATED(acb->bmap_first)) {
+ g_free(block);
+ block = NULL;
+ if (ret >= 0 && VDI_IS_ALLOCATED(bmap_first)) {
/* One or more new blocks were allocated. */
uint64_t offset;
- uint32_t bmap_first;
- uint32_t bmap_last;
- g_free(acb->block_buffer);
- acb->block_buffer = NULL;
- bmap_first = acb->bmap_first;
- bmap_last = acb->bmap_last;
logout("now writing modified block map entry %u...%u\n",
bmap_first, bmap_last);
/* Write modified sectors from block map. */
@@ -688,18 +657,17 @@ restart:
bmap_last /= (SECTOR_SIZE / sizeof(uint32_t));
n_sectors = bmap_last - bmap_first + 1;
offset = s->bmap_sector + bmap_first;
- acb->bmap_first = VDI_UNALLOCATED;
- acb->hd_iov.iov_base = (void *)((uint8_t *)&s->bmap[0] +
+ hd_iov.iov_base = (void *)((uint8_t *)&s->bmap[0] +
bmap_first * SECTOR_SIZE);
- acb->hd_iov.iov_len = n_sectors * SECTOR_SIZE;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+ hd_iov.iov_len = n_sectors * SECTOR_SIZE;
+ qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
logout("will write %u block map sectors starting from entry %u\n",
n_sectors, bmap_first);
- ret = bdrv_co_writev(bs->file, offset, n_sectors, &acb->hd_qiov);
+ ret = bdrv_co_writev(bs->file, offset, n_sectors, &hd_qiov);
}
}
- if (acb->qiov->niov > 1) {
+ if (acb->orig_buf) {
qemu_vfree(acb->orig_buf);
}
qemu_aio_release(acb);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 5/7] vdi: leave bounce buffering to block layer
2012-03-05 17:40 [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Paolo Bonzini
` (3 preceding siblings ...)
2012-03-05 17:40 ` [Qemu-devel] [PATCH 4/7] vdi: move aiocb fields to locals Paolo Bonzini
@ 2012-03-05 17:40 ` Paolo Bonzini
2012-03-16 9:54 ` Kevin Wolf
2012-03-05 17:40 ` [Qemu-devel] [PATCH 6/7] vdi: do not create useless iovecs Paolo Bonzini
` (2 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2012-03-05 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, sw
vdi.c really works as if it implemented bdrv_read and bdrv_write.
However, because only vector I/O is supported by the asynchronous
callbacks, it went through extra pain to bounce-buffer the I/O.
With the conversion to coroutines bdrv_read and bdrv_write are now
asynchronous, so they can be handled by the block layer now that the
format is coroutine-based.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/vdi.c | 68 ++++++++++------------------------------------------------
1 files changed, 12 insertions(+), 56 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 682583c..5070f91 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -144,12 +144,6 @@ void uuid_unparse(const uuid_t uu, char *out)
#endif
typedef struct {
- BlockDriverAIOCB common;
- uint8_t *buf;
- void *orig_buf;
-} VdiAIOCB;
-
-typedef struct {
char text[0x40];
uint32_t signature;
uint32_t version;
@@ -472,31 +466,9 @@ static int coroutine_fn vdi_co_is_allocated(BlockDriverState *bs,
return VDI_IS_ALLOCATED(bmap_entry);
}
-static AIOPool vdi_aio_pool = {
- .aiocb_size = sizeof(VdiAIOCB),
-};
-
-static VdiAIOCB *vdi_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov)
+static int vdi_co_read(BlockDriverState *bs,
+ int64_t sector_num, uint8_t *buf, int nb_sectors)
{
- VdiAIOCB *acb;
-
- logout("%p, %p\n", bs, qiov);
-
- acb = qemu_aio_get(&vdi_aio_pool, bs, NULL, NULL);
-
- if (qiov->niov > 1) {
- acb->buf = qemu_blockalign(bs, qiov->size);
- acb->orig_buf = acb->buf;
- } else {
- acb->buf = (uint8_t *)qiov->iov->iov_base;
- }
- return acb;
-}
-
-static int vdi_co_readv(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
-{
- VdiAIOCB *acb;
BDRVVdiState *s = bs->opaque;
uint32_t bmap_entry;
uint32_t block_index;
@@ -508,7 +479,6 @@ static int vdi_co_readv(BlockDriverState *bs,
int ret;
logout("\n");
- acb = vdi_aio_setup(bs, qiov);
restart:
block_index = sector_num / s->block_sectors;
@@ -525,13 +495,13 @@ restart:
bmap_entry = le32_to_cpu(s->bmap[block_index]);
if (!VDI_IS_ALLOCATED(bmap_entry)) {
/* Block not allocated, return zeros, no need to wait. */
- memset(acb->buf, 0, n_sectors * SECTOR_SIZE);
+ memset(buf, 0, n_sectors * SECTOR_SIZE);
ret = 0;
} else {
uint64_t offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors +
sector_in_block;
- hd_iov.iov_base = (void *)acb->buf;
+ hd_iov.iov_base = (void *)buf;
hd_iov.iov_len = n_sectors * SECTOR_SIZE;
qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
ret = bdrv_co_readv(bs->file, offset, n_sectors, &hd_qiov);
@@ -540,24 +510,18 @@ restart:
nb_sectors -= n_sectors;
sector_num += n_sectors;
- acb->buf += n_sectors * SECTOR_SIZE;
+ buf += n_sectors * SECTOR_SIZE;
if (ret >= 0 && nb_sectors > 0) {
goto restart;
}
- if (acb->orig_buf) {
- qemu_iovec_from_buffer(qiov, acb->orig_buf, qiov->size);
- qemu_vfree(acb->orig_buf);
- }
- qemu_aio_release(acb);
return ret;
}
-static int vdi_co_writev(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
+static int vdi_co_write(BlockDriverState *bs,
+ int64_t sector_num, const uint8_t *buf, int nb_sectors)
{
- VdiAIOCB *acb;
BDRVVdiState *s = bs->opaque;
uint32_t bmap_entry;
uint32_t block_index;
@@ -571,10 +535,6 @@ static int vdi_co_writev(BlockDriverState *bs,
int ret;
logout("\n");
- acb = vdi_aio_setup(bs, qiov);
- if (acb->orig_buf) {
- qemu_iovec_to_buffer(qiov, acb->buf);
- }
restart:
block_index = sector_num / s->block_sectors;
@@ -605,7 +565,7 @@ restart:
/* Copy data to be written to new block and zero unused parts. */
memset(block, 0, sector_in_block * SECTOR_SIZE);
memcpy(block + sector_in_block * SECTOR_SIZE,
- acb->buf, n_sectors * SECTOR_SIZE);
+ buf, n_sectors * SECTOR_SIZE);
memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
(s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
hd_iov.iov_base = (void *)block;
@@ -616,7 +576,7 @@ restart:
uint64_t offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors +
sector_in_block;
- hd_iov.iov_base = (void *)acb->buf;
+ hd_iov.iov_base = (void *)buf;
hd_iov.iov_len = n_sectors * SECTOR_SIZE;
qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
ret = bdrv_co_writev(bs->file, offset, n_sectors, &hd_qiov);
@@ -624,7 +584,7 @@ restart:
nb_sectors -= n_sectors;
sector_num += n_sectors;
- acb->buf += n_sectors * SECTOR_SIZE;
+ buf += n_sectors * SECTOR_SIZE;
logout("%u sectors written\n", n_sectors);
if (ret >= 0 && nb_sectors > 0) {
@@ -667,10 +627,6 @@ restart:
}
}
- if (acb->orig_buf) {
- qemu_vfree(acb->orig_buf);
- }
- qemu_aio_release(acb);
return ret;
}
@@ -830,9 +786,9 @@ static BlockDriver bdrv_vdi = {
.bdrv_co_is_allocated = vdi_co_is_allocated,
.bdrv_make_empty = vdi_make_empty,
- .bdrv_co_readv = vdi_co_readv,
+ .bdrv_read = vdi_co_read,
#if defined(CONFIG_VDI_WRITE)
- .bdrv_co_writev = vdi_co_writev,
+ .bdrv_write = vdi_co_write,
#endif
.bdrv_get_info = vdi_get_info,
--
1.7.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH 5/7] vdi: leave bounce buffering to block layer
2012-03-05 17:40 ` [Qemu-devel] [PATCH 5/7] vdi: leave bounce buffering to block layer Paolo Bonzini
@ 2012-03-16 9:54 ` Kevin Wolf
0 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2012-03-16 9:54 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: sw, qemu-devel
Am 05.03.2012 18:40, schrieb Paolo Bonzini:
> vdi.c really works as if it implemented bdrv_read and bdrv_write.
> However, because only vector I/O is supported by the asynchronous
> callbacks, it went through extra pain to bounce-buffer the I/O.
> With the conversion to coroutines bdrv_read and bdrv_write are now
> asynchronous, so they can be handled by the block layer now that the
> format is coroutine-based.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
In the long run, the right thing to do would be to convert VDI to deal
with vectored I/O, of course. Stefan, maybe you want to do that on top.
It's not that hard, I did it for qcow2 a while ago.
Kevin
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 6/7] vdi: do not create useless iovecs
2012-03-05 17:40 [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Paolo Bonzini
` (4 preceding siblings ...)
2012-03-05 17:40 ` [Qemu-devel] [PATCH 5/7] vdi: leave bounce buffering to block layer Paolo Bonzini
@ 2012-03-05 17:40 ` Paolo Bonzini
2012-03-05 17:40 ` [Qemu-devel] [PATCH 7/7] vdi: change goto to loop Paolo Bonzini
2012-03-15 21:28 ` [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Stefan Weil
7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-03-05 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, sw
Reads and writes to the underlying file can also occur with the simple
non-vectored I/O interfaces.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/vdi.c | 79 ++++++++++++++++++++++++----------------------------------
1 files changed, 33 insertions(+), 46 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index 5070f91..c192c7e 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -474,8 +474,6 @@ static int vdi_co_read(BlockDriverState *bs,
uint32_t block_index;
uint32_t sector_in_block;
uint32_t n_sectors;
- struct iovec hd_iov;
- QEMUIOVector hd_qiov;
int ret;
logout("\n");
@@ -501,10 +499,7 @@ restart:
uint64_t offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors +
sector_in_block;
- hd_iov.iov_base = (void *)buf;
- hd_iov.iov_len = n_sectors * SECTOR_SIZE;
- qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
- ret = bdrv_co_readv(bs->file, offset, n_sectors, &hd_qiov);
+ ret = bdrv_read(bs->file, offset, buf, n_sectors);
}
logout("%u sectors read\n", n_sectors);
@@ -529,8 +524,6 @@ static int vdi_co_write(BlockDriverState *bs,
uint32_t n_sectors;
uint32_t bmap_first;
uint32_t bmap_last;
- struct iovec hd_iov;
- QEMUIOVector hd_qiov;
uint8_t *block = NULL;
int ret;
@@ -568,18 +561,12 @@ restart:
buf, n_sectors * SECTOR_SIZE);
memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
(s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
- hd_iov.iov_base = (void *)block;
- hd_iov.iov_len = s->block_size;
- qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
- ret = bdrv_co_writev(bs->file, offset, s->block_sectors, &hd_qiov);
+ ret = bdrv_write(bs->file, offset, block, s->block_sectors);
} else {
uint64_t offset = s->header.offset_data / SECTOR_SIZE +
(uint64_t)bmap_entry * s->block_sectors +
sector_in_block;
- hd_iov.iov_base = (void *)buf;
- hd_iov.iov_len = n_sectors * SECTOR_SIZE;
- qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
- ret = bdrv_co_writev(bs->file, offset, n_sectors, &hd_qiov);
+ ret = bdrv_write(bs->file, offset, buf, n_sectors);
}
nb_sectors -= n_sectors;
@@ -592,39 +579,39 @@ restart:
}
logout("finished data write\n");
- if (ret >= 0) {
- ret = 0;
- if (block) {
- VdiHeader *header = (VdiHeader *) block;
- logout("now writing modified header\n");
- assert(VDI_IS_ALLOCATED(bmap_first));
- *header = s->header;
- vdi_header_to_le(header);
- hd_iov.iov_base = block;
- hd_iov.iov_len = SECTOR_SIZE;
- qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
- ret = bdrv_co_writev(bs->file, 0, 1, &hd_qiov);
- }
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (block) {
+ /* One or more new blocks were allocated. */
+ VdiHeader *header = (VdiHeader *) block;
+ uint8_t *base;
+ uint64_t offset;
+
+ logout("now writing modified header\n");
+ assert(VDI_IS_ALLOCATED(bmap_first));
+ *header = s->header;
+ vdi_header_to_le(header);
+ ret = bdrv_write(bs->file, 0, block, 1);
g_free(block);
block = NULL;
- if (ret >= 0 && VDI_IS_ALLOCATED(bmap_first)) {
- /* One or more new blocks were allocated. */
- uint64_t offset;
- logout("now writing modified block map entry %u...%u\n",
- bmap_first, bmap_last);
- /* Write modified sectors from block map. */
- bmap_first /= (SECTOR_SIZE / sizeof(uint32_t));
- bmap_last /= (SECTOR_SIZE / sizeof(uint32_t));
- n_sectors = bmap_last - bmap_first + 1;
- offset = s->bmap_sector + bmap_first;
- hd_iov.iov_base = (void *)((uint8_t *)&s->bmap[0] +
- bmap_first * SECTOR_SIZE);
- hd_iov.iov_len = n_sectors * SECTOR_SIZE;
- qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
- logout("will write %u block map sectors starting from entry %u\n",
- n_sectors, bmap_first);
- ret = bdrv_co_writev(bs->file, offset, n_sectors, &hd_qiov);
+
+ if (ret < 0) {
+ return ret;
}
+
+ logout("now writing modified block map entry %u...%u\n",
+ bmap_first, bmap_last);
+ /* Write modified sectors from block map. */
+ bmap_first /= (SECTOR_SIZE / sizeof(uint32_t));
+ bmap_last /= (SECTOR_SIZE / sizeof(uint32_t));
+ n_sectors = bmap_last - bmap_first + 1;
+ offset = s->bmap_sector + bmap_first;
+ base = ((uint8_t *)&s->bmap[0]) + bmap_first * SECTOR_SIZE;
+ logout("will write %u block map sectors starting from entry %u\n",
+ n_sectors, bmap_first);
+ ret = bdrv_write(bs->file, offset, base, n_sectors);
}
return ret;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH 7/7] vdi: change goto to loop
2012-03-05 17:40 [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Paolo Bonzini
` (5 preceding siblings ...)
2012-03-05 17:40 ` [Qemu-devel] [PATCH 6/7] vdi: do not create useless iovecs Paolo Bonzini
@ 2012-03-05 17:40 ` Paolo Bonzini
2012-03-15 21:28 ` [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Stefan Weil
7 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2012-03-05 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, sw
Finally reindent all code and change goto statements to a loop.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/vdi.c | 141 ++++++++++++++++++++++++++++------------------------------
1 files changed, 68 insertions(+), 73 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index c192c7e..41ec137 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -474,41 +474,38 @@ static int vdi_co_read(BlockDriverState *bs,
uint32_t block_index;
uint32_t sector_in_block;
uint32_t n_sectors;
- int ret;
+ int ret = 0;
logout("\n");
-restart:
- block_index = sector_num / s->block_sectors;
- sector_in_block = sector_num % s->block_sectors;
- n_sectors = s->block_sectors - sector_in_block;
- if (n_sectors > nb_sectors) {
- n_sectors = nb_sectors;
- }
-
- logout("will read %u sectors starting at sector %" PRIu64 "\n",
- n_sectors, sector_num);
+ while (ret >= 0 && nb_sectors > 0) {
+ block_index = sector_num / s->block_sectors;
+ sector_in_block = sector_num % s->block_sectors;
+ n_sectors = s->block_sectors - sector_in_block;
+ if (n_sectors > nb_sectors) {
+ n_sectors = nb_sectors;
+ }
- /* prepare next AIO request */
- bmap_entry = le32_to_cpu(s->bmap[block_index]);
- if (!VDI_IS_ALLOCATED(bmap_entry)) {
- /* Block not allocated, return zeros, no need to wait. */
- memset(buf, 0, n_sectors * SECTOR_SIZE);
- ret = 0;
- } else {
- uint64_t offset = s->header.offset_data / SECTOR_SIZE +
- (uint64_t)bmap_entry * s->block_sectors +
- sector_in_block;
- ret = bdrv_read(bs->file, offset, buf, n_sectors);
- }
- logout("%u sectors read\n", n_sectors);
+ logout("will read %u sectors starting at sector %" PRIu64 "\n",
+ n_sectors, sector_num);
- nb_sectors -= n_sectors;
- sector_num += n_sectors;
- buf += n_sectors * SECTOR_SIZE;
+ /* prepare next AIO request */
+ bmap_entry = le32_to_cpu(s->bmap[block_index]);
+ if (!VDI_IS_ALLOCATED(bmap_entry)) {
+ /* Block not allocated, return zeros, no need to wait. */
+ memset(buf, 0, n_sectors * SECTOR_SIZE);
+ ret = 0;
+ } else {
+ uint64_t offset = s->header.offset_data / SECTOR_SIZE +
+ (uint64_t)bmap_entry * s->block_sectors +
+ sector_in_block;
+ ret = bdrv_read(bs->file, offset, buf, n_sectors);
+ }
+ logout("%u sectors read\n", n_sectors);
- if (ret >= 0 && nb_sectors > 0) {
- goto restart;
+ nb_sectors -= n_sectors;
+ sector_num += n_sectors;
+ buf += n_sectors * SECTOR_SIZE;
}
return ret;
@@ -525,57 +522,55 @@ static int vdi_co_write(BlockDriverState *bs,
uint32_t bmap_first;
uint32_t bmap_last;
uint8_t *block = NULL;
- int ret;
+ int ret = 0;
logout("\n");
-restart:
- block_index = sector_num / s->block_sectors;
- sector_in_block = sector_num % s->block_sectors;
- n_sectors = s->block_sectors - sector_in_block;
- if (n_sectors > nb_sectors) {
- n_sectors = nb_sectors;
- }
-
- logout("will write %u sectors starting at sector %" PRIu64 "\n",
- n_sectors, sector_num);
+ while (ret >= 0 && nb_sectors > 0) {
+ block_index = sector_num / s->block_sectors;
+ sector_in_block = sector_num % s->block_sectors;
+ n_sectors = s->block_sectors - sector_in_block;
+ if (n_sectors > nb_sectors) {
+ n_sectors = nb_sectors;
+ }
- /* prepare next AIO request */
- bmap_entry = le32_to_cpu(s->bmap[block_index]);
- if (!VDI_IS_ALLOCATED(bmap_entry)) {
- /* Allocate new block and write to it. */
- uint64_t offset;
- bmap_entry = s->header.blocks_allocated;
- s->bmap[block_index] = cpu_to_le32(bmap_entry);
- s->header.blocks_allocated++;
- offset = s->header.offset_data / SECTOR_SIZE +
- (uint64_t)bmap_entry * s->block_sectors;
- if (block == NULL) {
- block = g_malloc(s->block_size);
- bmap_first = block_index;
+ logout("will write %u sectors starting at sector %" PRIu64 "\n",
+ n_sectors, sector_num);
+
+ /* prepare next AIO request */
+ bmap_entry = le32_to_cpu(s->bmap[block_index]);
+ if (!VDI_IS_ALLOCATED(bmap_entry)) {
+ /* Allocate new block and write to it. */
+ uint64_t offset;
+ bmap_entry = s->header.blocks_allocated;
+ s->bmap[block_index] = cpu_to_le32(bmap_entry);
+ s->header.blocks_allocated++;
+ offset = s->header.offset_data / SECTOR_SIZE +
+ (uint64_t)bmap_entry * s->block_sectors;
+ if (block == NULL) {
+ block = g_malloc(s->block_size);
+ bmap_first = block_index;
+ }
+ bmap_last = block_index;
+ /* Copy data to be written to new block and zero unused parts. */
+ memset(block, 0, sector_in_block * SECTOR_SIZE);
+ memcpy(block + sector_in_block * SECTOR_SIZE,
+ buf, n_sectors * SECTOR_SIZE);
+ memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
+ (s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
+ ret = bdrv_write(bs->file, offset, block, s->block_sectors);
+ } else {
+ uint64_t offset = s->header.offset_data / SECTOR_SIZE +
+ (uint64_t)bmap_entry * s->block_sectors +
+ sector_in_block;
+ ret = bdrv_write(bs->file, offset, buf, n_sectors);
}
- bmap_last = block_index;
- /* Copy data to be written to new block and zero unused parts. */
- memset(block, 0, sector_in_block * SECTOR_SIZE);
- memcpy(block + sector_in_block * SECTOR_SIZE,
- buf, n_sectors * SECTOR_SIZE);
- memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
- (s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
- ret = bdrv_write(bs->file, offset, block, s->block_sectors);
- } else {
- uint64_t offset = s->header.offset_data / SECTOR_SIZE +
- (uint64_t)bmap_entry * s->block_sectors +
- sector_in_block;
- ret = bdrv_write(bs->file, offset, buf, n_sectors);
- }
- nb_sectors -= n_sectors;
- sector_num += n_sectors;
- buf += n_sectors * SECTOR_SIZE;
+ nb_sectors -= n_sectors;
+ sector_num += n_sectors;
+ buf += n_sectors * SECTOR_SIZE;
- logout("%u sectors written\n", n_sectors);
- if (ret >= 0 && nb_sectors > 0) {
- goto restart;
+ logout("%u sectors written\n", n_sectors);
}
logout("finished data write\n");
--
1.7.7.6
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines
2012-03-05 17:40 [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Paolo Bonzini
` (6 preceding siblings ...)
2012-03-05 17:40 ` [Qemu-devel] [PATCH 7/7] vdi: change goto to loop Paolo Bonzini
@ 2012-03-15 21:28 ` Stefan Weil
2012-03-16 9:52 ` Kevin Wolf
7 siblings, 1 reply; 11+ messages in thread
From: Stefan Weil @ 2012-03-15 21:28 UTC (permalink / raw)
To: kwolf; +Cc: Paolo Bonzini, qemu-devel
Am 05.03.2012 18:40, schrieb Paolo Bonzini:
> Conversion to coroutines simplifies the code and removes the need to
> duplicate common features of the block layer. Each step in the conversion
> is detailed in the corresponding commit message.
>
> Tested with qemu-iotests.
>
> Paolo Bonzini (7):
> vdi: basic conversion to coroutines
> vdi: move end-of-I/O handling at the end
> vdi: merge aio_read_cb and aio_write_cb into callers
> vdi: move aiocb fields to locals
> vdi: leave bounce buffering to block layer
> vdi: do not create useless iovecs
> vdi: change goto to loop
>
> block/vdi.c | 421 +++++++++++++-------------------------------------
> 2 files changed, 108 insertions(+), 317 deletions(-)
Acked-by: Stefan Weil <sw@weilnetz.de>
Kevin, could you please add Paolo's patches to your block queue?
I did not review each single patch in detail, but the resulting
code looks sane and much better (and shorter) than the current code
(a little like my original code before I had to add asynchronous
I/O support).
As Paolo already tested with qemu-iotests, I did not re-run those
tests.
Thanks,
Stefan W.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines
2012-03-15 21:28 ` [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines Stefan Weil
@ 2012-03-16 9:52 ` Kevin Wolf
0 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2012-03-16 9:52 UTC (permalink / raw)
To: Stefan Weil; +Cc: Paolo Bonzini, qemu-devel
Am 15.03.2012 22:28, schrieb Stefan Weil:
> Am 05.03.2012 18:40, schrieb Paolo Bonzini:
>> Conversion to coroutines simplifies the code and removes the need to
>> duplicate common features of the block layer. Each step in the conversion
>> is detailed in the corresponding commit message.
>>
>> Tested with qemu-iotests.
>>
>> Paolo Bonzini (7):
>> vdi: basic conversion to coroutines
>> vdi: move end-of-I/O handling at the end
>> vdi: merge aio_read_cb and aio_write_cb into callers
>> vdi: move aiocb fields to locals
>> vdi: leave bounce buffering to block layer
>> vdi: do not create useless iovecs
>> vdi: change goto to loop
>>
>> block/vdi.c | 421 +++++++++++++-------------------------------------
>> 2 files changed, 108 insertions(+), 317 deletions(-)
>
> Acked-by: Stefan Weil <sw@weilnetz.de>
>
> Kevin, could you please add Paolo's patches to your block queue?
Yes, I was just waiting for your Acked-by.
Thanks, applied all patches to the block branch.
Kevin
^ permalink raw reply [flat|nested] 11+ messages in thread