* [Qemu-devel] [RFC PATCH 1/5] allocate AIO on stack
2011-07-20 7:56 [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup Frediano Ziglio
@ 2011-07-20 7:57 ` Frediano Ziglio
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 2/5] use more stack Frediano Ziglio
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Frediano Ziglio @ 2011-07-20 7:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Frediano Ziglio
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
block/qcow.c | 52 ++++++++++++++++------------------------------------
block/qcow2.c | 38 +++++++++++---------------------------
2 files changed, 27 insertions(+), 63 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 6447c2a..d19ef04 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -503,28 +503,12 @@ typedef struct QCowAIOCB {
BlockDriverAIOCB *hd_aiocb;
} QCowAIOCB;
-static void qcow_aio_cancel(BlockDriverAIOCB *blockacb)
-{
- QCowAIOCB *acb = container_of(blockacb, QCowAIOCB, common);
- if (acb->hd_aiocb)
- bdrv_aio_cancel(acb->hd_aiocb);
- qemu_aio_release(acb);
-}
-
-static AIOPool qcow_aio_pool = {
- .aiocb_size = sizeof(QCowAIOCB),
- .cancel = qcow_aio_cancel,
-};
-
static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
- int is_write)
+ int is_write, QCowAIOCB *acb)
{
- QCowAIOCB *acb;
-
- acb = qemu_aio_get(&qcow_aio_pool, bs, NULL, NULL);
- if (!acb)
- return NULL;
+ memset(acb, 0, sizeof(*acb));
+ acb->common.bs = bs;
acb->hd_aiocb = NULL;
acb->sector_num = sector_num;
acb->qiov = qiov;
@@ -543,9 +527,8 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
return acb;
}
-static int qcow_aio_read_cb(void *opaque)
+static int qcow_aio_read_cb(QCowAIOCB *acb)
{
- QCowAIOCB *acb = opaque;
BlockDriverState *bs = acb->common.bs;
BDRVQcowState *s = bs->opaque;
int index_in_cluster;
@@ -634,29 +617,27 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
BDRVQcowState *s = bs->opaque;
- QCowAIOCB *acb;
+ QCowAIOCB acb;
int ret;
- acb = qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 0);
+ qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 0, &acb);
qemu_co_mutex_lock(&s->lock);
do {
- ret = qcow_aio_read_cb(acb);
+ ret = qcow_aio_read_cb(&acb);
} while (ret > 0);
qemu_co_mutex_unlock(&s->lock);
- if (acb->qiov->niov > 1) {
- qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size);
- qemu_vfree(acb->orig_buf);
+ if (acb.qiov->niov > 1) {
+ qemu_iovec_from_buffer(acb.qiov, acb.orig_buf, acb.qiov->size);
+ qemu_vfree(acb.orig_buf);
}
- qemu_aio_release(acb);
return ret;
}
-static int qcow_aio_write_cb(void *opaque)
+static int qcow_aio_write_cb(QCowAIOCB *acb)
{
- QCowAIOCB *acb = opaque;
BlockDriverState *bs = acb->common.bs;
BDRVQcowState *s = bs->opaque;
int index_in_cluster;
@@ -714,23 +695,22 @@ static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
BDRVQcowState *s = bs->opaque;
- QCowAIOCB *acb;
+ QCowAIOCB acb;
int ret;
s->cluster_cache_offset = -1; /* disable compressed cache */
- acb = qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 1);
+ qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 1, &acb);
qemu_co_mutex_lock(&s->lock);
do {
- ret = qcow_aio_write_cb(acb);
+ ret = qcow_aio_write_cb(&acb);
} while (ret > 0);
qemu_co_mutex_unlock(&s->lock);
- if (acb->qiov->niov > 1) {
- qemu_vfree(acb->orig_buf);
+ if (acb.qiov->niov > 1) {
+ qemu_vfree(acb.orig_buf);
}
- qemu_aio_release(acb);
return ret;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index f07d550..edc068e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -388,17 +388,6 @@ typedef struct QCowAIOCB {
QLIST_ENTRY(QCowAIOCB) next_depend;
} QCowAIOCB;
-static void qcow2_aio_cancel(BlockDriverAIOCB *blockacb)
-{
- QCowAIOCB *acb = container_of(blockacb, QCowAIOCB, common);
- qemu_aio_release(acb);
-}
-
-static AIOPool qcow2_aio_pool = {
- .aiocb_size = sizeof(QCowAIOCB),
- .cancel = qcow2_aio_cancel,
-};
-
/*
* Returns 0 when the request is completed successfully, 1 when there is still
* a part left to do and -errno in error cases.
@@ -528,13 +517,10 @@ static int qcow2_aio_read_cb(QCowAIOCB *acb)
static QCowAIOCB *qcow2_aio_setup(BlockDriverState *bs, int64_t sector_num,
QEMUIOVector *qiov, int nb_sectors,
BlockDriverCompletionFunc *cb,
- void *opaque, int is_write)
+ void *opaque, int is_write, QCowAIOCB *acb)
{
- QCowAIOCB *acb;
-
- acb = qemu_aio_get(&qcow2_aio_pool, bs, cb, opaque);
- if (!acb)
- return NULL;
+ memset(acb, 0, sizeof(*acb));
+ acb->common.bs = bs;
acb->sector_num = sector_num;
acb->qiov = qiov;
acb->is_write = is_write;
@@ -554,19 +540,18 @@ static int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
BDRVQcowState *s = bs->opaque;
- QCowAIOCB *acb;
+ QCowAIOCB acb;
int ret;
- acb = qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 0);
+ qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 0, &acb);
qemu_co_mutex_lock(&s->lock);
do {
- ret = qcow2_aio_read_cb(acb);
+ ret = qcow2_aio_read_cb(&acb);
} while (ret > 0);
qemu_co_mutex_unlock(&s->lock);
- qemu_iovec_destroy(&acb->hd_qiov);
- qemu_aio_release(acb);
+ qemu_iovec_destroy(&acb.hd_qiov);
return ret;
}
@@ -670,20 +655,19 @@ static int qcow2_co_writev(BlockDriverState *bs,
QEMUIOVector *qiov)
{
BDRVQcowState *s = bs->opaque;
- QCowAIOCB *acb;
+ QCowAIOCB acb;
int ret;
- acb = qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 1);
+ qcow2_aio_setup(bs, sector_num, qiov, nb_sectors, NULL, NULL, 1, &acb);
s->cluster_cache_offset = -1; /* disable compressed cache */
qemu_co_mutex_lock(&s->lock);
do {
- ret = qcow2_aio_write_cb(acb);
+ ret = qcow2_aio_write_cb(&acb);
} while (ret > 0);
qemu_co_mutex_unlock(&s->lock);
- qemu_iovec_destroy(&acb->hd_qiov);
- qemu_aio_release(acb);
+ qemu_iovec_destroy(&acb.hd_qiov);
return ret;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [RFC PATCH 2/5] use more stack
2011-07-20 7:56 [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup Frediano Ziglio
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 1/5] allocate AIO on stack Frediano Ziglio
@ 2011-07-20 7:57 ` Frediano Ziglio
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 3/5] more stack work Frediano Ziglio
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Frediano Ziglio @ 2011-07-20 7:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Frediano Ziglio
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
block/qcow.c | 133 +++++++++++++++++++++++++++-------------------------------
1 files changed, 62 insertions(+), 71 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index d19ef04..cd1f9e3 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -487,20 +487,12 @@ static int qcow_read(BlockDriverState *bs, int64_t sector_num,
#endif
typedef struct QCowAIOCB {
- BlockDriverAIOCB common;
+ BlockDriverState *bs;
int64_t sector_num;
QEMUIOVector *qiov;
uint8_t *buf;
void *orig_buf;
int nb_sectors;
- int n;
- uint64_t cluster_offset;
- uint8_t *cluster_data;
- struct iovec hd_iov;
- bool is_write;
- QEMUBH *bh;
- QEMUIOVector hd_qiov;
- BlockDriverAIOCB *hd_aiocb;
} QCowAIOCB;
static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
@@ -508,11 +500,9 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
int is_write, QCowAIOCB *acb)
{
memset(acb, 0, sizeof(*acb));
- acb->common.bs = bs;
- acb->hd_aiocb = NULL;
+ acb->bs = bs;
acb->sector_num = sector_num;
acb->qiov = qiov;
- acb->is_write = is_write;
if (qiov->niov > 1) {
acb->buf = acb->orig_buf = qemu_blockalign(bs, qiov->size);
@@ -522,37 +512,36 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
acb->buf = (uint8_t *)qiov->iov->iov_base;
}
acb->nb_sectors = nb_sectors;
- acb->n = 0;
- acb->cluster_offset = 0;
return acb;
}
static int qcow_aio_read_cb(QCowAIOCB *acb)
{
- BlockDriverState *bs = acb->common.bs;
+ BlockDriverState *bs = acb->bs;
BDRVQcowState *s = bs->opaque;
int index_in_cluster;
- int ret;
-
- acb->hd_aiocb = NULL;
+ int ret, n = 0;
+ uint64_t cluster_offset = 0;
+ struct iovec hd_iov;
+ QEMUIOVector hd_qiov;
redo:
/* post process the read buffer */
- if (!acb->cluster_offset) {
+ if (!cluster_offset) {
/* nothing to do */
- } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
+ } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
/* nothing to do */
} else {
if (s->crypt_method) {
encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
- acb->n, 0,
+ n, 0,
&s->aes_decrypt_key);
}
}
- acb->nb_sectors -= acb->n;
- acb->sector_num += acb->n;
- acb->buf += acb->n * 512;
+ acb->nb_sectors -= n;
+ acb->sector_num += n;
+ acb->buf += n * 512;
if (acb->nb_sectors == 0) {
/* request completed */
@@ -560,57 +549,57 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
}
/* prepare next AIO request */
- acb->cluster_offset = get_cluster_offset(bs, acb->sector_num << 9,
+ cluster_offset = get_cluster_offset(bs, acb->sector_num << 9,
0, 0, 0, 0);
index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
- acb->n = s->cluster_sectors - index_in_cluster;
- if (acb->n > acb->nb_sectors)
- acb->n = acb->nb_sectors;
+ n = s->cluster_sectors - index_in_cluster;
+ if (n > acb->nb_sectors)
+ n = acb->nb_sectors;
- if (!acb->cluster_offset) {
+ if (!cluster_offset) {
if (bs->backing_hd) {
/* read from the base image */
- acb->hd_iov.iov_base = (void *)acb->buf;
- acb->hd_iov.iov_len = acb->n * 512;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+ hd_iov.iov_base = (void *)acb->buf;
+ hd_iov.iov_len = n * 512;
+ qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
qemu_co_mutex_unlock(&s->lock);
ret = bdrv_co_readv(bs->backing_hd, acb->sector_num,
- acb->n, &acb->hd_qiov);
+ n, &hd_qiov);
qemu_co_mutex_lock(&s->lock);
if (ret < 0) {
return -EIO;
}
} else {
/* Note: in this case, no need to wait */
- memset(acb->buf, 0, 512 * acb->n);
+ memset(acb->buf, 0, 512 * n);
goto redo;
}
- } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
+ } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
/* add AIO support for compressed blocks ? */
- if (decompress_cluster(bs, acb->cluster_offset) < 0) {
+ if (decompress_cluster(bs, cluster_offset) < 0) {
return -EIO;
}
memcpy(acb->buf,
- s->cluster_cache + index_in_cluster * 512, 512 * acb->n);
+ s->cluster_cache + index_in_cluster * 512, 512 * n);
goto redo;
} else {
- if ((acb->cluster_offset & 511) != 0) {
+ if ((cluster_offset & 511) != 0) {
return -EIO;
}
- acb->hd_iov.iov_base = (void *)acb->buf;
- acb->hd_iov.iov_len = acb->n * 512;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+ hd_iov.iov_base = (void *)acb->buf;
+ hd_iov.iov_len = n * 512;
+ qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
qemu_co_mutex_unlock(&s->lock);
ret = bdrv_co_readv(bs->file,
- (acb->cluster_offset >> 9) + index_in_cluster,
- acb->n, &acb->hd_qiov);
+ (cluster_offset >> 9) + index_in_cluster,
+ n, &hd_qiov);
qemu_co_mutex_lock(&s->lock);
if (ret < 0) {
return ret;
}
}
- return 1;
+ goto redo;
}
static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -623,9 +612,7 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 0, &acb);
qemu_co_mutex_lock(&s->lock);
- do {
- ret = qcow_aio_read_cb(&acb);
- } while (ret > 0);
+ ret = qcow_aio_read_cb(&acb);
qemu_co_mutex_unlock(&s->lock);
if (acb.qiov->niov > 1) {
@@ -638,18 +625,20 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
static int qcow_aio_write_cb(QCowAIOCB *acb)
{
- BlockDriverState *bs = acb->common.bs;
+ BlockDriverState *bs = acb->bs;
BDRVQcowState *s = bs->opaque;
int index_in_cluster;
uint64_t cluster_offset;
const uint8_t *src_buf;
- int ret;
-
- acb->hd_aiocb = NULL;
+ int ret, n = 0;
+ uint8_t *cluster_data = NULL;
+ struct iovec hd_iov;
+ QEMUIOVector hd_qiov;
- acb->nb_sectors -= acb->n;
- acb->sector_num += acb->n;
- acb->buf += acb->n * 512;
+ redo:
+ acb->nb_sectors -= n;
+ acb->sector_num += n;
+ acb->buf += n * 512;
if (acb->nb_sectors == 0) {
/* request completed */
@@ -657,38 +646,42 @@ static int qcow_aio_write_cb(QCowAIOCB *acb)
}
index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
- acb->n = s->cluster_sectors - index_in_cluster;
- if (acb->n > acb->nb_sectors)
- acb->n = acb->nb_sectors;
+ n = s->cluster_sectors - index_in_cluster;
+ if (n > acb->nb_sectors)
+ n = acb->nb_sectors;
cluster_offset = get_cluster_offset(bs, acb->sector_num << 9, 1, 0,
index_in_cluster,
- index_in_cluster + acb->n);
+ index_in_cluster + n);
if (!cluster_offset || (cluster_offset & 511) != 0) {
return -EIO;
}
if (s->crypt_method) {
- if (!acb->cluster_data) {
- acb->cluster_data = qemu_mallocz(s->cluster_size);
+ if (!cluster_data) {
+ cluster_data = qemu_mallocz(s->cluster_size);
}
- encrypt_sectors(s, acb->sector_num, acb->cluster_data, acb->buf,
- acb->n, 1, &s->aes_encrypt_key);
- src_buf = acb->cluster_data;
+ encrypt_sectors(s, acb->sector_num, cluster_data, acb->buf,
+ n, 1, &s->aes_encrypt_key);
+ src_buf = cluster_data;
} else {
src_buf = acb->buf;
}
- acb->hd_iov.iov_base = (void *)src_buf;
- acb->hd_iov.iov_len = acb->n * 512;
- qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
+ hd_iov.iov_base = (void *)src_buf;
+ hd_iov.iov_len = n * 512;
+ qemu_iovec_init_external(&hd_qiov, &hd_iov, 1);
qemu_co_mutex_unlock(&s->lock);
ret = bdrv_co_writev(bs->file,
(cluster_offset >> 9) + index_in_cluster,
- acb->n, &acb->hd_qiov);
+ n, &hd_qiov);
+ if (cluster_data) {
+ free(cluster_data);
+ cluster_data = NULL;
+ }
qemu_co_mutex_lock(&s->lock);
if (ret < 0) {
return ret;
}
- return 1;
+ goto redo;
}
static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
@@ -703,9 +696,7 @@ static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
qcow_aio_setup(bs, sector_num, qiov, nb_sectors, 1, &acb);
qemu_co_mutex_lock(&s->lock);
- do {
- ret = qcow_aio_write_cb(&acb);
- } while (ret > 0);
+ ret = qcow_aio_write_cb(&acb);
qemu_co_mutex_unlock(&s->lock);
if (acb.qiov->niov > 1) {
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [RFC PATCH 3/5] more stack work
2011-07-20 7:56 [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup Frediano Ziglio
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 1/5] allocate AIO on stack Frediano Ziglio
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 2/5] use more stack Frediano Ziglio
@ 2011-07-20 7:57 ` Frediano Ziglio
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 4/5] avoid dandling pointers Frediano Ziglio
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Frediano Ziglio @ 2011-07-20 7:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Frediano Ziglio
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
block/qcow.c | 53 ++++++++++++++++++++++++++---------------------------
1 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index cd1f9e3..8ccd7d7 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -520,35 +520,18 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
BlockDriverState *bs = acb->bs;
BDRVQcowState *s = bs->opaque;
int index_in_cluster;
- int ret, n = 0;
- uint64_t cluster_offset = 0;
+ int ret, n;
+ uint64_t cluster_offset;
struct iovec hd_iov;
QEMUIOVector hd_qiov;
redo:
- /* post process the read buffer */
- if (!cluster_offset) {
- /* nothing to do */
- } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
- /* nothing to do */
- } else {
- if (s->crypt_method) {
- encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
- n, 0,
- &s->aes_decrypt_key);
- }
- }
-
- acb->nb_sectors -= n;
- acb->sector_num += n;
- acb->buf += n * 512;
-
if (acb->nb_sectors == 0) {
/* request completed */
return 0;
}
- /* prepare next AIO request */
+ /* prepare next request */
cluster_offset = get_cluster_offset(bs, acb->sector_num << 9,
0, 0, 0, 0);
index_in_cluster = acb->sector_num & (s->cluster_sectors - 1);
@@ -572,7 +555,6 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
} else {
/* Note: in this case, no need to wait */
memset(acb->buf, 0, 512 * n);
- goto redo;
}
} else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
/* add AIO support for compressed blocks ? */
@@ -581,7 +563,6 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
}
memcpy(acb->buf,
s->cluster_cache + index_in_cluster * 512, 512 * n);
- goto redo;
} else {
if ((cluster_offset & 511) != 0) {
return -EIO;
@@ -599,6 +580,23 @@ static int qcow_aio_read_cb(QCowAIOCB *acb)
}
}
+ /* post process the read buffer */
+ if (!cluster_offset) {
+ /* nothing to do */
+ } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
+ /* nothing to do */
+ } else {
+ if (s->crypt_method) {
+ encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
+ n, 0,
+ &s->aes_decrypt_key);
+ }
+ }
+
+ acb->nb_sectors -= n;
+ acb->sector_num += n;
+ acb->buf += n * 512;
+
goto redo;
}
@@ -630,16 +628,12 @@ static int qcow_aio_write_cb(QCowAIOCB *acb)
int index_in_cluster;
uint64_t cluster_offset;
const uint8_t *src_buf;
- int ret, n = 0;
+ int ret, n;
uint8_t *cluster_data = NULL;
struct iovec hd_iov;
QEMUIOVector hd_qiov;
redo:
- acb->nb_sectors -= n;
- acb->sector_num += n;
- acb->buf += n * 512;
-
if (acb->nb_sectors == 0) {
/* request completed */
return 0;
@@ -681,6 +675,11 @@ static int qcow_aio_write_cb(QCowAIOCB *acb)
if (ret < 0) {
return ret;
}
+
+ acb->nb_sectors -= n;
+ acb->sector_num += n;
+ acb->buf += n * 512;
+
goto redo;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [RFC PATCH 4/5] avoid dandling pointers
2011-07-20 7:56 [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup Frediano Ziglio
` (2 preceding siblings ...)
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 3/5] more stack work Frediano Ziglio
@ 2011-07-20 7:57 ` Frediano Ziglio
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 5/5] qemu_aio_get used to clear all allocated buffer Frediano Ziglio
2011-07-20 13:03 ` [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup Kevin Wolf
5 siblings, 0 replies; 12+ messages in thread
From: Frediano Ziglio @ 2011-07-20 7:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Frediano Ziglio
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
block/qcow.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 8ccd7d7..007fb57 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -616,6 +616,7 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
if (acb.qiov->niov > 1) {
qemu_iovec_from_buffer(acb.qiov, acb.orig_buf, acb.qiov->size);
qemu_vfree(acb.orig_buf);
+ acb.orig_buf = NULL;
}
return ret;
@@ -700,6 +701,7 @@ static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
if (acb.qiov->niov > 1) {
qemu_vfree(acb.orig_buf);
+ acb.orig_buf = NULL;
}
return ret;
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [RFC PATCH 5/5] qemu_aio_get used to clear all allocated buffer
2011-07-20 7:56 [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup Frediano Ziglio
` (3 preceding siblings ...)
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 4/5] avoid dandling pointers Frediano Ziglio
@ 2011-07-20 7:57 ` Frediano Ziglio
2011-07-20 13:05 ` Kevin Wolf
2011-07-20 13:03 ` [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup Kevin Wolf
5 siblings, 1 reply; 12+ messages in thread
From: Frediano Ziglio @ 2011-07-20 7:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Frediano Ziglio
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
---
block/qcow.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 007fb57..8fd1ee5 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -499,7 +499,6 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
int is_write, QCowAIOCB *acb)
{
- memset(acb, 0, sizeof(*acb));
acb->bs = bs;
acb->sector_num = sector_num;
acb->qiov = qiov;
@@ -509,6 +508,7 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
if (is_write)
qemu_iovec_to_buffer(qiov, acb->buf);
} else {
+ acb->orig_buf = NULL;
acb->buf = (uint8_t *)qiov->iov->iov_base;
}
acb->nb_sectors = nb_sectors;
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [RFC PATCH 5/5] qemu_aio_get used to clear all allocated buffer
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 5/5] qemu_aio_get used to clear all allocated buffer Frediano Ziglio
@ 2011-07-20 13:05 ` Kevin Wolf
2011-07-20 13:45 ` Frediano Ziglio
0 siblings, 1 reply; 12+ messages in thread
From: Kevin Wolf @ 2011-07-20 13:05 UTC (permalink / raw)
To: Frediano Ziglio; +Cc: qemu-devel
Am 20.07.2011 09:57, schrieb Frediano Ziglio:
> Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
> ---
> block/qcow.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/block/qcow.c b/block/qcow.c
> index 007fb57..8fd1ee5 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -499,7 +499,6 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
> int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
> int is_write, QCowAIOCB *acb)
> {
> - memset(acb, 0, sizeof(*acb));
> acb->bs = bs;
> acb->sector_num = sector_num;
> acb->qiov = qiov;
> @@ -509,6 +508,7 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
> if (is_write)
> qemu_iovec_to_buffer(qiov, acb->buf);
> } else {
> + acb->orig_buf = NULL;
> acb->buf = (uint8_t *)qiov->iov->iov_base;
> }
> acb->nb_sectors = nb_sectors;
What does this fix? Removing the memset looks like changing code for no
obvious reason. Is there any state in acb that must survive qcow_aio_setup?
Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [RFC PATCH 5/5] qemu_aio_get used to clear all allocated buffer
2011-07-20 13:05 ` Kevin Wolf
@ 2011-07-20 13:45 ` Frediano Ziglio
0 siblings, 0 replies; 12+ messages in thread
From: Frediano Ziglio @ 2011-07-20 13:45 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
2011/7/20 Kevin Wolf <kwolf@redhat.com>:
> Am 20.07.2011 09:57, schrieb Frediano Ziglio:
>> Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
>> ---
>> block/qcow.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/block/qcow.c b/block/qcow.c
>> index 007fb57..8fd1ee5 100644
>> --- a/block/qcow.c
>> +++ b/block/qcow.c
>> @@ -499,7 +499,6 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
>> int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
>> int is_write, QCowAIOCB *acb)
>> {
>> - memset(acb, 0, sizeof(*acb));
>> acb->bs = bs;
>> acb->sector_num = sector_num;
>> acb->qiov = qiov;
>> @@ -509,6 +508,7 @@ static QCowAIOCB *qcow_aio_setup(BlockDriverState *bs,
>> if (is_write)
>> qemu_iovec_to_buffer(qiov, acb->buf);
>> } else {
>> + acb->orig_buf = NULL;
>> acb->buf = (uint8_t *)qiov->iov->iov_base;
>> }
>> acb->nb_sectors = nb_sectors;
>
> What does this fix? Removing the memset looks like changing code for no
> obvious reason. Is there any state in acb that must survive qcow_aio_setup?
>
> Kevin
>
No, this is not a fix, just an optimization, memset was used to clear
entire structure but all fields are set by qcow_aio_setup, only
orig_buf left.
The comment, I must admit is terrible! And it's mine! I'll rewrite all
comments, perhaps merge some commits and send all patches again.
Frediano
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup
2011-07-20 7:56 [Qemu-devel] [RFC PATCH 0/5] Coroutines cleanup Frediano Ziglio
` (4 preceding siblings ...)
2011-07-20 7:57 ` [Qemu-devel] [RFC PATCH 5/5] qemu_aio_get used to clear all allocated buffer Frediano Ziglio
@ 2011-07-20 13:03 ` Kevin Wolf
5 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2011-07-20 13:03 UTC (permalink / raw)
To: Frediano Ziglio; +Cc: qemu-devel
Am 20.07.2011 09:56, schrieb Frediano Ziglio:
> These patches mostly cleanup some AIO code using coroutines.
> These patches apply to Kevin's repository, branch coroutine-block.
> Mostly they use stack instead of allocated AIO structure.
I think your changes generally make sense as an example of how to
convert stuff to coroutines.
The thing that I'm not quite sure about is whether we should do it in
qcow1, which isn't frequently used and gets relatively little testing.
My main focus with qcow1 is keeping things working and maybe fixing
serious bugs. Code cleanups are always a (small, but not negligible)
risk of breaking things.
Anyway, I'm not totally opposed to merging this, but you really need to
improve the commit messages. First thing is that the subject should
start with the subsystem name ("qcow: Allocate AIOCB on stack"). I'm
also sure that you can be more precise than "more stack work". Please
use some more lines in the commit message to explain in more detail what
you're changing and ideally also why you're doing it.
Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread