* [Qemu-devel] [PATCH v3 01/15] block: Rename blk_write_zeroes()
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 14:40 ` Max Reitz
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 02/15] block: keep a list of block jobs Kevin Wolf
` (13 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
From: Eric Blake <eblake@redhat.com>
Commit 983a1600 changed the semantics of blk_write_zeroes() to
be byte-based rather than sector-based, but did not change the
name, which is an open invitation for other code to misuse the
function. Renaming to pwrite_zeroes() makes it more in line
with other byte-based interfaces, and will help make it easier
to track which remaining write_zeroes interfaces still need
conversion.
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/block-backend.c | 14 +++++++-------
block/parallels.c | 4 ++--
hw/scsi/scsi-disk.c | 2 +-
include/sysemu/block-backend.h | 14 +++++++-------
qemu-img.c | 4 ++--
qemu-io-cmds.c | 22 +++++++++++-----------
6 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 14e528e..f9da1d1 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -855,8 +855,8 @@ int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
return ret;
}
-int blk_write_zeroes(BlockBackend *blk, int64_t offset,
- int count, BdrvRequestFlags flags)
+int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags)
{
return blk_prw(blk, offset, NULL, count, blk_write_entry,
flags | BDRV_REQ_ZERO_WRITE);
@@ -971,9 +971,9 @@ static void blk_aio_write_entry(void *opaque)
blk_aio_complete(acb);
}
-BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t offset,
- int count, BdrvRequestFlags flags,
- BlockCompletionFunc *cb, void *opaque)
+BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags,
+ BlockCompletionFunc *cb, void *opaque)
{
return blk_aio_prwv(blk, offset, count, NULL, blk_aio_write_entry,
flags | BDRV_REQ_ZERO_WRITE, cb, opaque);
@@ -1462,8 +1462,8 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
return qemu_aio_get(aiocb_info, blk_bs(blk), cb, opaque);
}
-int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t offset,
- int count, BdrvRequestFlags flags)
+int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags)
{
return blk_co_pwritev(blk, offset, count, NULL,
flags | BDRV_REQ_ZERO_WRITE);
diff --git a/block/parallels.c b/block/parallels.c
index 88cface..99fc0f7 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -517,8 +517,8 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
if (ret < 0) {
goto exit;
}
- ret = blk_write_zeroes(file, BDRV_SECTOR_SIZE,
- (bat_sectors - 1) << BDRV_SECTOR_BITS, 0);
+ ret = blk_pwrite_zeroes(file, BDRV_SECTOR_SIZE,
+ (bat_sectors - 1) << BDRV_SECTOR_BITS, 0);
if (ret < 0) {
goto exit;
}
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index ce89c98..a3ecad4 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1778,7 +1778,7 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r, uint8_t *inbuf)
block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
nb_sectors * s->qdev.blocksize,
BLOCK_ACCT_WRITE);
- r->req.aiocb = blk_aio_write_zeroes(s->qdev.conf.blk,
+ r->req.aiocb = blk_aio_pwrite_zeroes(s->qdev.conf.blk,
r->req.cmd.lba * s->qdev.blocksize,
nb_sectors * s->qdev.blocksize,
flags, scsi_aio_complete, r);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 9d6615c..9571726 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -113,11 +113,11 @@ void *blk_get_attached_dev(BlockBackend *blk);
void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
int count);
-int blk_write_zeroes(BlockBackend *blk, int64_t offset,
- int count, BdrvRequestFlags flags);
-BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t offset,
- int count, BdrvRequestFlags flags,
- BlockCompletionFunc *cb, void *opaque);
+int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags);
+BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags,
+ BlockCompletionFunc *cb, void *opaque);
int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count);
int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int count,
BdrvRequestFlags flags);
@@ -195,8 +195,8 @@ int blk_get_open_flags_from_root_state(BlockBackend *blk);
void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
BlockCompletionFunc *cb, void *opaque);
-int coroutine_fn blk_co_write_zeroes(BlockBackend *blk, int64_t offset,
- int count, BdrvRequestFlags flags);
+int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+ int count, BdrvRequestFlags flags);
int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
int blk_truncate(BlockBackend *blk, int64_t offset);
diff --git a/qemu-img.c b/qemu-img.c
index 7ed8ef2..2065348 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1606,8 +1606,8 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors,
if (s->has_zero_init) {
break;
}
- ret = blk_write_zeroes(s->target, sector_num << BDRV_SECTOR_BITS,
- n << BDRV_SECTOR_BITS, 0);
+ ret = blk_pwrite_zeroes(s->target, sector_num << BDRV_SECTOR_BITS,
+ n << BDRV_SECTOR_BITS, 0);
if (ret < 0) {
return ret;
}
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index e766791..09e879f 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -451,12 +451,12 @@ typedef struct {
bool done;
} CoWriteZeroes;
-static void coroutine_fn co_write_zeroes_entry(void *opaque)
+static void coroutine_fn co_pwrite_zeroes_entry(void *opaque)
{
CoWriteZeroes *data = opaque;
- data->ret = blk_co_write_zeroes(data->blk, data->offset, data->count,
- data->flags);
+ data->ret = blk_co_pwrite_zeroes(data->blk, data->offset, data->count,
+ data->flags);
data->done = true;
if (data->ret < 0) {
*data->total = data->ret;
@@ -466,8 +466,8 @@ static void coroutine_fn co_write_zeroes_entry(void *opaque)
*data->total = data->count;
}
-static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int64_t count,
- int flags, int64_t *total)
+static int do_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+ int64_t count, int flags, int64_t *total)
{
Coroutine *co;
CoWriteZeroes data = {
@@ -483,7 +483,7 @@ static int do_co_write_zeroes(BlockBackend *blk, int64_t offset, int64_t count,
return -ERANGE;
}
- co = qemu_coroutine_create(co_write_zeroes_entry);
+ co = qemu_coroutine_create(co_pwrite_zeroes_entry);
qemu_coroutine_enter(co, &data);
while (!data.done) {
aio_poll(blk_get_aio_context(blk), true);
@@ -901,7 +901,7 @@ static void write_help(void)
" -C, -- report statistics in a machine parsable format\n"
" -q, -- quiet mode, do not show I/O statistics\n"
" -u, -- with -z, allow unmapping\n"
-" -z, -- write zeroes using blk_co_write_zeroes\n"
+" -z, -- write zeroes using blk_co_pwrite_zeroes\n"
"\n");
}
@@ -1033,7 +1033,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
if (bflag) {
cnt = do_save_vmstate(blk, buf, offset, count, &total);
} else if (zflag) {
- cnt = do_co_write_zeroes(blk, offset, count, flags, &total);
+ cnt = do_co_pwrite_zeroes(blk, offset, count, flags, &total);
} else if (cflag) {
cnt = do_write_compressed(blk, buf, offset, count, &total);
} else {
@@ -1376,7 +1376,7 @@ static void aio_write_help(void)
" -i, -- treat request as invalid, for exercising stats\n"
" -q, -- quiet mode, do not show I/O statistics\n"
" -u, -- with -z, allow unmapping\n"
-" -z, -- write zeroes using blk_aio_write_zeroes\n"
+" -z, -- write zeroes using blk_aio_pwrite_zeroes\n"
"\n");
}
@@ -1475,8 +1475,8 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
}
ctx->qiov.size = count;
- blk_aio_write_zeroes(blk, ctx->offset, count, flags, aio_write_done,
- ctx);
+ blk_aio_pwrite_zeroes(blk, ctx->offset, count, flags, aio_write_done,
+ ctx);
} else {
nr_iov = argc - optind;
ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 01/15] block: Rename blk_write_zeroes()
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 01/15] block: Rename blk_write_zeroes() Kevin Wolf
@ 2016-05-25 14:40 ` Max Reitz
0 siblings, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 14:40 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> From: Eric Blake <eblake@redhat.com>
>
> Commit 983a1600 changed the semantics of blk_write_zeroes() to
> be byte-based rather than sector-based, but did not change the
> name, which is an open invitation for other code to misuse the
> function. Renaming to pwrite_zeroes() makes it more in line
> with other byte-based interfaces, and will help make it easier
> to track which remaining write_zeroes interfaces still need
> conversion.
>
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/block-backend.c | 14 +++++++-------
> block/parallels.c | 4 ++--
> hw/scsi/scsi-disk.c | 2 +-
> include/sysemu/block-backend.h | 14 +++++++-------
> qemu-img.c | 4 ++--
> qemu-io-cmds.c | 22 +++++++++++-----------
> 6 files changed, 30 insertions(+), 30 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3 02/15] block: keep a list of block jobs
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 01/15] block: Rename blk_write_zeroes() Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 03/15] block: Cancel jobs first in bdrv_close_all() Kevin Wolf
` (12 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
From: Alberto Garcia <berto@igalia.com>
The current way to obtain the list of existing block jobs is to
iterate over all root nodes and check which ones own a job.
Since we want to be able to support block jobs in other nodes as well,
this patch keeps a list of jobs that is updated every time one is
created or destroyed.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockjob.c | 13 +++++++++++++
include/block/blockjob.h | 14 ++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/blockjob.c b/blockjob.c
index 5b840a7..0f1bc77 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -50,6 +50,16 @@ struct BlockJobTxn {
int refcnt;
};
+static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs);
+
+BlockJob *block_job_next(BlockJob *job)
+{
+ if (!job) {
+ return QLIST_FIRST(&block_jobs);
+ }
+ return QLIST_NEXT(job, job_list);
+}
+
void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
int64_t speed, BlockCompletionFunc *cb,
void *opaque, Error **errp)
@@ -76,6 +86,8 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
job->refcnt = 1;
bs->job = job;
+ QLIST_INSERT_HEAD(&block_jobs, job, job_list);
+
/* Only set speed when necessary to avoid NotSupported error */
if (speed != 0) {
Error *local_err = NULL;
@@ -103,6 +115,7 @@ void block_job_unref(BlockJob *job)
bdrv_unref(job->bs);
error_free(job->blocker);
g_free(job->id);
+ QLIST_REMOVE(job, job_list);
g_free(job);
}
}
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 073a433..30bb2c6 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -135,6 +135,9 @@ struct BlockJob {
*/
bool deferred_to_main_loop;
+ /** Element of the list of block jobs */
+ QLIST_ENTRY(BlockJob) job_list;
+
/** Status that is published by the query-block-jobs QMP API */
BlockDeviceIoStatus iostatus;
@@ -173,6 +176,17 @@ struct BlockJob {
};
/**
+ * block_job_next:
+ * @job: A block job, or %NULL.
+ *
+ * Get the next element from the list of block jobs after @job, or the
+ * first one if @job is %NULL.
+ *
+ * Returns the requested job, or %NULL if there are no more jobs left.
+ */
+BlockJob *block_job_next(BlockJob *job);
+
+/**
* block_job_create:
* @job_type: The class object for the newly-created job.
* @bs: The block
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH v3 03/15] block: Cancel jobs first in bdrv_close_all()
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 01/15] block: Rename blk_write_zeroes() Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 02/15] block: keep a list of block jobs Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 14:44 ` Max Reitz
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 04/15] block: Default to enabled write cache in blk_new() Kevin Wolf
` (11 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
So far, bdrv_close_all() first removed all root BlockDriverStates of
BlockBackends and monitor owned BDSes, and then assumed that the
remaining BDSes must be related to jobs and cancelled these jobs.
This order doesn't work that well any more when block jobs use
BlockBackends internally because then they will lose their BDS before
being cancelled.
This patch changes bdrv_close_all() to first cancel all jobs and then
remove all root BDSes from the remaining BBs.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block.c | 23 ++---------------------
blockjob.c | 13 +++++++++++++
include/block/blockjob.h | 7 +++++++
3 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/block.c b/block.c
index 45cddd6..736432f 100644
--- a/block.c
+++ b/block.c
@@ -2209,8 +2209,7 @@ static void bdrv_close(BlockDriverState *bs)
void bdrv_close_all(void)
{
- BlockDriverState *bs;
- AioContext *aio_context;
+ block_job_cancel_sync_all();
/* Drop references from requests still in flight, such as canceled block
* jobs whose AIO context has not been polled yet */
@@ -2219,25 +2218,7 @@ void bdrv_close_all(void)
blk_remove_all_bs();
blockdev_close_all_bdrv_states();
- /* Cancel all block jobs */
- while (!QTAILQ_EMPTY(&all_bdrv_states)) {
- QTAILQ_FOREACH(bs, &all_bdrv_states, bs_list) {
- aio_context = bdrv_get_aio_context(bs);
-
- aio_context_acquire(aio_context);
- if (bs->job) {
- block_job_cancel_sync(bs->job);
- aio_context_release(aio_context);
- break;
- }
- aio_context_release(aio_context);
- }
-
- /* All the remaining BlockDriverStates are referenced directly or
- * indirectly from block jobs, so there needs to be at least one BDS
- * directly used by a block job */
- assert(bs);
- }
+ assert(QTAILQ_EMPTY(&all_bdrv_states));
}
static void change_parent_backing_link(BlockDriverState *from,
diff --git a/blockjob.c b/blockjob.c
index 0f1bc77..e916b41 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -331,6 +331,19 @@ int block_job_cancel_sync(BlockJob *job)
return block_job_finish_sync(job, &block_job_cancel_err, NULL);
}
+void block_job_cancel_sync_all(void)
+{
+ BlockJob *job;
+ AioContext *aio_context;
+
+ while ((job = QLIST_FIRST(&block_jobs))) {
+ aio_context = bdrv_get_aio_context(job->bs);
+ aio_context_acquire(aio_context);
+ block_job_cancel_sync(job);
+ aio_context_release(aio_context);
+ }
+}
+
int block_job_complete_sync(BlockJob *job, Error **errp)
{
return block_job_finish_sync(job, &block_job_complete, errp);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 30bb2c6..4ac6831 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -371,6 +371,13 @@ bool block_job_is_paused(BlockJob *job);
int block_job_cancel_sync(BlockJob *job);
/**
+ * block_job_cancel_sync_all:
+ *
+ * Synchronously cancels all jobs using block_job_cancel_sync().
+ */
+void block_job_cancel_sync_all(void);
+
+/**
* block_job_complete_sync:
* @job: The job to be completed.
* @errp: Error object which may be set by block_job_complete(); this is not
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 03/15] block: Cancel jobs first in bdrv_close_all()
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 03/15] block: Cancel jobs first in bdrv_close_all() Kevin Wolf
@ 2016-05-25 14:44 ` Max Reitz
0 siblings, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 14:44 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 945 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> So far, bdrv_close_all() first removed all root BlockDriverStates of
> BlockBackends and monitor owned BDSes, and then assumed that the
> remaining BDSes must be related to jobs and cancelled these jobs.
>
> This order doesn't work that well any more when block jobs use
> BlockBackends internally because then they will lose their BDS before
> being cancelled.
>
> This patch changes bdrv_close_all() to first cancel all jobs and then
> remove all root BDSes from the remaining BBs.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
> block.c | 23 ++---------------------
> blockjob.c | 13 +++++++++++++
> include/block/blockjob.h | 7 +++++++
> 3 files changed, 22 insertions(+), 21 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3 04/15] block: Default to enabled write cache in blk_new()
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (2 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 03/15] block: Cancel jobs first in bdrv_close_all() Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 05/15] block: Convert block job core to BlockBackend Kevin Wolf
` (10 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
The existing users of the function are:
1. blk_new_open(), which already enabled the write cache
2. Some test cases that don't care about the setting
3. blockdev_init() for empty drives, where the cache mode is overridden
with the value from the options when a medium is inserted
Therefore, this patch doesn't change the current behaviour. It will be
convenient, however, for additional users of blk_new() (like block
jobs) if the most sensible WCE setting is the default.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/block-backend.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index f9da1d1..c8b13f1 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -125,6 +125,8 @@ BlockBackend *blk_new(void)
blk = g_new0(BlockBackend, 1);
blk->refcnt = 1;
+ blk_set_enable_write_cache(blk, true);
+
qemu_co_queue_init(&blk->public.throttled_reqs[0]);
qemu_co_queue_init(&blk->public.throttled_reqs[1]);
@@ -160,7 +162,6 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
return NULL;
}
- blk_set_enable_write_cache(blk, true);
blk->root = bdrv_root_attach_child(bs, "root", &child_root, blk);
return blk;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH v3 05/15] block: Convert block job core to BlockBackend
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (3 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 04/15] block: Default to enabled write cache in blk_new() Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 14:51 ` Max Reitz
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public Kevin Wolf
` (9 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
This adds a new BlockBackend field to the BlockJob struct, which
coexists with the BlockDriverState while converting the individual jobs.
When creating a block job, a new BlockBackend is created on top of the
given BlockDriverState, and it is destroyed when the BlockJob ends. The
reference to the BDS is now held by the BlockBackend instead of calling
bdrv_ref/unref manually.
We have to be careful when we use bdrv_replace_in_backing_chain() in
block jobs because this changes the BDS that job->blk points to. At the
moment block jobs are too tightly coupled with their BDS, so that moving
a job to another BDS isn't easily possible; therefore, we need to just
manually undo this change afterwards.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/mirror.c | 3 +++
blockjob.c | 37 ++++++++++++++++++++-----------------
include/block/blockjob.h | 3 ++-
3 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index b9986d8..efca8fc 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -478,6 +478,9 @@ static void mirror_exit(BlockJob *job, void *opaque)
bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
}
bdrv_replace_in_backing_chain(to_replace, s->target);
+ /* We just changed the BDS the job BB refers to */
+ blk_remove_bs(job->blk);
+ blk_insert_bs(job->blk, src);
}
out:
diff --git a/blockjob.c b/blockjob.c
index e916b41..2097e1d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -64,13 +64,17 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
int64_t speed, BlockCompletionFunc *cb,
void *opaque, Error **errp)
{
+ BlockBackend *blk;
BlockJob *job;
if (bs->job) {
error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
return NULL;
}
- bdrv_ref(bs);
+
+ blk = blk_new();
+ blk_insert_bs(blk, bs);
+
job = g_malloc0(driver->instance_size);
error_setg(&job->blocker, "block device is in use by block job: %s",
BlockJobType_lookup[driver->job_type]);
@@ -80,6 +84,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
job->driver = driver;
job->id = g_strdup(bdrv_get_device_name(bs));
job->bs = bs;
+ job->blk = blk;
job->cb = cb;
job->opaque = opaque;
job->busy = true;
@@ -110,9 +115,10 @@ void block_job_ref(BlockJob *job)
void block_job_unref(BlockJob *job)
{
if (--job->refcnt == 0) {
- job->bs->job = NULL;
- bdrv_op_unblock_all(job->bs, job->blocker);
- bdrv_unref(job->bs);
+ BlockDriverState *bs = blk_bs(job->blk);
+ bs->job = NULL;
+ bdrv_op_unblock_all(bs, job->blocker);
+ blk_unref(job->blk);
error_free(job->blocker);
g_free(job->id);
QLIST_REMOVE(job, job_list);
@@ -153,7 +159,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
txn->aborting = true;
/* We are the first failed job. Cancel other jobs. */
QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
- ctx = bdrv_get_aio_context(other_job->bs);
+ ctx = blk_get_aio_context(other_job->blk);
aio_context_acquire(ctx);
}
QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
@@ -170,7 +176,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
assert(other_job->completed);
}
QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
- ctx = bdrv_get_aio_context(other_job->bs);
+ ctx = blk_get_aio_context(other_job->blk);
block_job_completed_single(other_job);
aio_context_release(ctx);
}
@@ -192,7 +198,7 @@ static void block_job_completed_txn_success(BlockJob *job)
}
/* We are the last completed job, commit the transaction. */
QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
- ctx = bdrv_get_aio_context(other_job->bs);
+ ctx = blk_get_aio_context(other_job->blk);
aio_context_acquire(ctx);
assert(other_job->ret == 0);
block_job_completed_single(other_job);
@@ -202,9 +208,7 @@ static void block_job_completed_txn_success(BlockJob *job)
void block_job_completed(BlockJob *job, int ret)
{
- BlockDriverState *bs = job->bs;
-
- assert(bs->job == job);
+ assert(blk_bs(job->blk)->job == job);
assert(!job->completed);
job->completed = true;
job->ret = ret;
@@ -295,11 +299,10 @@ static int block_job_finish_sync(BlockJob *job,
void (*finish)(BlockJob *, Error **errp),
Error **errp)
{
- BlockDriverState *bs = job->bs;
Error *local_err = NULL;
int ret;
- assert(bs->job == job);
+ assert(blk_bs(job->blk)->job == job);
block_job_ref(job);
finish(job, &local_err);
@@ -310,7 +313,7 @@ static int block_job_finish_sync(BlockJob *job,
}
while (!job->completed) {
aio_poll(job->deferred_to_main_loop ? qemu_get_aio_context() :
- bdrv_get_aio_context(bs),
+ blk_get_aio_context(job->blk),
true);
}
ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
@@ -337,7 +340,7 @@ void block_job_cancel_sync_all(void)
AioContext *aio_context;
while ((job = QLIST_FIRST(&block_jobs))) {
- aio_context = bdrv_get_aio_context(job->bs);
+ aio_context = blk_get_aio_context(job->blk);
aio_context_acquire(aio_context);
block_job_cancel_sync(job);
aio_context_release(aio_context);
@@ -362,7 +365,7 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
if (block_job_is_paused(job)) {
qemu_coroutine_yield();
} else {
- co_aio_sleep_ns(bdrv_get_aio_context(job->bs), type, ns);
+ co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
}
job->busy = true;
}
@@ -491,7 +494,7 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
aio_context_acquire(data->aio_context);
/* Fetch BDS AioContext again, in case it has changed */
- aio_context = bdrv_get_aio_context(data->job->bs);
+ aio_context = blk_get_aio_context(data->job->blk);
aio_context_acquire(aio_context);
data->job->deferred_to_main_loop = false;
@@ -511,7 +514,7 @@ void block_job_defer_to_main_loop(BlockJob *job,
BlockJobDeferToMainLoopData *data = g_malloc(sizeof(*data));
data->job = job;
data->bh = qemu_bh_new(block_job_defer_to_main_loop_bh, data);
- data->aio_context = bdrv_get_aio_context(job->bs);
+ data->aio_context = blk_get_aio_context(job->blk);
data->fn = fn;
data->opaque = opaque;
job->deferred_to_main_loop = true;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 4ac6831..32012af 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -82,7 +82,8 @@ struct BlockJob {
const BlockJobDriver *driver;
/** The block device on which the job is operating. */
- BlockDriverState *bs;
+ BlockDriverState *bs; /* TODO Remove */
+ BlockBackend *blk;
/**
* The ID of the block job. Currently the BlockBackend name of the BDS
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 05/15] block: Convert block job core to BlockBackend
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 05/15] block: Convert block job core to BlockBackend Kevin Wolf
@ 2016-05-25 14:51 ` Max Reitz
0 siblings, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 14:51 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> This adds a new BlockBackend field to the BlockJob struct, which
> coexists with the BlockDriverState while converting the individual jobs.
>
> When creating a block job, a new BlockBackend is created on top of the
> given BlockDriverState, and it is destroyed when the BlockJob ends. The
> reference to the BDS is now held by the BlockBackend instead of calling
> bdrv_ref/unref manually.
>
> We have to be careful when we use bdrv_replace_in_backing_chain() in
> block jobs because this changes the BDS that job->blk points to. At the
> moment block jobs are too tightly coupled with their BDS, so that moving
> a job to another BDS isn't easily possible; therefore, we need to just
> manually undo this change afterwards.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
> block/mirror.c | 3 +++
> blockjob.c | 37 ++++++++++++++++++++-----------------
> include/block/blockjob.h | 3 ++-
> 3 files changed, 25 insertions(+), 18 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (4 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 05/15] block: Convert block job core to BlockBackend Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 13:41 ` Alberto Garcia
` (2 more replies)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 07/15] stream: Use BlockBackend for I/O Kevin Wolf
` (8 subsequent siblings)
14 siblings, 3 replies; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
Also add trace points now that the function can be directly called.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/block-backend.c | 21 ++++++++++++++-------
include/sysemu/block-backend.h | 6 ++++++
trace-events | 4 ++++
3 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index c8b13f1..34500e6 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -19,6 +19,7 @@
#include "sysemu/sysemu.h"
#include "qapi-event.h"
#include "qemu/id.h"
+#include "trace.h"
/* Number of coroutines to reserve per attached device model */
#define COROUTINE_POOL_RESERVATION 64
@@ -741,11 +742,15 @@ static int blk_check_request(BlockBackend *blk, int64_t sector_num,
nb_sectors * BDRV_SECTOR_SIZE);
}
-static int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
- unsigned int bytes, QEMUIOVector *qiov,
- BdrvRequestFlags flags)
+int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
+ unsigned int bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
- int ret = blk_check_byte_request(blk, offset, bytes);
+ int ret;
+
+ trace_blk_co_preadv(blk, blk_bs(blk), offset, bytes, flags);
+
+ ret = blk_check_byte_request(blk, offset, bytes);
if (ret < 0) {
return ret;
}
@@ -758,12 +763,14 @@ static int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
return bdrv_co_preadv(blk_bs(blk), offset, bytes, qiov, flags);
}
-static int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
- unsigned int bytes, QEMUIOVector *qiov,
- BdrvRequestFlags flags)
+int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
+ unsigned int bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags)
{
int ret;
+ trace_blk_co_pwritev(blk, blk_bs(blk), offset, bytes, flags);
+
ret = blk_check_byte_request(blk, offset, bytes);
if (ret < 0) {
return ret;
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 9571726..c04af8e 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -113,6 +113,12 @@ void *blk_get_attached_dev(BlockBackend *blk);
void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
int count);
+int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
+ unsigned int bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags);
+int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
+ unsigned int bytes, QEMUIOVector *qiov,
+ BdrvRequestFlags flags);
int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
int count, BdrvRequestFlags flags);
BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset,
diff --git a/trace-events b/trace-events
index b53c354..5d431bd 100644
--- a/trace-events
+++ b/trace-events
@@ -61,6 +61,10 @@ virtio_console_chr_event(unsigned int port, int event) "port %u, event %d"
bdrv_open_common(void *bs, const char *filename, int flags, const char *format_name) "bs %p filename \"%s\" flags %#x format_name \"%s\""
bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
+# block/block-backend.c
+blk_co_preadv(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags %x"
+blk_co_pwritev(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags %x"
+
# block/io.c
bdrv_aio_discard(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public Kevin Wolf
@ 2016-05-25 13:41 ` Alberto Garcia
2016-05-25 14:08 ` Eric Blake
2016-05-25 14:54 ` Max Reitz
2 siblings, 0 replies; 28+ messages in thread
From: Alberto Garcia @ 2016-05-25 13:41 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, jcody, jsnow, qemu-devel, eblake
On Wed 25 May 2016 02:29:14 PM CEST, Kevin Wolf wrote:
> Also add trace points now that the function can be directly called.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public Kevin Wolf
2016-05-25 13:41 ` Alberto Garcia
@ 2016-05-25 14:08 ` Eric Blake
2016-05-25 14:54 ` Max Reitz
2 siblings, 0 replies; 28+ messages in thread
From: Eric Blake @ 2016-05-25 14:08 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, berto, jcody, jsnow, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 543 bytes --]
On 05/25/2016 06:29 AM, Kevin Wolf wrote:
> Also add trace points now that the function can be directly called.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/block-backend.c | 21 ++++++++++++++-------
> include/sysemu/block-backend.h | 6 ++++++
> trace-events | 4 ++++
> 3 files changed, 24 insertions(+), 7 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public Kevin Wolf
2016-05-25 13:41 ` Alberto Garcia
2016-05-25 14:08 ` Eric Blake
@ 2016-05-25 14:54 ` Max Reitz
2 siblings, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 14:54 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 715 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> Also add trace points now that the function can be directly called.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/block-backend.c | 21 ++++++++++++++-------
> include/sysemu/block-backend.h | 6 ++++++
> trace-events | 4 ++++
> 3 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index c8b13f1..34500e6 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -19,6 +19,7 @@
> #include "sysemu/sysemu.h"
> #include "qapi-event.h"
> #include "qemu/id.h"
> +#include "trace.h"
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3 07/15] stream: Use BlockBackend for I/O
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (5 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 06/15] block: Make blk_co_preadv/pwritev() public Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 14:56 ` Max Reitz
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 08/15] mirror: Allow target that already has a BlockBackend Kevin Wolf
` (7 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
This changes the streaming block job to use the job's BlockBackend for
performing the COR reads. job->bs isn't used by the streaming code any
more afterwards.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/io.c | 9 ---------
block/stream.c | 15 +++++++++------
include/block/block.h | 2 --
trace-events | 1 -
4 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/block/io.c b/block/io.c
index 9bc1d45..bc2eda2 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1117,15 +1117,6 @@ int coroutine_fn bdrv_co_readv_no_serialising(BlockDriverState *bs,
BDRV_REQ_NO_SERIALISING);
}
-int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
-{
- trace_bdrv_co_copy_on_readv(bs, sector_num, nb_sectors);
-
- return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov,
- BDRV_REQ_COPY_ON_READ);
-}
-
#define MAX_WRITE_ZEROES_BOUNCE_BUFFER 32768
static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
diff --git a/block/stream.c b/block/stream.c
index 40aa322..c0efbda 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -39,7 +39,7 @@ typedef struct StreamBlockJob {
char *backing_file_str;
} StreamBlockJob;
-static int coroutine_fn stream_populate(BlockDriverState *bs,
+static int coroutine_fn stream_populate(BlockBackend *blk,
int64_t sector_num, int nb_sectors,
void *buf)
{
@@ -52,7 +52,8 @@ static int coroutine_fn stream_populate(BlockDriverState *bs,
qemu_iovec_init_external(&qiov, &iov, 1);
/* Copy-on-read the unallocated clusters */
- return bdrv_co_copy_on_readv(bs, sector_num, nb_sectors, &qiov);
+ return blk_co_preadv(blk, sector_num * BDRV_SECTOR_SIZE, qiov.size, &qiov,
+ BDRV_REQ_COPY_ON_READ);
}
typedef struct {
@@ -64,6 +65,7 @@ static void stream_complete(BlockJob *job, void *opaque)
{
StreamBlockJob *s = container_of(job, StreamBlockJob, common);
StreamCompleteData *data = opaque;
+ BlockDriverState *bs = blk_bs(job->blk);
BlockDriverState *base = s->base;
if (!block_job_is_cancelled(&s->common) && data->reached_end &&
@@ -75,8 +77,8 @@ static void stream_complete(BlockJob *job, void *opaque)
base_fmt = base->drv->format_name;
}
}
- data->ret = bdrv_change_backing_file(job->bs, base_id, base_fmt);
- bdrv_set_backing_hd(job->bs, base);
+ data->ret = bdrv_change_backing_file(bs, base_id, base_fmt);
+ bdrv_set_backing_hd(bs, base);
}
g_free(s->backing_file_str);
@@ -88,7 +90,8 @@ static void coroutine_fn stream_run(void *opaque)
{
StreamBlockJob *s = opaque;
StreamCompleteData *data;
- BlockDriverState *bs = s->common.bs;
+ BlockBackend *blk = s->common.blk;
+ BlockDriverState *bs = blk_bs(blk);
BlockDriverState *base = s->base;
int64_t sector_num = 0;
int64_t end = -1;
@@ -159,7 +162,7 @@ wait:
goto wait;
}
}
- ret = stream_populate(bs, sector_num, n, buf);
+ ret = stream_populate(blk, sector_num, n, buf);
}
if (ret < 0) {
BlockErrorAction action =
diff --git a/include/block/block.h b/include/block/block.h
index e899f7d..0a4b973 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -242,8 +242,6 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
const void *buf, int count);
int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov);
-int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
int coroutine_fn bdrv_co_readv_no_serialising(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
diff --git a/trace-events b/trace-events
index 5d431bd..1bd595b 100644
--- a/trace-events
+++ b/trace-events
@@ -72,7 +72,6 @@ bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %
bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
bdrv_aio_write_zeroes(void *bs, int64_t sector_num, int nb_sectors, int flags, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x opaque %p"
bdrv_co_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
-bdrv_co_copy_on_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
bdrv_co_readv_no_serialising(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
bdrv_co_write_zeroes(void *bs, int64_t sector_num, int nb_sector, int flags) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 07/15] stream: Use BlockBackend for I/O
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 07/15] stream: Use BlockBackend for I/O Kevin Wolf
@ 2016-05-25 14:56 ` Max Reitz
0 siblings, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 14:56 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 621 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> This changes the streaming block job to use the job's BlockBackend for
> performing the COR reads. job->bs isn't used by the streaming code any
> more afterwards.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
> block/io.c | 9 ---------
> block/stream.c | 15 +++++++++------
> include/block/block.h | 2 --
> trace-events | 1 -
> 4 files changed, 9 insertions(+), 18 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3 08/15] mirror: Allow target that already has a BlockBackend
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (6 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 07/15] stream: Use BlockBackend for I/O Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 09/15] mirror: Use BlockBackend for I/O Kevin Wolf
` (6 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
We had to forbid mirroring to a target BDS that already had a BB
attached because the node swapping at job completion would add a second
BB and we didn't support multiple BBs on a single BDS at the time. Now
we do, so we can lift the restriction.
As we allow additional BlockBackends for the target, we must expect
other users to be sending requests. There may no requests be in flight
during the graph modification, so we have to drain those users now.
The core part of this patch is a revert of commit 40365552.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/mirror.c | 33 ++++++---------------------------
blockdev.c | 4 ----
tests/qemu-iotests/041 | 27 ---------------------------
tests/qemu-iotests/041.out | 4 ++--
4 files changed, 8 insertions(+), 60 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index efca8fc..23aa10e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -20,7 +20,6 @@
#include "qapi/qmp/qerror.h"
#include "qemu/ratelimit.h"
#include "qemu/bitmap.h"
-#include "qemu/error-report.h"
#define SLICE_TIME 100000000ULL /* ns */
#define MAX_IN_FLIGHT 16
@@ -466,24 +465,20 @@ static void mirror_exit(BlockJob *job, void *opaque)
to_replace = s->to_replace;
}
- /* This was checked in mirror_start_job(), but meanwhile one of the
- * nodes could have been newly attached to a BlockBackend. */
- if (bdrv_has_blk(to_replace) && bdrv_has_blk(s->target)) {
- error_report("block job: Can't create node with two BlockBackends");
- data->ret = -EINVAL;
- goto out;
- }
-
if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
}
+
+ /* The mirror job has no requests in flight any more, but we need to
+ * drain potential other users of the BDS before changing the graph. */
+ bdrv_drained_begin(s->target);
bdrv_replace_in_backing_chain(to_replace, s->target);
+ bdrv_drained_end(s->target);
+
/* We just changed the BDS the job BB refers to */
blk_remove_bs(job->blk);
blk_insert_bs(job->blk, src);
}
-
-out:
if (s->to_replace) {
bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
error_free(s->replace_blocker);
@@ -807,7 +802,6 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
bool is_none_mode, BlockDriverState *base)
{
MirrorBlockJob *s;
- BlockDriverState *replaced_bs;
if (granularity == 0) {
granularity = bdrv_get_default_bitmap_granularity(target);
@@ -824,21 +818,6 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
buf_size = DEFAULT_MIRROR_BUF_SIZE;
}
- /* We can't support this case as long as the block layer can't handle
- * multiple BlockBackends per BlockDriverState. */
- if (replaces) {
- replaced_bs = bdrv_lookup_bs(replaces, replaces, errp);
- if (replaced_bs == NULL) {
- return;
- }
- } else {
- replaced_bs = bs;
- }
- if (bdrv_has_blk(replaced_bs) && bdrv_has_blk(target)) {
- error_setg(errp, "Can't create node with two BlockBackends");
- return;
- }
-
s = block_job_create(driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
diff --git a/blockdev.c b/blockdev.c
index af0b022..3c06746 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3459,10 +3459,6 @@ static void blockdev_mirror_common(BlockDriverState *bs,
if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) {
return;
}
- if (bdrv_has_blk(target)) {
- error_setg(errp, "Cannot mirror to an attached block device");
- return;
- }
if (!bs->backing && sync == MIRROR_SYNC_MODE_TOP) {
sync = MIRROR_SYNC_MODE_FULL;
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index b1c542f..ed1d9d4 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -207,33 +207,6 @@ class TestSingleBlockdev(TestSingleDrive):
test_image_not_found = None
test_small_buffer2 = None
-class TestBlockdevAttached(iotests.QMPTestCase):
- image_len = 1 * 1024 * 1024 # MB
-
- def setUp(self):
- iotests.create_image(backing_img, self.image_len)
- qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
- qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, target_img)
- self.vm = iotests.VM().add_drive(test_img)
- self.vm.launch()
-
- def tearDown(self):
- self.vm.shutdown()
- os.remove(test_img)
- os.remove(target_img)
-
- def test_blockdev_attached(self):
- self.assert_no_active_block_jobs()
- args = {'options':
- {'driver': iotests.imgfmt,
- 'id': 'drive1',
- 'file': { 'filename': target_img, 'driver': 'file' } } }
- result = self.vm.qmp("blockdev-add", **args)
- self.assert_qmp(result, 'return', {})
- result = self.vm.qmp('blockdev-mirror', device='drive0', sync='full',
- target='drive1')
- self.assert_qmp(result, 'error/class', 'GenericError')
-
class TestSingleDriveZeroLength(TestSingleDrive):
image_len = 0
test_small_buffer2 = None
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index b67d050..b0cadc8 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-............................................................................
+...........................................................................
----------------------------------------------------------------------
-Ran 76 tests
+Ran 75 tests
OK
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH v3 09/15] mirror: Use BlockBackend for I/O
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (7 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 08/15] mirror: Allow target that already has a BlockBackend Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 15:02 ` Max Reitz
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 10/15] backup: Don't leak BackupBlockJob in error path Kevin Wolf
` (5 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
This changes the mirror block job to use the job's BlockBackend for
performing its I/O. job->bs isn't used by the mirroring code any more
afterwards.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/mirror.c | 70 ++++++++++++++++++++++++++++++++--------------------------
blockdev.c | 4 +---
2 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 23aa10e..80fd3c7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -35,7 +35,7 @@ typedef struct MirrorBuffer {
typedef struct MirrorBlockJob {
BlockJob common;
RateLimit limit;
- BlockDriverState *target;
+ BlockBackend *target;
BlockDriverState *base;
/* The name of the graph node to replace */
char *replaces;
@@ -156,7 +156,8 @@ static void mirror_read_complete(void *opaque, int ret)
mirror_iteration_done(op, ret);
return;
}
- bdrv_aio_writev(s->target, op->sector_num, &op->qiov, op->nb_sectors,
+ blk_aio_pwritev(s->target, op->sector_num * BDRV_SECTOR_SIZE, &op->qiov,
+ op->nb_sectors * BDRV_SECTOR_SIZE,
mirror_write_complete, op);
}
@@ -185,7 +186,7 @@ static int mirror_cow_align(MirrorBlockJob *s,
need_cow |= !test_bit((*sector_num + *nb_sectors - 1) / chunk_sectors,
s->cow_bitmap);
if (need_cow) {
- bdrv_round_to_clusters(s->target, *sector_num, *nb_sectors,
+ bdrv_round_to_clusters(blk_bs(s->target), *sector_num, *nb_sectors,
&align_sector_num, &align_nb_sectors);
}
@@ -223,7 +224,7 @@ static inline void mirror_wait_for_io(MirrorBlockJob *s)
static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
int nb_sectors)
{
- BlockDriverState *source = s->common.bs;
+ BlockBackend *source = s->common.blk;
int sectors_per_chunk, nb_chunks;
int ret = nb_sectors;
MirrorOp *op;
@@ -273,7 +274,8 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
s->sectors_in_flight += nb_sectors;
trace_mirror_one_iteration(s, sector_num, nb_sectors);
- bdrv_aio_readv(source, sector_num, &op->qiov, nb_sectors,
+ blk_aio_preadv(source, sector_num * BDRV_SECTOR_SIZE, &op->qiov,
+ nb_sectors * BDRV_SECTOR_SIZE,
mirror_read_complete, op);
return ret;
}
@@ -295,10 +297,11 @@ static void mirror_do_zero_or_discard(MirrorBlockJob *s,
s->in_flight++;
s->sectors_in_flight += nb_sectors;
if (is_discard) {
- bdrv_aio_discard(s->target, sector_num, op->nb_sectors,
- mirror_write_complete, op);
+ blk_aio_discard(s->target, sector_num, op->nb_sectors,
+ mirror_write_complete, op);
} else {
- bdrv_aio_write_zeroes(s->target, sector_num, op->nb_sectors,
+ blk_aio_pwrite_zeroes(s->target, sector_num * BDRV_SECTOR_SIZE,
+ op->nb_sectors * BDRV_SECTOR_SIZE,
s->unmap ? BDRV_REQ_MAY_UNMAP : 0,
mirror_write_complete, op);
}
@@ -306,7 +309,7 @@ static void mirror_do_zero_or_discard(MirrorBlockJob *s,
static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
{
- BlockDriverState *source = s->common.bs;
+ BlockDriverState *source = blk_bs(s->common.blk);
int64_t sector_num, first_chunk;
uint64_t delay_ns = 0;
/* At least the first dirty chunk is mirrored in one iteration. */
@@ -383,7 +386,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
} else if (ret >= 0 && !(ret & BDRV_BLOCK_DATA)) {
int64_t target_sector_num;
int target_nb_sectors;
- bdrv_round_to_clusters(s->target, sector_num, io_sectors,
+ bdrv_round_to_clusters(blk_bs(s->target), sector_num, io_sectors,
&target_sector_num, &target_nb_sectors);
if (target_sector_num == sector_num &&
target_nb_sectors == io_sectors) {
@@ -448,7 +451,8 @@ static void mirror_exit(BlockJob *job, void *opaque)
MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
MirrorExitData *data = opaque;
AioContext *replace_aio_context = NULL;
- BlockDriverState *src = s->common.bs;
+ BlockDriverState *src = blk_bs(s->common.blk);
+ BlockDriverState *target_bs = blk_bs(s->target);
/* Make sure that the source BDS doesn't go away before we called
* block_job_completed(). */
@@ -460,20 +464,20 @@ static void mirror_exit(BlockJob *job, void *opaque)
}
if (s->should_complete && data->ret == 0) {
- BlockDriverState *to_replace = s->common.bs;
+ BlockDriverState *to_replace = src;
if (s->to_replace) {
to_replace = s->to_replace;
}
- if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
- bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
+ if (bdrv_get_flags(target_bs) != bdrv_get_flags(to_replace)) {
+ bdrv_reopen(target_bs, bdrv_get_flags(to_replace), NULL);
}
/* The mirror job has no requests in flight any more, but we need to
* drain potential other users of the BDS before changing the graph. */
- bdrv_drained_begin(s->target);
- bdrv_replace_in_backing_chain(to_replace, s->target);
- bdrv_drained_end(s->target);
+ bdrv_drained_begin(target_bs);
+ bdrv_replace_in_backing_chain(to_replace, target_bs);
+ bdrv_drained_end(target_bs);
/* We just changed the BDS the job BB refers to */
blk_remove_bs(job->blk);
@@ -488,8 +492,8 @@ static void mirror_exit(BlockJob *job, void *opaque)
aio_context_release(replace_aio_context);
}
g_free(s->replaces);
- bdrv_op_unblock_all(s->target, s->common.blocker);
- bdrv_unref(s->target);
+ bdrv_op_unblock_all(target_bs, s->common.blocker);
+ blk_unref(s->target);
block_job_completed(&s->common, data->ret);
g_free(data);
bdrv_drained_end(src);
@@ -503,7 +507,8 @@ static void coroutine_fn mirror_run(void *opaque)
{
MirrorBlockJob *s = opaque;
MirrorExitData *data;
- BlockDriverState *bs = s->common.bs;
+ BlockDriverState *bs = blk_bs(s->common.blk);
+ BlockDriverState *target_bs = blk_bs(s->target);
int64_t sector_num, end, length;
uint64_t last_pause_ns;
BlockDriverInfo bdi;
@@ -539,18 +544,18 @@ static void coroutine_fn mirror_run(void *opaque)
* the destination do COW. Instead, we copy sectors around the
* dirty data if needed. We need a bitmap to do that.
*/
- bdrv_get_backing_filename(s->target, backing_filename,
+ bdrv_get_backing_filename(target_bs, backing_filename,
sizeof(backing_filename));
- if (!bdrv_get_info(s->target, &bdi) && bdi.cluster_size) {
+ if (!bdrv_get_info(target_bs, &bdi) && bdi.cluster_size) {
target_cluster_size = bdi.cluster_size;
}
- if (backing_filename[0] && !s->target->backing
+ if (backing_filename[0] && !target_bs->backing
&& s->granularity < target_cluster_size) {
s->buf_size = MAX(s->buf_size, target_cluster_size);
s->cow_bitmap = bitmap_new(length);
}
s->target_cluster_sectors = target_cluster_size >> BDRV_SECTOR_BITS;
- s->max_iov = MIN(s->common.bs->bl.max_iov, s->target->bl.max_iov);
+ s->max_iov = MIN(bs->bl.max_iov, target_bs->bl.max_iov);
end = s->bdev_length / BDRV_SECTOR_SIZE;
s->buf = qemu_try_blockalign(bs, s->buf_size);
@@ -565,7 +570,7 @@ static void coroutine_fn mirror_run(void *opaque)
if (!s->is_none_mode) {
/* First part, loop on the sectors and initialize the dirty bitmap. */
BlockDriverState *base = s->base;
- bool mark_all_dirty = s->base == NULL && !bdrv_has_zero_init(s->target);
+ bool mark_all_dirty = s->base == NULL && !bdrv_has_zero_init(target_bs);
for (sector_num = 0; sector_num < end; ) {
/* Just to make sure we are not exceeding int limit. */
@@ -635,7 +640,7 @@ static void coroutine_fn mirror_run(void *opaque)
should_complete = false;
if (s->in_flight == 0 && cnt == 0) {
trace_mirror_before_flush(s);
- ret = bdrv_flush(s->target);
+ ret = blk_flush(s->target);
if (ret < 0) {
if (mirror_error_action(s, false, -ret) ==
BLOCK_ERROR_ACTION_REPORT) {
@@ -713,7 +718,7 @@ immediate_exit:
data->ret = ret;
/* Before we switch to target in mirror_exit, make sure data doesn't
* change. */
- bdrv_drained_begin(s->common.bs);
+ bdrv_drained_begin(bs);
if (qemu_get_aio_context() == bdrv_get_aio_context(bs)) {
/* FIXME: virtio host notifiers run on iohandler_ctx, therefore the
* above bdrv_drained_end isn't enough to quiesce it. This is ugly, we
@@ -740,7 +745,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
Error *local_err = NULL;
int ret;
- ret = bdrv_open_backing_file(s->target, NULL, "backing", &local_err);
+ ret = bdrv_open_backing_file(blk_bs(s->target), NULL, "backing",
+ &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
return;
@@ -823,10 +829,12 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
return;
}
+ s->target = blk_new();
+ blk_insert_bs(s->target, target);
+
s->replaces = g_strdup(replaces);
s->on_source_error = on_source_error;
s->on_target_error = on_target_error;
- s->target = target;
s->is_none_mode = is_none_mode;
s->base = base;
s->granularity = granularity;
@@ -836,11 +844,12 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
if (!s->dirty_bitmap) {
g_free(s->replaces);
+ blk_unref(s->target);
block_job_unref(&s->common);
return;
}
- bdrv_op_block_all(s->target, s->common.blocker);
+ bdrv_op_block_all(target, s->common.blocker);
s->common.co = qemu_coroutine_create(mirror_run);
trace_mirror_start(bs, s, s->common.co, opaque);
@@ -913,7 +922,6 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
}
}
- bdrv_ref(base);
mirror_start_job(bs, base, NULL, speed, 0, 0,
on_error, on_error, false, cb, opaque, &local_err,
&commit_active_job_driver, false, base);
diff --git a/blockdev.c b/blockdev.c
index 3c06746..66477ff 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3621,9 +3621,9 @@ void qmp_drive_mirror(const char *device, const char *target,
has_on_target_error, on_target_error,
has_unmap, unmap,
&local_err);
+ bdrv_unref(target_bs);
if (local_err) {
error_propagate(errp, local_err);
- bdrv_unref(target_bs);
}
out:
aio_context_release(aio_context);
@@ -3667,7 +3667,6 @@ void qmp_blockdev_mirror(const char *device, const char *target,
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
- bdrv_ref(target_bs);
bdrv_set_aio_context(target_bs, aio_context);
blockdev_mirror_common(bs, target_bs,
@@ -3681,7 +3680,6 @@ void qmp_blockdev_mirror(const char *device, const char *target,
&local_err);
if (local_err) {
error_propagate(errp, local_err);
- bdrv_unref(target_bs);
}
aio_context_release(aio_context);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 09/15] mirror: Use BlockBackend for I/O
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 09/15] mirror: Use BlockBackend for I/O Kevin Wolf
@ 2016-05-25 15:02 ` Max Reitz
0 siblings, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 15:02 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 520 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> This changes the mirror block job to use the job's BlockBackend for
> performing its I/O. job->bs isn't used by the mirroring code any more
> afterwards.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/mirror.c | 70 ++++++++++++++++++++++++++++++++--------------------------
> blockdev.c | 4 +---
> 2 files changed, 40 insertions(+), 34 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3 10/15] backup: Don't leak BackupBlockJob in error path
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (8 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 09/15] mirror: Use BlockBackend for I/O Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 11/15] backup: Pack Notifier within BackupBlockJob Kevin Wolf
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/backup.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index fec45e8..a990cf1 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -485,6 +485,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
{
int64_t len;
BlockDriverInfo bdi;
+ BackupBlockJob *job = NULL;
int ret;
assert(bs);
@@ -542,8 +543,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
goto error;
}
- BackupBlockJob *job = block_job_create(&backup_job_driver, bs, speed,
- cb, opaque, errp);
+ job = block_job_create(&backup_job_driver, bs, speed, cb, opaque, errp);
if (!job) {
goto error;
}
@@ -584,4 +584,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
if (sync_bitmap) {
bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
}
+ if (job) {
+ block_job_unref(&job->common);
+ }
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH v3 11/15] backup: Pack Notifier within BackupBlockJob
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (9 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 10/15] backup: Don't leak BackupBlockJob in error path Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 12/15] backup: Remove bs parameter from backup_do_cow() Kevin Wolf
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
From: John Snow <jsnow@redhat.com>
Instead of relying on peeking at bs->job, we want to explicitly get
a reference to the job that was involved in this notifier callback.
Pack the Notifier inside of the BackupBlockJob so we can use
container_of to get a reference back to the BackupBlockJob object.
This cuts out one more case where we rely unnecessarily on bs->job.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/backup.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index a990cf1..a57288f 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -47,6 +47,7 @@ typedef struct BackupBlockJob {
uint64_t sectors_read;
unsigned long *done_bitmap;
int64_t cluster_size;
+ NotifierWithReturn before_write;
QLIST_HEAD(, CowRequest) inflight_reqs;
} BackupBlockJob;
@@ -94,11 +95,11 @@ static void cow_request_end(CowRequest *req)
}
static int coroutine_fn backup_do_cow(BlockDriverState *bs,
+ BackupBlockJob *job,
int64_t sector_num, int nb_sectors,
bool *error_is_read,
bool is_write_notifier)
{
- BackupBlockJob *job = (BackupBlockJob *)bs->job;
CowRequest cow_request;
struct iovec iov;
QEMUIOVector bounce_qiov;
@@ -197,6 +198,7 @@ static int coroutine_fn backup_before_write_notify(
NotifierWithReturn *notifier,
void *opaque)
{
+ BackupBlockJob *job = container_of(notifier, BackupBlockJob, before_write);
BdrvTrackedRequest *req = opaque;
int64_t sector_num = req->offset >> BDRV_SECTOR_BITS;
int nb_sectors = req->bytes >> BDRV_SECTOR_BITS;
@@ -204,7 +206,8 @@ static int coroutine_fn backup_before_write_notify(
assert((req->offset & (BDRV_SECTOR_SIZE - 1)) == 0);
assert((req->bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
- return backup_do_cow(req->bs, sector_num, nb_sectors, NULL, true);
+ return backup_do_cow(req->bs, job, sector_num,
+ nb_sectors, NULL, true);
}
static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
@@ -343,7 +346,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
if (yield_and_check(job)) {
return ret;
}
- ret = backup_do_cow(bs, cluster * sectors_per_cluster,
+ ret = backup_do_cow(bs, job, cluster * sectors_per_cluster,
sectors_per_cluster, &error_is_read,
false);
if ((ret < 0) &&
@@ -378,9 +381,6 @@ static void coroutine_fn backup_run(void *opaque)
BackupCompleteData *data;
BlockDriverState *bs = job->common.bs;
BlockDriverState *target = job->target;
- NotifierWithReturn before_write = {
- .notify = backup_before_write_notify,
- };
int64_t start, end;
int64_t sectors_per_cluster = cluster_size_sectors(job);
int ret = 0;
@@ -393,7 +393,8 @@ static void coroutine_fn backup_run(void *opaque)
job->done_bitmap = bitmap_new(end);
- bdrv_add_before_write_notifier(bs, &before_write);
+ job->before_write.notify = backup_before_write_notify;
+ bdrv_add_before_write_notifier(bs, &job->before_write);
if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
while (!block_job_is_cancelled(&job->common)) {
@@ -445,7 +446,7 @@ static void coroutine_fn backup_run(void *opaque)
}
}
/* FULL sync mode we copy the whole drive. */
- ret = backup_do_cow(bs, start * sectors_per_cluster,
+ ret = backup_do_cow(bs, job, start * sectors_per_cluster,
sectors_per_cluster, &error_is_read, false);
if (ret < 0) {
/* Depending on error action, fail now or retry cluster */
@@ -461,7 +462,7 @@ static void coroutine_fn backup_run(void *opaque)
}
}
- notifier_with_return_remove(&before_write);
+ notifier_with_return_remove(&job->before_write);
/* wait until pending backup_do_cow() calls have completed */
qemu_co_rwlock_wrlock(&job->flush_rwlock);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH v3 12/15] backup: Remove bs parameter from backup_do_cow()
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (10 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 11/15] backup: Pack Notifier within BackupBlockJob Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 13/15] backup: Use BlockBackend for I/O Kevin Wolf
` (2 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
Now that we pass the job to the function, bs is implied by that.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
block/backup.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index a57288f..670ba50 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -94,12 +94,12 @@ static void cow_request_end(CowRequest *req)
qemu_co_queue_restart_all(&req->wait_queue);
}
-static int coroutine_fn backup_do_cow(BlockDriverState *bs,
- BackupBlockJob *job,
+static int coroutine_fn backup_do_cow(BackupBlockJob *job,
int64_t sector_num, int nb_sectors,
bool *error_is_read,
bool is_write_notifier)
{
+ BlockDriverState *bs = job->common.bs;
CowRequest cow_request;
struct iovec iov;
QEMUIOVector bounce_qiov;
@@ -203,11 +203,11 @@ static int coroutine_fn backup_before_write_notify(
int64_t sector_num = req->offset >> BDRV_SECTOR_BITS;
int nb_sectors = req->bytes >> BDRV_SECTOR_BITS;
+ assert(req->bs == job->common.bs);
assert((req->offset & (BDRV_SECTOR_SIZE - 1)) == 0);
assert((req->bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
- return backup_do_cow(req->bs, job, sector_num,
- nb_sectors, NULL, true);
+ return backup_do_cow(job, sector_num, nb_sectors, NULL, true);
}
static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
@@ -324,7 +324,6 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
int64_t end;
int64_t last_cluster = -1;
int64_t sectors_per_cluster = cluster_size_sectors(job);
- BlockDriverState *bs = job->common.bs;
HBitmapIter hbi;
granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
@@ -346,7 +345,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
if (yield_and_check(job)) {
return ret;
}
- ret = backup_do_cow(bs, job, cluster * sectors_per_cluster,
+ ret = backup_do_cow(job, cluster * sectors_per_cluster,
sectors_per_cluster, &error_is_read,
false);
if ((ret < 0) &&
@@ -446,7 +445,7 @@ static void coroutine_fn backup_run(void *opaque)
}
}
/* FULL sync mode we copy the whole drive. */
- ret = backup_do_cow(bs, job, start * sectors_per_cluster,
+ ret = backup_do_cow(job, start * sectors_per_cluster,
sectors_per_cluster, &error_is_read, false);
if (ret < 0) {
/* Depending on error action, fail now or retry cluster */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [Qemu-devel] [PATCH v3 13/15] backup: Use BlockBackend for I/O
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (11 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 12/15] backup: Remove bs parameter from backup_do_cow() Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 15:17 ` Max Reitz
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 14/15] commit: " Kevin Wolf
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 15/15] blockjob: Remove BlockJob.bs Kevin Wolf
14 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
This changes the backup block job to use the job's BlockBackend for
performing its I/O. job->bs isn't used by the backup code any more
afterwards.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/backup.c | 46 +++++++++++++++++++++-------------------------
block/io.c | 9 ---------
blockdev.c | 4 +---
include/block/block.h | 2 --
trace-events | 1 -
5 files changed, 22 insertions(+), 40 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 670ba50..feeb9f8 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -36,7 +36,7 @@ typedef struct CowRequest {
typedef struct BackupBlockJob {
BlockJob common;
- BlockDriverState *target;
+ BlockBackend *target;
/* bitmap for sync=incremental */
BdrvDirtyBitmap *sync_bitmap;
MirrorSyncMode sync_mode;
@@ -99,7 +99,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
bool *error_is_read,
bool is_write_notifier)
{
- BlockDriverState *bs = job->common.bs;
+ BlockBackend *blk = job->common.blk;
CowRequest cow_request;
struct iovec iov;
QEMUIOVector bounce_qiov;
@@ -132,20 +132,15 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
start * sectors_per_cluster);
if (!bounce_buffer) {
- bounce_buffer = qemu_blockalign(bs, job->cluster_size);
+ bounce_buffer = blk_blockalign(blk, job->cluster_size);
}
iov.iov_base = bounce_buffer;
iov.iov_len = n * BDRV_SECTOR_SIZE;
qemu_iovec_init_external(&bounce_qiov, &iov, 1);
- if (is_write_notifier) {
- ret = bdrv_co_readv_no_serialising(bs,
- start * sectors_per_cluster,
- n, &bounce_qiov);
- } else {
- ret = bdrv_co_readv(bs, start * sectors_per_cluster, n,
- &bounce_qiov);
- }
+ ret = blk_co_preadv(blk, start * job->cluster_size,
+ bounce_qiov.size, &bounce_qiov,
+ is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
if (ret < 0) {
trace_backup_do_cow_read_fail(job, start, ret);
if (error_is_read) {
@@ -155,13 +150,11 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
}
if (buffer_is_zero(iov.iov_base, iov.iov_len)) {
- ret = bdrv_co_write_zeroes(job->target,
- start * sectors_per_cluster,
- n, BDRV_REQ_MAY_UNMAP);
+ ret = blk_co_pwrite_zeroes(job->target, start * job->cluster_size,
+ bounce_qiov.size, BDRV_REQ_MAY_UNMAP);
} else {
- ret = bdrv_co_writev(job->target,
- start * sectors_per_cluster, n,
- &bounce_qiov);
+ ret = blk_co_pwritev(job->target, start * job->cluster_size,
+ bounce_qiov.size, &bounce_qiov, 0);
}
if (ret < 0) {
trace_backup_do_cow_write_fail(job, start, ret);
@@ -203,7 +196,7 @@ static int coroutine_fn backup_before_write_notify(
int64_t sector_num = req->offset >> BDRV_SECTOR_BITS;
int nb_sectors = req->bytes >> BDRV_SECTOR_BITS;
- assert(req->bs == job->common.bs);
+ assert(req->bs == blk_bs(job->common.blk));
assert((req->offset & (BDRV_SECTOR_SIZE - 1)) == 0);
assert((req->bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
@@ -224,7 +217,7 @@ static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
{
BdrvDirtyBitmap *bm;
- BlockDriverState *bs = job->common.bs;
+ BlockDriverState *bs = blk_bs(job->common.blk);
if (ret < 0 || block_job_is_cancelled(&job->common)) {
/* Merge the successor back into the parent, delete nothing. */
@@ -282,7 +275,7 @@ static void backup_complete(BlockJob *job, void *opaque)
BackupBlockJob *s = container_of(job, BackupBlockJob, common);
BackupCompleteData *data = opaque;
- bdrv_unref(s->target);
+ blk_unref(s->target);
block_job_completed(job, data->ret);
g_free(data);
@@ -378,8 +371,8 @@ static void coroutine_fn backup_run(void *opaque)
{
BackupBlockJob *job = opaque;
BackupCompleteData *data;
- BlockDriverState *bs = job->common.bs;
- BlockDriverState *target = job->target;
+ BlockDriverState *bs = blk_bs(job->common.blk);
+ BlockBackend *target = job->target;
int64_t start, end;
int64_t sectors_per_cluster = cluster_size_sectors(job);
int ret = 0;
@@ -468,7 +461,7 @@ static void coroutine_fn backup_run(void *opaque)
qemu_co_rwlock_unlock(&job->flush_rwlock);
g_free(job->done_bitmap);
- bdrv_op_unblock_all(target, job->common.blocker);
+ bdrv_op_unblock_all(blk_bs(target), job->common.blocker);
data = g_malloc(sizeof(*data));
data->ret = ret;
@@ -548,9 +541,11 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
goto error;
}
+ job->target = blk_new();
+ blk_insert_bs(job->target, target);
+
job->on_source_error = on_source_error;
job->on_target_error = on_target_error;
- job->target = target;
job->sync_mode = sync_mode;
job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ?
sync_bitmap : NULL;
@@ -558,7 +553,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
/* If there is no backing file on the target, we cannot rely on COW if our
* backup cluster size is smaller than the target cluster size. Even for
* targets with a backing file, try to avoid COW if possible. */
- ret = bdrv_get_info(job->target, &bdi);
+ ret = bdrv_get_info(target, &bdi);
if (ret < 0 && !target->backing) {
error_setg_errno(errp, -ret,
"Couldn't determine the cluster size of the target image, "
@@ -585,6 +580,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
}
if (job) {
+ blk_unref(job->target);
block_job_unref(&job->common);
}
}
diff --git a/block/io.c b/block/io.c
index bc2eda2..2d832aa 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1108,15 +1108,6 @@ int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov, 0);
}
-int coroutine_fn bdrv_co_readv_no_serialising(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
-{
- trace_bdrv_co_readv_no_serialising(bs, sector_num, nb_sectors);
-
- return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov,
- BDRV_REQ_NO_SERIALISING);
-}
-
#define MAX_WRITE_ZEROES_BOUNCE_BUFFER 32768
static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
diff --git a/blockdev.c b/blockdev.c
index 66477ff..717785e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3293,8 +3293,8 @@ static void do_drive_backup(const char *device, const char *target,
backup_start(bs, target_bs, speed, sync, bmap,
on_source_error, on_target_error,
block_job_cb, bs, txn, &local_err);
+ bdrv_unref(target_bs);
if (local_err != NULL) {
- bdrv_unref(target_bs);
error_propagate(errp, local_err);
goto out;
}
@@ -3378,12 +3378,10 @@ void do_blockdev_backup(const char *device, const char *target,
}
target_bs = blk_bs(target_blk);
- bdrv_ref(target_bs);
bdrv_set_aio_context(target_bs, aio_context);
backup_start(bs, target_bs, speed, sync, NULL, on_source_error,
on_target_error, block_job_cb, bs, txn, &local_err);
if (local_err != NULL) {
- bdrv_unref(target_bs);
error_propagate(errp, local_err);
}
out:
diff --git a/include/block/block.h b/include/block/block.h
index 0a4b973..70ea299 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -242,8 +242,6 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
const void *buf, int count);
int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov);
-int coroutine_fn bdrv_co_readv_no_serialising(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov);
/*
diff --git a/trace-events b/trace-events
index 1bd595b..a7aca8d 100644
--- a/trace-events
+++ b/trace-events
@@ -72,7 +72,6 @@ bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %
bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p"
bdrv_aio_write_zeroes(void *bs, int64_t sector_num, int nb_sectors, int flags, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x opaque %p"
bdrv_co_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
-bdrv_co_readv_no_serialising(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
bdrv_co_write_zeroes(void *bs, int64_t sector_num, int nb_sector, int flags) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x"
bdrv_co_do_copy_on_readv(void *bs, int64_t sector_num, int nb_sectors, int64_t cluster_sector_num, int cluster_nb_sectors) "bs %p sector_num %"PRId64" nb_sectors %d cluster_sector_num %"PRId64" cluster_nb_sectors %d"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 13/15] backup: Use BlockBackend for I/O
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 13/15] backup: Use BlockBackend for I/O Kevin Wolf
@ 2016-05-25 15:17 ` Max Reitz
0 siblings, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 15:17 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 627 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> This changes the backup block job to use the job's BlockBackend for
> performing its I/O. job->bs isn't used by the backup code any more
> afterwards.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/backup.c | 46 +++++++++++++++++++++-------------------------
> block/io.c | 9 ---------
> blockdev.c | 4 +---
> include/block/block.h | 2 --
> trace-events | 1 -
> 5 files changed, 22 insertions(+), 40 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3 14/15] commit: Use BlockBackend for I/O
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (12 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 13/15] backup: Use BlockBackend for I/O Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 15:19 ` Max Reitz
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 15/15] blockjob: Remove BlockJob.bs Kevin Wolf
14 siblings, 1 reply; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
This changes the commit block job to use the job's BlockBackend for
performing its I/O. job->bs isn't used by the commit code any more
afterwards.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/commit.c | 53 +++++++++++++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index f308c8c..8a00e11 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -36,28 +36,36 @@ typedef struct CommitBlockJob {
BlockJob common;
RateLimit limit;
BlockDriverState *active;
- BlockDriverState *top;
- BlockDriverState *base;
+ BlockBackend *top;
+ BlockBackend *base;
BlockdevOnError on_error;
int base_flags;
int orig_overlay_flags;
char *backing_file_str;
} CommitBlockJob;
-static int coroutine_fn commit_populate(BlockDriverState *bs,
- BlockDriverState *base,
+static int coroutine_fn commit_populate(BlockBackend *bs, BlockBackend *base,
int64_t sector_num, int nb_sectors,
void *buf)
{
int ret = 0;
+ QEMUIOVector qiov;
+ struct iovec iov = {
+ .iov_base = buf,
+ .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
+ };
- ret = bdrv_read(bs, sector_num, buf, nb_sectors);
- if (ret) {
+ qemu_iovec_init_external(&qiov, &iov, 1);
+
+ ret = blk_co_preadv(bs, sector_num * BDRV_SECTOR_SIZE,
+ qiov.size, &qiov, 0);
+ if (ret < 0) {
return ret;
}
- ret = bdrv_write(base, sector_num, buf, nb_sectors);
- if (ret) {
+ ret = blk_co_pwritev(base, sector_num * BDRV_SECTOR_SIZE,
+ qiov.size, &qiov, 0);
+ if (ret < 0) {
return ret;
}
@@ -73,8 +81,8 @@ static void commit_complete(BlockJob *job, void *opaque)
CommitBlockJob *s = container_of(job, CommitBlockJob, common);
CommitCompleteData *data = opaque;
BlockDriverState *active = s->active;
- BlockDriverState *top = s->top;
- BlockDriverState *base = s->base;
+ BlockDriverState *top = blk_bs(s->top);
+ BlockDriverState *base = blk_bs(s->base);
BlockDriverState *overlay_bs;
int ret = data->ret;
@@ -94,6 +102,8 @@ static void commit_complete(BlockJob *job, void *opaque)
bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL);
}
g_free(s->backing_file_str);
+ blk_unref(s->top);
+ blk_unref(s->base);
block_job_completed(&s->common, ret);
g_free(data);
}
@@ -102,8 +112,6 @@ static void coroutine_fn commit_run(void *opaque)
{
CommitBlockJob *s = opaque;
CommitCompleteData *data;
- BlockDriverState *top = s->top;
- BlockDriverState *base = s->base;
int64_t sector_num, end;
int ret = 0;
int n = 0;
@@ -111,27 +119,27 @@ static void coroutine_fn commit_run(void *opaque)
int bytes_written = 0;
int64_t base_len;
- ret = s->common.len = bdrv_getlength(top);
+ ret = s->common.len = blk_getlength(s->top);
if (s->common.len < 0) {
goto out;
}
- ret = base_len = bdrv_getlength(base);
+ ret = base_len = blk_getlength(s->base);
if (base_len < 0) {
goto out;
}
if (base_len < s->common.len) {
- ret = bdrv_truncate(base, s->common.len);
+ ret = blk_truncate(s->base, s->common.len);
if (ret) {
goto out;
}
}
end = s->common.len >> BDRV_SECTOR_BITS;
- buf = qemu_blockalign(top, COMMIT_BUFFER_SIZE);
+ buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE);
for (sector_num = 0; sector_num < end; sector_num += n) {
uint64_t delay_ns = 0;
@@ -146,7 +154,8 @@ wait:
break;
}
/* Copy if allocated above the base */
- ret = bdrv_is_allocated_above(top, base, sector_num,
+ ret = bdrv_is_allocated_above(blk_bs(s->top), blk_bs(s->base),
+ sector_num,
COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE,
&n);
copy = (ret == 1);
@@ -158,7 +167,7 @@ wait:
goto wait;
}
}
- ret = commit_populate(top, base, sector_num, n, buf);
+ ret = commit_populate(s->top, s->base, sector_num, n, buf);
bytes_written += n * BDRV_SECTOR_SIZE;
}
if (ret < 0) {
@@ -253,8 +262,12 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
return;
}
- s->base = base;
- s->top = top;
+ s->base = blk_new();
+ blk_insert_bs(s->base, base);
+
+ s->top = blk_new();
+ blk_insert_bs(s->top, top);
+
s->active = bs;
s->base_flags = orig_base_flags;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 14/15] commit: Use BlockBackend for I/O
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 14/15] commit: " Kevin Wolf
@ 2016-05-25 15:19 ` Max Reitz
0 siblings, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 15:19 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 482 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> This changes the commit block job to use the job's BlockBackend for
> performing its I/O. job->bs isn't used by the commit code any more
> afterwards.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> block/commit.c | 53 +++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 33 insertions(+), 20 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH v3 15/15] blockjob: Remove BlockJob.bs
2016-05-25 12:29 [Qemu-devel] [PATCH v3 00/15] block jobs: Convert I/O to BlockBackend Kevin Wolf
` (13 preceding siblings ...)
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 14/15] commit: " Kevin Wolf
@ 2016-05-25 12:29 ` Kevin Wolf
2016-05-25 15:21 ` Max Reitz
2016-05-25 15:48 ` Eric Blake
14 siblings, 2 replies; 28+ messages in thread
From: Kevin Wolf @ 2016-05-25 12:29 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, berto, jcody, jsnow, qemu-devel, eblake
There is a single remaining user in qemu-img, and another one in a test
case, both of which can be trivially converted to using BlockJob.blk
instead.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockjob.c | 1 -
include/block/blockjob.h | 1 -
qemu-img.c | 2 +-
tests/test-blockjob-txn.c | 3 ++-
4 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index 2097e1d..c095cc5 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -83,7 +83,6 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
job->driver = driver;
job->id = g_strdup(bdrv_get_device_name(bs));
- job->bs = bs;
job->blk = blk;
job->cb = cb;
job->opaque = opaque;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 32012af..86d2807 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -82,7 +82,6 @@ struct BlockJob {
const BlockJobDriver *driver;
/** The block device on which the job is operating. */
- BlockDriverState *bs; /* TODO Remove */
BlockBackend *blk;
/**
diff --git a/qemu-img.c b/qemu-img.c
index 2065348..4b56ad3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -775,7 +775,7 @@ static void common_block_job_cb(void *opaque, int ret)
static void run_block_job(BlockJob *job, Error **errp)
{
- AioContext *aio_context = bdrv_get_aio_context(job->bs);
+ AioContext *aio_context = blk_get_aio_context(job->blk);
do {
aio_poll(aio_context, true);
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index 55fad95..828389b 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -15,6 +15,7 @@
#include "qapi/error.h"
#include "qemu/main-loop.h"
#include "block/blockjob.h"
+#include "sysemu/block-backend.h"
typedef struct {
BlockJob common;
@@ -30,7 +31,7 @@ static const BlockJobDriver test_block_job_driver = {
static void test_block_job_complete(BlockJob *job, void *opaque)
{
- BlockDriverState *bs = job->bs;
+ BlockDriverState *bs = blk_bs(job->blk);
int rc = (intptr_t)opaque;
if (block_job_is_cancelled(job)) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [Qemu-devel] [PATCH v3 15/15] blockjob: Remove BlockJob.bs
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 15/15] blockjob: Remove BlockJob.bs Kevin Wolf
@ 2016-05-25 15:21 ` Max Reitz
2016-05-25 15:48 ` Eric Blake
1 sibling, 0 replies; 28+ messages in thread
From: Max Reitz @ 2016-05-25 15:21 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: berto, jcody, jsnow, qemu-devel, eblake
[-- Attachment #1: Type: text/plain, Size: 506 bytes --]
On 25.05.2016 14:29, Kevin Wolf wrote:
> There is a single remaining user in qemu-img, and another one in a test
> case, both of which can be trivially converted to using BlockJob.blk
> instead.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockjob.c | 1 -
> include/block/blockjob.h | 1 -
> qemu-img.c | 2 +-
> tests/test-blockjob-txn.c | 3 ++-
> 4 files changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH v3 15/15] blockjob: Remove BlockJob.bs
2016-05-25 12:29 ` [Qemu-devel] [PATCH v3 15/15] blockjob: Remove BlockJob.bs Kevin Wolf
2016-05-25 15:21 ` Max Reitz
@ 2016-05-25 15:48 ` Eric Blake
1 sibling, 0 replies; 28+ messages in thread
From: Eric Blake @ 2016-05-25 15:48 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, berto, jcody, jsnow, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 619 bytes --]
On 05/25/2016 06:29 AM, Kevin Wolf wrote:
> There is a single remaining user in qemu-img, and another one in a test
> case, both of which can be trivially converted to using BlockJob.blk
> instead.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockjob.c | 1 -
> include/block/blockjob.h | 1 -
> qemu-img.c | 2 +-
> tests/test-blockjob-txn.c | 3 ++-
> 4 files changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread