Re: [Qemu-devel] [PATCH V3 3/6] ide: add support for IDEBufferedRequest

2015-11-12 Thread Peter Lieven

Am 12.11.2015 um 10:57 schrieb Fam Zheng:

On Fri, 11/06 09:42, Peter Lieven wrote:

+BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
+   QEMUIOVector *iov, int nb_sectors,
+   BlockCompletionFunc *cb, void *opaque)
+{
+BlockAIOCB *aioreq;
+IDEBufferedRequest *req;
+int c = 0;
+
+QLIST_FOREACH(req, &s->buffered_requests, list) {
+c++;
+}
+if (c > MAX_BUFFERED_REQS) {
+return blk_abort_aio_request(s->blk, cb, opaque, -EIO);
+}
+
+req = g_new0(IDEBufferedRequest, 1);
+req->original_qiov = iov;
+req->original_cb = cb;
+req->original_opaque = opaque;
+req->iov.iov_base = qemu_blockalign(blk_bs(s->blk), iov->size);

Where is this bounce buffer freed?


Oops, during conversion form req->buf to req->iov this got lost.
It should be freed in ide_buffered_readv_cb.

I will respin after you had a look at the other patches as well.

Thanks,
Peter




Re: [Qemu-devel] [PATCH V3 3/6] ide: add support for IDEBufferedRequest

2015-11-12 Thread Fam Zheng
On Fri, 11/06 09:42, Peter Lieven wrote:
> +BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
> +   QEMUIOVector *iov, int nb_sectors,
> +   BlockCompletionFunc *cb, void *opaque)
> +{
> +BlockAIOCB *aioreq;
> +IDEBufferedRequest *req;
> +int c = 0;
> +
> +QLIST_FOREACH(req, &s->buffered_requests, list) {
> +c++;
> +}
> +if (c > MAX_BUFFERED_REQS) {
> +return blk_abort_aio_request(s->blk, cb, opaque, -EIO);
> +}
> +
> +req = g_new0(IDEBufferedRequest, 1);
> +req->original_qiov = iov;
> +req->original_cb = cb;
> +req->original_opaque = opaque;
> +req->iov.iov_base = qemu_blockalign(blk_bs(s->blk), iov->size);

Where is this bounce buffer freed?

> +req->iov.iov_len = iov->size;
> +qemu_iovec_init_external(&req->qiov, &req->iov, 1);
> +
> +aioreq = blk_aio_readv(s->blk, sector_num, &req->qiov, nb_sectors,
> +   ide_buffered_readv_cb, req);
> +
> +QLIST_INSERT_HEAD(&s->buffered_requests, req, list);
> +return aioreq;
> +}
> +
>  static void ide_sector_read(IDEState *s);
>  
>  static void ide_sector_read_cb(void *opaque, int ret)



[Qemu-devel] [PATCH V3 3/6] ide: add support for IDEBufferedRequest

2015-11-06 Thread Peter Lieven
this patch adds a new aio readv compatible function which copies
all data through a bounce buffer. These buffered requests can be
flagged as orphaned which means that their original callback has
already been invoked and the request has just not been completed
by the backend storage. The bounce buffer guarantees that guest
memory corruption is avoided when such a orphaned request is
completed by the backend at a later stage.

This trick only works for read requests as a write request completed
at a later stage might corrupt data as there is no way to control
if and what data has already been written to the storage.

Signed-off-by: Peter Lieven 
---
 hw/ide/core.c | 46 ++
 hw/ide/internal.h | 14 ++
 2 files changed, 60 insertions(+)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 364ba21..53f9c2c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -561,6 +561,52 @@ static bool ide_sect_range_ok(IDEState *s,
 return true;
 }
 
+static void ide_buffered_readv_cb(void *opaque, int ret)
+{
+IDEBufferedRequest *req = opaque;
+if (!req->orphaned) {
+if (!ret) {
+qemu_iovec_from_buf(req->original_qiov, 0, req->iov.iov_base,
+req->original_qiov->size);
+}
+req->original_cb(req->original_opaque, ret);
+}
+QLIST_REMOVE(req, list);
+g_free(req);
+}
+
+#define MAX_BUFFERED_REQS 16
+
+BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
+   QEMUIOVector *iov, int nb_sectors,
+   BlockCompletionFunc *cb, void *opaque)
+{
+BlockAIOCB *aioreq;
+IDEBufferedRequest *req;
+int c = 0;
+
+QLIST_FOREACH(req, &s->buffered_requests, list) {
+c++;
+}
+if (c > MAX_BUFFERED_REQS) {
+return blk_abort_aio_request(s->blk, cb, opaque, -EIO);
+}
+
+req = g_new0(IDEBufferedRequest, 1);
+req->original_qiov = iov;
+req->original_cb = cb;
+req->original_opaque = opaque;
+req->iov.iov_base = qemu_blockalign(blk_bs(s->blk), iov->size);
+req->iov.iov_len = iov->size;
+qemu_iovec_init_external(&req->qiov, &req->iov, 1);
+
+aioreq = blk_aio_readv(s->blk, sector_num, &req->qiov, nb_sectors,
+   ide_buffered_readv_cb, req);
+
+QLIST_INSERT_HEAD(&s->buffered_requests, req, list);
+return aioreq;
+}
+
 static void ide_sector_read(IDEState *s);
 
 static void ide_sector_read_cb(void *opaque, int ret)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 05e93ff..3d1116f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -343,6 +343,16 @@ enum ide_dma_cmd {
 #define ide_cmd_is_read(s) \
((s)->dma_cmd == IDE_DMA_READ)
 
+typedef struct IDEBufferedRequest {
+QLIST_ENTRY(IDEBufferedRequest) list;
+struct iovec iov;
+QEMUIOVector qiov;
+QEMUIOVector *original_qiov;
+BlockCompletionFunc *original_cb;
+void *original_opaque;
+bool orphaned;
+} IDEBufferedRequest;
+
 /* NOTE: IDEState represents in fact one drive */
 struct IDEState {
 IDEBus *bus;
@@ -396,6 +406,7 @@ struct IDEState {
 BlockAIOCB *pio_aiocb;
 struct iovec iov;
 QEMUIOVector qiov;
+QLIST_HEAD(, IDEBufferedRequest) buffered_requests;
 /* ATA DMA state */
 int32_t io_buffer_offset;
 int32_t io_buffer_size;
@@ -572,6 +583,9 @@ void ide_set_inactive(IDEState *s, bool more);
 BlockAIOCB *ide_issue_trim(BlockBackend *blk,
 int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
 BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
+   QEMUIOVector *iov, int nb_sectors,
+   BlockCompletionFunc *cb, void *opaque);
 
 /* hw/ide/atapi.c */
 void ide_atapi_cmd(IDEState *s);
-- 
1.9.1