* [Qemu-devel] [PULL v2 01/34] stream: Fix prototype of stream_start()
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 02/34] blockjob: Update description of the 'id' field Kevin Wolf
` (33 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
'stream-start' has a parameter called 'backing-file', which is the
string to be written to bs->backing when the job finishes.
In the stream_start() implementation it is called 'backing_file_str',
but it the prototype in the header file it is called 'base_id'.
This patch fixes it so the name is the same in both cases and is
consistent with other cases (like commit_start()).
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block_int.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 47b9aac..2a27a194 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -642,8 +642,8 @@ int is_windows_drive(const char *filename);
* @bs: Block device to operate on.
* @base: Block device that will become the new base, or %NULL to
* flatten the whole backing file chain onto @bs.
- * @base_id: The file name that will be written to @bs as the new
- * backing file if the job completes. Ignored if @base is %NULL.
+ * @backing_file_str: The file name that will be written to @bs as the
+ * the new backing file if the job completes. Ignored if @base is %NULL.
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
* @on_error: The action to take upon error.
* @cb: Completion function for the job.
@@ -654,11 +654,12 @@ int is_windows_drive(const char *filename);
* in @bs, but allocated in any image between @base and @bs (both
* exclusive) will be written to @bs. At the end of a successful
* streaming job, the backing file of @bs will be changed to
- * @base_id in the written image and to @base in the live BlockDriverState.
+ * @backing_file_str in the written image and to @base in the live
+ * BlockDriverState.
*/
void stream_start(BlockDriverState *bs, BlockDriverState *base,
- const char *base_id, int64_t speed, BlockdevOnError on_error,
- BlockCompletionFunc *cb,
+ const char *backing_file_str, int64_t speed,
+ BlockdevOnError on_error, BlockCompletionFunc *cb,
void *opaque, Error **errp);
/**
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 02/34] blockjob: Update description of the 'id' field
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 01/34] stream: Fix prototype of stream_start() Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 03/34] blockjob: Add block_job_get() Kevin Wolf
` (32 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
The 'id' field of the BlockJob structure will be able to hold any ID,
not only a device name. This patch updates the description of that
field and the error messages where it is being used.
Soon we'll add the ability to set an arbitrary ID when creating a
block job.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/mirror.c | 3 ++-
blockjob.c | 3 ++-
include/block/blockjob.h | 5 +----
include/qapi/qmp/qerror.h | 3 ---
4 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 8d96049..6e3dbd2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -761,7 +761,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
target = blk_bs(s->target);
if (!s->synced) {
- error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
+ error_setg(errp, "The active block job '%s' cannot be completed",
+ job->id);
return;
}
diff --git a/blockjob.c b/blockjob.c
index 205da9d..ce0e27c 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -290,7 +290,8 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
void block_job_complete(BlockJob *job, Error **errp)
{
if (job->pause_count || job->cancelled || !job->driver->complete) {
- error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
+ error_setg(errp, "The active block job '%s' cannot be completed",
+ job->id);
return;
}
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index f7f5687..97b86f1 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -107,10 +107,7 @@ struct BlockJob {
BlockBackend *blk;
/**
- * The ID of the block job. Currently the BlockBackend name of the BDS
- * owning the job at the time when the job is started.
- *
- * TODO Decouple block job IDs from BlockBackend names
+ * The ID of the block job.
*/
char *id;
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index d08652a..6586c9f 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -19,9 +19,6 @@
#define QERR_BASE_NOT_FOUND \
"Base '%s' not found"
-#define QERR_BLOCK_JOB_NOT_READY \
- "The active block job for device '%s' cannot be completed"
-
#define QERR_BUS_NO_HOTPLUG \
"Bus '%s' does not support hotplugging"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 03/34] blockjob: Add block_job_get()
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 01/34] stream: Fix prototype of stream_start() Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 02/34] blockjob: Update description of the 'id' field Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 04/34] block: Use block_job_get() in find_block_job() Kevin Wolf
` (31 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
Currently the way to look for a specific block job is to iterate the
list manually using block_job_next().
Since we want to be able to identify a job primarily by its ID it
makes sense to have a function that does just that.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockjob.c | 13 +++++++++++++
include/block/blockjob.h | 10 ++++++++++
2 files changed, 23 insertions(+)
diff --git a/blockjob.c b/blockjob.c
index ce0e27c..ca2291b 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -60,6 +60,19 @@ BlockJob *block_job_next(BlockJob *job)
return QLIST_NEXT(job, job_list);
}
+BlockJob *block_job_get(const char *id)
+{
+ BlockJob *job;
+
+ QLIST_FOREACH(job, &block_jobs, job_list) {
+ if (!strcmp(id, job->id)) {
+ return job;
+ }
+ }
+
+ return NULL;
+}
+
/* Normally the job runs in its BlockBackend's AioContext. The exception is
* block_job_defer_to_main_loop() where it runs in the QEMU main loop. Code
* that supports both cases uses this helper function.
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 97b86f1..934bf1c 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -212,6 +212,16 @@ struct BlockJob {
BlockJob *block_job_next(BlockJob *job);
/**
+ * block_job_get:
+ * @id: The id of the block job.
+ *
+ * Get the block job identified by @id (which must not be %NULL).
+ *
+ * Returns the requested job, or %NULL if it doesn't exist.
+ */
+BlockJob *block_job_get(const char *id);
+
+/**
* 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] 37+ messages in thread* [Qemu-devel] [PULL v2 04/34] block: Use block_job_get() in find_block_job()
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 03/34] blockjob: Add block_job_get() Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 05/34] blockjob: Add 'job_id' parameter to block_job_create() Kevin Wolf
` (30 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
find_block_job() looks for a block backend with a specified name,
checks whether it has a block job and acquires its AioContext.
We want to identify jobs by their ID and not by the block backend
they're attached to, so this patch ignores the backends altogether and
gets the job directly. Apart from making the code simpler, this will
allow us to find block jobs once they start having user-specified IDs.
To ensure backward compatibility we keep ERROR_CLASS_DEVICE_NOT_ACTIVE
as the error class if the job doesn't exist. In subsequent patches
we'll also need to keep the device name as the default job ID if the
user doesn't specify a different one.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 40 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 0f8065c..e649521 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3704,42 +3704,28 @@ void qmp_blockdev_mirror(const char *device, const char *target,
aio_context_release(aio_context);
}
-/* Get the block job for a given device name and acquire its AioContext */
-static BlockJob *find_block_job(const char *device, AioContext **aio_context,
+/* Get a block job using its ID and acquire its AioContext */
+static BlockJob *find_block_job(const char *id, AioContext **aio_context,
Error **errp)
{
- BlockBackend *blk;
- BlockDriverState *bs;
+ BlockJob *job;
- *aio_context = NULL;
+ assert(id != NULL);
- blk = blk_by_name(device);
- if (!blk) {
- goto notfound;
- }
-
- *aio_context = blk_get_aio_context(blk);
- aio_context_acquire(*aio_context);
+ *aio_context = NULL;
- if (!blk_is_available(blk)) {
- goto notfound;
- }
- bs = blk_bs(blk);
+ job = block_job_get(id);
- if (!bs->job) {
- goto notfound;
+ if (!job) {
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
+ "Block job '%s' not found", id);
+ return NULL;
}
- return bs->job;
+ *aio_context = blk_get_aio_context(job->blk);
+ aio_context_acquire(*aio_context);
-notfound:
- error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
- "No active block job on device '%s'", device);
- if (*aio_context) {
- aio_context_release(*aio_context);
- *aio_context = NULL;
- }
- return NULL;
+ return job;
}
void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 05/34] blockjob: Add 'job_id' parameter to block_job_create()
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (3 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 04/34] block: Use block_job_get() in find_block_job() Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 06/34] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' Kevin Wolf
` (29 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
When a new job is created, the job ID is taken from the device name of
the BDS. This patch adds a new 'job_id' parameter to let the caller
provide one instead.
This patch also verifies that the ID is always unique and well-formed.
This causes problems in a couple of places where no ID is being set,
because the BDS does not have a device name.
In the case of test_block_job_start() (from test-blockjob-txn.c) we
can simply use this new 'job_id' parameter to set the missing ID.
In the case of img_commit() (from qemu-img.c) we still don't have the
API to make commit_active_start() set the job ID, so we solve it by
setting a default value. We'll get rid of this as soon as we extend
the API.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/backup.c | 3 ++-
block/commit.c | 2 +-
block/mirror.c | 2 +-
block/stream.c | 2 +-
blockjob.c | 29 +++++++++++++++++++++++++----
include/block/blockjob.h | 8 +++++---
tests/test-blockjob-txn.c | 7 +++++--
7 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index f87f8d5..19c587c 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -541,7 +541,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
goto error;
}
- job = block_job_create(&backup_job_driver, bs, speed, cb, opaque, errp);
+ job = block_job_create(NULL, &backup_job_driver, bs, speed,
+ cb, opaque, errp);
if (!job) {
goto error;
}
diff --git a/block/commit.c b/block/commit.c
index 379efb7..137bb03 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -236,7 +236,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
return;
}
- s = block_job_create(&commit_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(NULL, &commit_job_driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/block/mirror.c b/block/mirror.c
index 6e3dbd2..08d88ca 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -873,7 +873,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
buf_size = DEFAULT_MIRROR_BUF_SIZE;
}
- s = block_job_create(driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(NULL, driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/block/stream.c b/block/stream.c
index c0efbda..e4319d3 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -226,7 +226,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base,
{
StreamBlockJob *s;
- s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(NULL, &stream_job_driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/blockjob.c b/blockjob.c
index ca2291b..511c0db 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -33,6 +33,7 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qjson.h"
#include "qemu/coroutine.h"
+#include "qemu/id.h"
#include "qmp-commands.h"
#include "qemu/timer.h"
#include "qapi-event.h"
@@ -116,9 +117,9 @@ static void block_job_detach_aio_context(void *opaque)
block_job_unref(job);
}
-void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
- int64_t speed, BlockCompletionFunc *cb,
- void *opaque, Error **errp)
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+ BlockDriverState *bs, int64_t speed,
+ BlockCompletionFunc *cb, void *opaque, Error **errp)
{
BlockBackend *blk;
BlockJob *job;
@@ -129,6 +130,26 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
return NULL;
}
+ if (job_id == NULL) {
+ job_id = bdrv_get_device_name(bs);
+ /* Assign a default ID if the BDS does not have a device
+ * name. We'll get rid of this soon when we finish extending
+ * the API of all commands that create block jobs. */
+ if (job_id[0] == '\0') {
+ job_id = "default_job";
+ }
+ }
+
+ if (!id_wellformed(job_id)) {
+ error_setg(errp, "Invalid job ID '%s'", job_id);
+ return NULL;
+ }
+
+ if (block_job_get(job_id)) {
+ error_setg(errp, "Job ID '%s' already in use", job_id);
+ return NULL;
+ }
+
blk = blk_new();
blk_insert_bs(blk, bs);
@@ -139,7 +160,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
job->driver = driver;
- job->id = g_strdup(bdrv_get_device_name(bs));
+ job->id = g_strdup(job_id);
job->blk = blk;
job->cb = cb;
job->opaque = opaque;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 934bf1c..4ddb4ae 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -223,6 +223,8 @@ BlockJob *block_job_get(const char *id);
/**
* block_job_create:
+ * @job_id: The id of the newly-created job, or %NULL to have one
+ * generated automatically.
* @job_type: The class object for the newly-created job.
* @bs: The block
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
@@ -239,9 +241,9 @@ BlockJob *block_job_get(const char *id);
* This function is not part of the public job interface; it should be
* called from a wrapper that is specific to the job type.
*/
-void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
- int64_t speed, BlockCompletionFunc *cb,
- void *opaque, Error **errp);
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+ BlockDriverState *bs, int64_t speed,
+ BlockCompletionFunc *cb, void *opaque, Error **errp);
/**
* block_job_sleep_ns:
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index d3030f1..50e232a 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -91,11 +91,14 @@ static BlockJob *test_block_job_start(unsigned int iterations,
BlockDriverState *bs;
TestBlockJob *s;
TestBlockJobCBData *data;
+ static unsigned counter;
+ char job_id[24];
data = g_new0(TestBlockJobCBData, 1);
bs = bdrv_new();
- s = block_job_create(&test_block_job_driver, bs, 0, test_block_job_cb,
- data, &error_abort);
+ snprintf(job_id, sizeof(job_id), "job%u", counter++);
+ s = block_job_create(job_id, &test_block_job_driver, bs, 0,
+ test_block_job_cb, data, &error_abort);
s->iterations = iterations;
s->use_timer = use_timer;
s->rc = rc;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 06/34] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (4 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 05/34] blockjob: Add 'job_id' parameter to block_job_create() Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 07/34] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup' Kevin Wolf
` (28 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
This patch adds a new optional 'job-id' parameter to 'blockdev-mirror'
and 'drive-mirror', allowing the user to specify the ID of the block
job to be created.
The HMP 'drive_mirror' command remains unchanged.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/mirror.c | 15 ++++++++-------
blockdev.c | 15 ++++++++-------
hmp.c | 2 +-
include/block/block_int.h | 6 ++++--
qapi/block-core.json | 12 +++++++++---
qmp-commands.hx | 10 +++++++---
6 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 08d88ca..702a686 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -843,8 +843,8 @@ static const BlockJobDriver commit_active_job_driver = {
.attached_aio_context = mirror_attached_aio_context,
};
-static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
- const char *replaces,
+static void mirror_start_job(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, const char *replaces,
int64_t speed, uint32_t granularity,
int64_t buf_size,
BlockMirrorBackingMode backing_mode,
@@ -873,7 +873,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
buf_size = DEFAULT_MIRROR_BUF_SIZE;
}
- s = block_job_create(NULL, driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(job_id, driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
@@ -906,8 +906,8 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
qemu_coroutine_enter(s->common.co, s);
}
-void mirror_start(BlockDriverState *bs, BlockDriverState *target,
- const char *replaces,
+void mirror_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, const char *replaces,
int64_t speed, uint32_t granularity, int64_t buf_size,
MirrorSyncMode mode, BlockMirrorBackingMode backing_mode,
BlockdevOnError on_source_error,
@@ -925,7 +925,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
}
is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
- mirror_start_job(bs, target, replaces,
+ mirror_start_job(job_id, bs, target, replaces,
speed, granularity, buf_size, backing_mode,
on_source_error, on_target_error, unmap, cb, opaque, errp,
&mirror_job_driver, is_none_mode, base);
@@ -973,7 +973,8 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
}
}
- mirror_start_job(bs, base, NULL, speed, 0, 0, MIRROR_LEAVE_BACKING_CHAIN,
+ mirror_start_job(NULL, bs, base, NULL, speed, 0, 0,
+ MIRROR_LEAVE_BACKING_CHAIN,
on_error, on_error, false, cb, opaque, &local_err,
&commit_active_job_driver, false, base);
if (local_err) {
diff --git a/blockdev.c b/blockdev.c
index e649521..2008690 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3422,7 +3422,7 @@ void qmp_blockdev_backup(const char *device, const char *target,
/* Parameter check and block job starting for drive mirroring.
* Caller should hold @device and @target's aio context (must be the same).
**/
-static void blockdev_mirror_common(BlockDriverState *bs,
+static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
BlockDriverState *target,
bool has_replaces, const char *replaces,
enum MirrorSyncMode sync,
@@ -3482,15 +3482,15 @@ static void blockdev_mirror_common(BlockDriverState *bs,
/* pass the node name to replace to mirror start since it's loose coupling
* and will allow to check whether the node still exist at mirror completion
*/
- mirror_start(bs, target,
+ mirror_start(job_id, bs, target,
has_replaces ? replaces : NULL,
speed, granularity, buf_size, sync, backing_mode,
on_source_error, on_target_error, unmap,
block_job_cb, bs, errp);
}
-void qmp_drive_mirror(const char *device, const char *target,
- bool has_format, const char *format,
+void qmp_drive_mirror(bool has_job_id, const char *job_id, const char *device,
+ const char *target, bool has_format, const char *format,
bool has_node_name, const char *node_name,
bool has_replaces, const char *replaces,
enum MirrorSyncMode sync,
@@ -3634,7 +3634,7 @@ void qmp_drive_mirror(const char *device, const char *target,
bdrv_set_aio_context(target_bs, aio_context);
- blockdev_mirror_common(bs, target_bs,
+ blockdev_mirror_common(has_job_id ? job_id : NULL, bs, target_bs,
has_replaces, replaces, sync, backing_mode,
has_speed, speed,
has_granularity, granularity,
@@ -3649,7 +3649,8 @@ out:
aio_context_release(aio_context);
}
-void qmp_blockdev_mirror(const char *device, const char *target,
+void qmp_blockdev_mirror(bool has_job_id, const char *job_id,
+ const char *device, const char *target,
bool has_replaces, const char *replaces,
MirrorSyncMode sync,
bool has_speed, int64_t speed,
@@ -3690,7 +3691,7 @@ void qmp_blockdev_mirror(const char *device, const char *target,
bdrv_set_aio_context(target_bs, aio_context);
- blockdev_mirror_common(bs, target_bs,
+ blockdev_mirror_common(has_job_id ? job_id : NULL, bs, target_bs,
has_replaces, replaces, sync, backing_mode,
has_speed, speed,
has_granularity, granularity,
diff --git a/hmp.c b/hmp.c
index 0cf5baa..edf9c7d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1097,7 +1097,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
}
- qmp_drive_mirror(device, filename, !!format, format,
+ qmp_drive_mirror(false, NULL, device, filename, !!format, format,
false, NULL, false, NULL,
full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
true, mode, false, 0, false, 0, false, 0,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2a27a194..ad44fb9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -697,6 +697,8 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
void *opaque, Error **errp);
/*
* mirror_start:
+ * @job_id: The id of the newly-created job, or %NULL to use the
+ * device name of @bs.
* @bs: Block device to operate on.
* @target: Block device to write to.
* @replaces: Block graph node name to replace once the mirror is done. Can
@@ -718,8 +720,8 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
* manually completed. At the end of a successful mirroring job,
* @bs will be switched to read from @target.
*/
-void mirror_start(BlockDriverState *bs, BlockDriverState *target,
- const char *replaces,
+void mirror_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, const char *replaces,
int64_t speed, uint32_t granularity, int64_t buf_size,
MirrorSyncMode mode, BlockMirrorBackingMode backing_mode,
BlockdevOnError on_source_error,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ac8f5f6..18cdd5b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1108,6 +1108,9 @@
#
# Start mirroring a block device's writes to a new destination.
#
+# @job-id: #optional identifier for the newly-created block job. If
+# omitted, the device name will be used. (Since 2.7)
+#
# @device: the name of the device whose writes should be mirrored.
#
# @target: the target of the new image. If the file exists, or if it
@@ -1160,8 +1163,8 @@
# Since 1.3
##
{ 'command': 'drive-mirror',
- 'data': { 'device': 'str', 'target': 'str', '*format': 'str',
- '*node-name': 'str', '*replaces': 'str',
+ 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
+ '*format': 'str', '*node-name': 'str', '*replaces': 'str',
'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
'*speed': 'int', '*granularity': 'uint32',
'*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
@@ -1243,6 +1246,9 @@
#
# Start mirroring a block device's writes to a new destination.
#
+# @job-id: #optional identifier for the newly-created block job. If
+# omitted, the device name will be used. (Since 2.7)
+#
# @device: the name of the device whose writes should be mirrored.
#
# @target: the id or node-name of the block device to mirror to. This mustn't be
@@ -1279,7 +1285,7 @@
# Since 2.6
##
{ 'command': 'blockdev-mirror',
- 'data': { 'device': 'str', 'target': 'str',
+ 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
'*replaces': 'str',
'sync': 'MirrorSyncMode',
'*speed': 'int', '*granularity': 'uint32',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6937e83..6a2ec8f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1656,8 +1656,8 @@ EQMP
{
.name = "drive-mirror",
- .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
- "node-name:s?,replaces:s?,"
+ .args_type = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?,"
+ "format:s?,node-name:s?,replaces:s?,"
"on-source-error:s?,on-target-error:s?,"
"unmap:b?,"
"granularity:i?,buf-size:i?",
@@ -1677,6 +1677,8 @@ of the source.
Arguments:
+- "job-id": Identifier for the newly-created block job. If omitted,
+ the device name will be used. (json-string, optional)
- "device": device name to operate on (json-string)
- "target": name of new image file (json-string)
- "format": format of new image (json-string, optional)
@@ -1720,7 +1722,7 @@ EQMP
{
.name = "blockdev-mirror",
- .args_type = "sync:s,device:B,target:B,replaces:s?,speed:i?,"
+ .args_type = "job-id:s?,sync:s,device:B,target:B,replaces:s?,speed:i?,"
"on-source-error:s?,on-target-error:s?,"
"granularity:i?,buf-size:i?",
.mhandler.cmd_new = qmp_marshal_blockdev_mirror,
@@ -1735,6 +1737,8 @@ specifies the target of mirror operation.
Arguments:
+- "job-id": Identifier for the newly-created block job. If omitted,
+ the device name will be used. (json-string, optional)
- "device": device name to operate on (json-string)
- "target": device name to mirror to (json-string)
- "replaces": the block driver node name to replace when finished
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 07/34] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup'
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (5 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 06/34] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 08/34] stream: Add 'job-id' parameter to 'block-stream' Kevin Wolf
` (27 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
This patch adds a new optional 'job-id' parameter to 'blockdev-backup'
and 'drive-backup', allowing the user to specify the ID of the block
job to be created.
The HMP 'drive_backup' command remains unchanged.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/backup.c | 8 ++++----
blockdev.c | 43 ++++++++++++++++++++++++-------------------
hmp.c | 2 +-
include/block/block_int.h | 8 +++++---
qapi/block-core.json | 12 +++++++++---
qmp-commands.hx | 10 +++++++---
6 files changed, 50 insertions(+), 33 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 19c587c..dce6614 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -474,9 +474,9 @@ static void coroutine_fn backup_run(void *opaque)
block_job_defer_to_main_loop(&job->common, backup_complete, data);
}
-void backup_start(BlockDriverState *bs, BlockDriverState *target,
- int64_t speed, MirrorSyncMode sync_mode,
- BdrvDirtyBitmap *sync_bitmap,
+void backup_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, int64_t speed,
+ MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
@@ -541,7 +541,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
goto error;
}
- job = block_job_create(NULL, &backup_job_driver, bs, speed,
+ job = block_job_create(job_id, &backup_job_driver, bs, speed,
cb, opaque, errp);
if (!job) {
goto error;
diff --git a/blockdev.c b/blockdev.c
index 2008690..920d987 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1836,9 +1836,9 @@ typedef struct DriveBackupState {
BlockJob *job;
} DriveBackupState;
-static void do_drive_backup(const char *device, const char *target,
- bool has_format, const char *format,
- enum MirrorSyncMode sync,
+static void do_drive_backup(const char *job_id, const char *device,
+ const char *target, bool has_format,
+ const char *format, enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
bool has_speed, int64_t speed,
bool has_bitmap, const char *bitmap,
@@ -1876,7 +1876,8 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
bdrv_drained_begin(blk_bs(blk));
state->bs = blk_bs(blk);
- do_drive_backup(backup->device, backup->target,
+ do_drive_backup(backup->has_job_id ? backup->job_id : NULL,
+ backup->device, backup->target,
backup->has_format, backup->format,
backup->sync,
backup->has_mode, backup->mode,
@@ -1921,8 +1922,8 @@ typedef struct BlockdevBackupState {
AioContext *aio_context;
} BlockdevBackupState;
-static void do_blockdev_backup(const char *device, const char *target,
- enum MirrorSyncMode sync,
+static void do_blockdev_backup(const char *job_id, const char *device,
+ const char *target, enum MirrorSyncMode sync,
bool has_speed, int64_t speed,
bool has_on_source_error,
BlockdevOnError on_source_error,
@@ -1968,8 +1969,8 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
state->bs = blk_bs(blk);
bdrv_drained_begin(state->bs);
- do_blockdev_backup(backup->device, backup->target,
- backup->sync,
+ do_blockdev_backup(backup->has_job_id ? backup->job_id : NULL,
+ backup->device, backup->target, backup->sync,
backup->has_speed, backup->speed,
backup->has_on_source_error, backup->on_source_error,
backup->has_on_target_error, backup->on_target_error,
@@ -3182,9 +3183,9 @@ out:
aio_context_release(aio_context);
}
-static void do_drive_backup(const char *device, const char *target,
- bool has_format, const char *format,
- enum MirrorSyncMode sync,
+static void do_drive_backup(const char *job_id, const char *device,
+ const char *target, bool has_format,
+ const char *format, enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
bool has_speed, int64_t speed,
bool has_bitmap, const char *bitmap,
@@ -3303,7 +3304,7 @@ static void do_drive_backup(const char *device, const char *target,
}
}
- backup_start(bs, target_bs, speed, sync, bmap,
+ backup_start(job_id, bs, target_bs, speed, sync, bmap,
on_source_error, on_target_error,
block_job_cb, bs, txn, &local_err);
bdrv_unref(target_bs);
@@ -3316,7 +3317,8 @@ out:
aio_context_release(aio_context);
}
-void qmp_drive_backup(const char *device, const char *target,
+void qmp_drive_backup(bool has_job_id, const char *job_id,
+ const char *device, const char *target,
bool has_format, const char *format,
enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
@@ -3326,7 +3328,8 @@ void qmp_drive_backup(const char *device, const char *target,
bool has_on_target_error, BlockdevOnError on_target_error,
Error **errp)
{
- return do_drive_backup(device, target, has_format, format, sync,
+ return do_drive_backup(has_job_id ? job_id : NULL, device, target,
+ has_format, format, sync,
has_mode, mode, has_speed, speed,
has_bitmap, bitmap,
has_on_source_error, on_source_error,
@@ -3339,8 +3342,8 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
return bdrv_named_nodes_list(errp);
}
-void do_blockdev_backup(const char *device, const char *target,
- enum MirrorSyncMode sync,
+void do_blockdev_backup(const char *job_id, const char *device,
+ const char *target, enum MirrorSyncMode sync,
bool has_speed, int64_t speed,
bool has_on_source_error,
BlockdevOnError on_source_error,
@@ -3395,7 +3398,7 @@ void do_blockdev_backup(const char *device, const char *target,
goto out;
}
}
- backup_start(bs, target_bs, speed, sync, NULL, on_source_error,
+ backup_start(job_id, bs, target_bs, speed, sync, NULL, on_source_error,
on_target_error, block_job_cb, bs, txn, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
@@ -3404,7 +3407,8 @@ out:
aio_context_release(aio_context);
}
-void qmp_blockdev_backup(const char *device, const char *target,
+void qmp_blockdev_backup(bool has_job_id, const char *job_id,
+ const char *device, const char *target,
enum MirrorSyncMode sync,
bool has_speed, int64_t speed,
bool has_on_source_error,
@@ -3413,7 +3417,8 @@ void qmp_blockdev_backup(const char *device, const char *target,
BlockdevOnError on_target_error,
Error **errp)
{
- do_blockdev_backup(device, target, sync, has_speed, speed,
+ do_blockdev_backup(has_job_id ? job_id : NULL, device, target,
+ sync, has_speed, speed,
has_on_source_error, on_source_error,
has_on_target_error, on_target_error,
NULL, errp);
diff --git a/hmp.c b/hmp.c
index edf9c7d..62eca70 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1127,7 +1127,7 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
}
- qmp_drive_backup(device, filename, !!format, format,
+ qmp_drive_backup(false, NULL, device, filename, !!format, format,
full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
true, mode, false, 0, false, NULL,
false, 0, false, 0, &err);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ad44fb9..a0b9f92 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -732,6 +732,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
/*
* backup_start:
+ * @job_id: The id of the newly-created job, or %NULL to use the
+ * device name of @bs.
* @bs: Block device to operate on.
* @target: Block device to write to.
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
@@ -746,9 +748,9 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
* Start a backup operation on @bs. Clusters in @bs are written to @target
* until the job is cancelled or manually completed.
*/
-void backup_start(BlockDriverState *bs, BlockDriverState *target,
- int64_t speed, MirrorSyncMode sync_mode,
- BdrvDirtyBitmap *sync_bitmap,
+void backup_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, int64_t speed,
+ MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 18cdd5b..ee44ce4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -866,6 +866,9 @@
##
# @DriveBackup
#
+# @job-id: #optional identifier for the newly-created block job. If
+# omitted, the device name will be used. (Since 2.7)
+#
# @device: the name of the device which should be copied.
#
# @target: the target of the new image. If the file exists, or if it
@@ -903,8 +906,8 @@
# Since: 1.6
##
{ 'struct': 'DriveBackup',
- 'data': { 'device': 'str', 'target': 'str', '*format': 'str',
- 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
+ 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
+ '*format': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
'*speed': 'int', '*bitmap': 'str',
'*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError' } }
@@ -912,6 +915,9 @@
##
# @BlockdevBackup
#
+# @job-id: #optional identifier for the newly-created block job. If
+# omitted, the device name will be used. (Since 2.7)
+#
# @device: the name of the device which should be copied.
#
# @target: the name of the backup target device.
@@ -938,7 +944,7 @@
# Since: 2.3
##
{ 'struct': 'BlockdevBackup',
- 'data': { 'device': 'str', 'target': 'str',
+ 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
'sync': 'MirrorSyncMode',
'*speed': 'int',
'*on-source-error': 'BlockdevOnError',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6a2ec8f..a032089 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1212,8 +1212,8 @@ EQMP
{
.name = "drive-backup",
- .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
- "bitmap:s?,on-source-error:s?,on-target-error:s?",
+ .args_type = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?,"
+ "format:s?,bitmap:s?,on-source-error:s?,on-target-error:s?",
.mhandler.cmd_new = qmp_marshal_drive_backup,
},
@@ -1229,6 +1229,8 @@ block-job-cancel command.
Arguments:
+- "job-id": Identifier for the newly-created block job. If omitted,
+ the device name will be used. (json-string, optional)
- "device": the name of the device which should be copied.
(json-string)
- "target": the target of the new image. If the file exists, or if it is a
@@ -1266,7 +1268,7 @@ EQMP
{
.name = "blockdev-backup",
- .args_type = "sync:s,device:B,target:B,speed:i?,"
+ .args_type = "job-id:s?,sync:s,device:B,target:B,speed:i?,"
"on-source-error:s?,on-target-error:s?",
.mhandler.cmd_new = qmp_marshal_blockdev_backup,
},
@@ -1280,6 +1282,8 @@ as backup target.
Arguments:
+- "job-id": Identifier for the newly-created block job. If omitted,
+ the device name will be used. (json-string, optional)
- "device": the name of the device which should be copied.
(json-string)
- "target": the name of the backup target device. (json-string)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 08/34] stream: Add 'job-id' parameter to 'block-stream'
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (6 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 07/34] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup' Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 09/34] commit: Add 'job-id' parameter to 'block-commit' Kevin Wolf
` (26 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
This patch adds a new optional 'job-id' parameter to 'block-stream',
allowing the user to specify the ID of the block job to be created.
The HMP 'block_stream' command remains unchanged.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/stream.c | 12 ++++++------
blockdev.c | 6 +++---
hmp.c | 2 +-
include/block/block_int.h | 10 ++++++----
qapi/block-core.json | 8 ++++++--
qmp-commands.hx | 4 +++-
6 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/block/stream.c b/block/stream.c
index e4319d3..54c8cd8 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -218,15 +218,15 @@ static const BlockJobDriver stream_job_driver = {
.set_speed = stream_set_speed,
};
-void stream_start(BlockDriverState *bs, BlockDriverState *base,
- const char *backing_file_str, int64_t speed,
- BlockdevOnError on_error,
- BlockCompletionFunc *cb,
- void *opaque, Error **errp)
+void stream_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, const char *backing_file_str,
+ int64_t speed, BlockdevOnError on_error,
+ BlockCompletionFunc *cb, void *opaque, Error **errp)
{
StreamBlockJob *s;
- s = block_job_create(NULL, &stream_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(job_id, &stream_job_driver, bs, speed,
+ cb, opaque, errp);
if (!s) {
return;
}
diff --git a/blockdev.c b/blockdev.c
index 920d987..d6f1d4d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3005,7 +3005,7 @@ static void block_job_cb(void *opaque, int ret)
}
}
-void qmp_block_stream(const char *device,
+void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
bool has_base, const char *base,
bool has_backing_file, const char *backing_file,
bool has_speed, int64_t speed,
@@ -3064,8 +3064,8 @@ void qmp_block_stream(const char *device,
/* backing_file string overrides base bs filename */
base_name = has_backing_file ? backing_file : base_name;
- stream_start(bs, base_bs, base_name, has_speed ? speed : 0,
- on_error, block_job_cb, bs, &local_err);
+ stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name,
+ has_speed ? speed : 0, on_error, block_job_cb, bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
diff --git a/hmp.c b/hmp.c
index 62eca70..3ca79c3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1485,7 +1485,7 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
const char *base = qdict_get_try_str(qdict, "base");
int64_t speed = qdict_get_try_int(qdict, "speed", 0);
- qmp_block_stream(device, base != NULL, base, false, NULL,
+ qmp_block_stream(false, NULL, device, base != NULL, base, false, NULL,
qdict_haskey(qdict, "speed"), speed,
true, BLOCKDEV_ON_ERROR_REPORT, &error);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a0b9f92..db364bb 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -639,6 +639,8 @@ int is_windows_drive(const char *filename);
/**
* stream_start:
+ * @job_id: The id of the newly-created job, or %NULL to use the
+ * device name of @bs.
* @bs: Block device to operate on.
* @base: Block device that will become the new base, or %NULL to
* flatten the whole backing file chain onto @bs.
@@ -657,10 +659,10 @@ int is_windows_drive(const char *filename);
* @backing_file_str in the written image and to @base in the live
* BlockDriverState.
*/
-void stream_start(BlockDriverState *bs, BlockDriverState *base,
- const char *backing_file_str, int64_t speed,
- BlockdevOnError on_error, BlockCompletionFunc *cb,
- void *opaque, Error **errp);
+void stream_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, const char *backing_file_str,
+ int64_t speed, BlockdevOnError on_error,
+ BlockCompletionFunc *cb, void *opaque, Error **errp);
/**
* commit_start:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ee44ce4..94f4733 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1425,6 +1425,9 @@
# On successful completion the image file is updated to drop the backing file
# and the BLOCK_JOB_COMPLETED event is emitted.
#
+# @job-id: #optional identifier for the newly-created block job. If
+# omitted, the device name will be used. (Since 2.7)
+#
# @device: the device name
#
# @base: #optional the common backing file name
@@ -1456,8 +1459,9 @@
# Since: 1.1
##
{ 'command': 'block-stream',
- 'data': { 'device': 'str', '*base': 'str', '*backing-file': 'str',
- '*speed': 'int', '*on-error': 'BlockdevOnError' } }
+ 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
+ '*backing-file': 'str', '*speed': 'int',
+ '*on-error': 'BlockdevOnError' } }
##
# @block-job-set-speed:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a032089..d61ea20 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1106,7 +1106,7 @@ EQMP
{
.name = "block-stream",
- .args_type = "device:B,base:s?,speed:o?,backing-file:s?,on-error:s?",
+ .args_type = "job-id:s?,device:B,base:s?,speed:o?,backing-file:s?,on-error:s?",
.mhandler.cmd_new = qmp_marshal_block_stream,
},
@@ -1118,6 +1118,8 @@ Copy data from a backing file into a block device.
Arguments:
+- "job-id": Identifier for the newly-created block job. If omitted,
+ the device name will be used. (json-string, optional)
- "device": The device's ID, must be unique (json-string)
- "base": The file name of the backing image above which copying starts
(json-string, optional)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 09/34] commit: Add 'job-id' parameter to 'block-commit'
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (7 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 08/34] stream: Add 'job-id' parameter to 'block-stream' Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 10/34] qemu-img: Set the ID of the block job in img_commit() Kevin Wolf
` (25 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
This patch adds a new optional 'job-id' parameter to 'block-commit',
allowing the user to specify the ID of the block job to be created.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/commit.c | 7 ++++---
block/mirror.c | 6 +++---
blockdev.c | 9 +++++----
include/block/block_int.h | 16 ++++++++++------
qapi/block-core.json | 5 ++++-
qemu-img.c | 2 +-
qmp-commands.hx | 4 +++-
7 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 137bb03..23368fa 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -211,8 +211,8 @@ static const BlockJobDriver commit_job_driver = {
.set_speed = commit_set_speed,
};
-void commit_start(BlockDriverState *bs, BlockDriverState *base,
- BlockDriverState *top, int64_t speed,
+void commit_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, BlockDriverState *top, int64_t speed,
BlockdevOnError on_error, BlockCompletionFunc *cb,
void *opaque, const char *backing_file_str, Error **errp)
{
@@ -236,7 +236,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
return;
}
- s = block_job_create(NULL, &commit_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(job_id, &commit_job_driver, bs, speed,
+ cb, opaque, errp);
if (!s) {
return;
}
diff --git a/block/mirror.c b/block/mirror.c
index 702a686..705fbc0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -931,8 +931,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
&mirror_job_driver, is_none_mode, base);
}
-void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
- int64_t speed,
+void commit_active_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, int64_t speed,
BlockdevOnError on_error,
BlockCompletionFunc *cb,
void *opaque, Error **errp)
@@ -973,7 +973,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
}
}
- mirror_start_job(NULL, bs, base, NULL, speed, 0, 0,
+ mirror_start_job(job_id, bs, base, NULL, speed, 0, 0,
MIRROR_LEAVE_BACKING_CHAIN,
on_error, on_error, false, cb, opaque, &local_err,
&commit_active_job_driver, false, base);
diff --git a/blockdev.c b/blockdev.c
index d6f1d4d..aa23dc2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3077,7 +3077,7 @@ out:
aio_context_release(aio_context);
}
-void qmp_block_commit(const char *device,
+void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,
bool has_base, const char *base,
bool has_top, const char *top,
bool has_backing_file, const char *backing_file,
@@ -3168,10 +3168,11 @@ void qmp_block_commit(const char *device,
" but 'top' is the active layer");
goto out;
}
- commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
- bs, &local_err);
+ commit_active_start(has_job_id ? job_id : NULL, bs, base_bs, speed,
+ on_error, block_job_cb, bs, &local_err);
} else {
- commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
+ commit_start(has_job_id ? job_id : NULL, bs, base_bs, top_bs, speed,
+ on_error, block_job_cb, bs,
has_backing_file ? backing_file : NULL, &local_err);
}
if (local_err != NULL) {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index db364bb..8054146 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -666,6 +666,8 @@ void stream_start(const char *job_id, BlockDriverState *bs,
/**
* commit_start:
+ * @job_id: The id of the newly-created job, or %NULL to use the
+ * device name of @bs.
* @bs: Active block device.
* @top: Top block device to be committed.
* @base: Block device that will be written into, and become the new top.
@@ -677,12 +679,14 @@ void stream_start(const char *job_id, BlockDriverState *bs,
* @errp: Error object.
*
*/
-void commit_start(BlockDriverState *bs, BlockDriverState *base,
- BlockDriverState *top, int64_t speed,
- BlockdevOnError on_error, BlockCompletionFunc *cb,
- void *opaque, const char *backing_file_str, Error **errp);
+void commit_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, BlockDriverState *top, int64_t speed,
+ BlockdevOnError on_error, BlockCompletionFunc *cb,
+ void *opaque, const char *backing_file_str, Error **errp);
/**
* commit_active_start:
+ * @job_id: The id of the newly-created job, or %NULL to use the
+ * device name of @bs.
* @bs: Active block device to be committed.
* @base: Block device that will be written into, and become the new top.
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
@@ -692,8 +696,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
* @errp: Error object.
*
*/
-void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
- int64_t speed,
+void commit_active_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, int64_t speed,
BlockdevOnError on_error,
BlockCompletionFunc *cb,
void *opaque, Error **errp);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 94f4733..1e4b16d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1010,6 +1010,9 @@
# Live commit of data from overlay image nodes into backing nodes - i.e.,
# writes data between 'top' and 'base' into 'base'.
#
+# @job-id: #optional identifier for the newly-created block job. If
+# omitted, the device name will be used. (Since 2.7)
+#
# @device: the name of the device
#
# @base: #optional The file name of the backing image to write data into.
@@ -1061,7 +1064,7 @@
#
##
{ 'command': 'block-commit',
- 'data': { 'device': 'str', '*base': 'str', '*top': 'str',
+ 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str', '*top': 'str',
'*backing-file': 'str', '*speed': 'int' } }
##
diff --git a/qemu-img.c b/qemu-img.c
index ea5970b..a162f34 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -920,7 +920,7 @@ static int img_commit(int argc, char **argv)
.bs = bs,
};
- commit_active_start(bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
+ commit_active_start(NULL, bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
common_block_job_cb, &cbi, &local_err);
if (local_err) {
goto done;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d61ea20..c46c65c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1151,7 +1151,7 @@ EQMP
{
.name = "block-commit",
- .args_type = "device:B,base:s?,top:s?,backing-file:s?,speed:o?",
+ .args_type = "job-id:s?,device:B,base:s?,top:s?,backing-file:s?,speed:o?",
.mhandler.cmd_new = qmp_marshal_block_commit,
},
@@ -1164,6 +1164,8 @@ data between 'top' and 'base' into 'base'.
Arguments:
+- "job-id": Identifier for the newly-created block job. If omitted,
+ the device name will be used. (json-string, optional)
- "device": The device's ID, must be unique (json-string)
- "base": The file name of the backing image to write data into.
If not specified, this is the deepest backing image
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 10/34] qemu-img: Set the ID of the block job in img_commit()
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (8 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 09/34] commit: Add 'job-id' parameter to 'block-commit' Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 11/34] blockjob: Update description of the 'device' field in the QMP API Kevin Wolf
` (24 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
img_commit() creates a block job without an ID. This is no longer
allowed now that we require it to be unique and well-formed. We were
solving this by having a fallback in block_job_create(), but now that
we extended the API of commit_active_start() we can finally set an
explicit ID and revert that change.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockjob.c | 6 ------
qemu-img.c | 2 +-
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index 511c0db..3b9cec7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -132,12 +132,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
if (job_id == NULL) {
job_id = bdrv_get_device_name(bs);
- /* Assign a default ID if the BDS does not have a device
- * name. We'll get rid of this soon when we finish extending
- * the API of all commands that create block jobs. */
- if (job_id[0] == '\0') {
- job_id = "default_job";
- }
}
if (!id_wellformed(job_id)) {
diff --git a/qemu-img.c b/qemu-img.c
index a162f34..969edce 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -920,7 +920,7 @@ static int img_commit(int argc, char **argv)
.bs = bs,
};
- commit_active_start(NULL, bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
+ commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
common_block_job_cb, &cbi, &local_err);
if (local_err) {
goto done;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 11/34] blockjob: Update description of the 'device' field in the QMP API
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (9 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 10/34] qemu-img: Set the ID of the block job in img_commit() Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-14 19:30 ` Eric Blake
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 12/34] osdep: Introduce qemu_dup Kevin Wolf
` (23 subsequent siblings)
34 siblings, 1 reply; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
The 'device' field in all BLOCK_JOB_* events and 'block-job-*' command
is no longer the device name, but the ID of the job. This patch
updates the documentation to clarify that.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
docs/qmp-events.txt | 12 ++++++++----
qapi/block-core.json | 35 +++++++++++++++++++++++++----------
2 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index fa7574d..7967ec4 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -92,7 +92,8 @@ Data:
- "type": Job type (json-string; "stream" for image streaming
"commit" for block commit)
-- "device": Device name (json-string)
+- "device": Job identifier. Originally the device name but other
+ values are allowed since QEMU 2.7 (json-string)
- "len": Maximum progress value (json-int)
- "offset": Current progress value (json-int)
On success this is equal to len.
@@ -116,7 +117,8 @@ Data:
- "type": Job type (json-string; "stream" for image streaming
"commit" for block commit)
-- "device": Device name (json-string)
+- "device": Job identifier. Originally the device name but other
+ values are allowed since QEMU 2.7 (json-string)
- "len": Maximum progress value (json-int)
- "offset": Current progress value (json-int)
On success this is equal to len.
@@ -143,7 +145,8 @@ Emitted when a block job encounters an error.
Data:
-- "device": device name (json-string)
+- "device": Job identifier. Originally the device name but other
+ values are allowed since QEMU 2.7 (json-string)
- "operation": I/O operation (json-string, "read" or "write")
- "action": action that has been taken, it's one of the following (json-string):
"ignore": error has been ignored, the job may fail later
@@ -167,7 +170,8 @@ Data:
- "type": Job type (json-string; "stream" for image streaming
"commit" for block commit)
-- "device": Device name (json-string)
+- "device": Job identifier. Originally the device name but other
+ values are allowed since QEMU 2.7 (json-string)
- "len": Maximum progress value (json-int)
- "offset": Current progress value (json-int)
On success this is equal to len.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1e4b16d..3f77dac 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -713,7 +713,8 @@
#
# @type: the job type ('stream' for image streaming)
#
-# @device: the block device name
+# @device: The job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @len: the maximum progress value
#
@@ -1475,7 +1476,9 @@
#
# Throttling can be disabled by setting the speed to 0.
#
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# @speed: the maximum speed, in bytes per second, or 0 for unlimited.
# Defaults to 0.
@@ -1506,7 +1509,9 @@
# operation can be started at a later time to finish copying all data from the
# backing file.
#
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# @force: #optional whether to allow cancellation of a paused job (default
# false). Since 1.3.
@@ -1532,7 +1537,9 @@
# the operation is actually paused. Cancelling a paused job automatically
# resumes it.
#
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# Returns: Nothing on success
# If no background operation is active on this device, DeviceNotActive
@@ -1552,7 +1559,9 @@
#
# This command also clears the error status of the job.
#
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# Returns: Nothing on success
# If no background operation is active on this device, DeviceNotActive
@@ -1578,7 +1587,9 @@
#
# A cancelled or paused job cannot be completed.
#
-# @device: the device name
+# @device: The job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# Returns: Nothing on success
# If no background operation is active on this device, DeviceNotActive
@@ -2424,7 +2435,8 @@
#
# @type: job type
#
-# @device: device name
+# @device: The job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @len: maximum progress value
#
@@ -2455,7 +2467,8 @@
#
# @type: job type
#
-# @device: device name
+# @device: The job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @len: maximum progress value
#
@@ -2478,7 +2491,8 @@
#
# Emitted when a block job encounters an error
#
-# @device: device name
+# @device: The job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @operation: I/O operation
#
@@ -2498,7 +2512,8 @@
#
# @type: job type
#
-# @device: device name
+# @device: The job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @len: maximum progress value
#
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PULL v2 11/34] blockjob: Update description of the 'device' field in the QMP API
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 11/34] blockjob: Update description of the 'device' field in the QMP API Kevin Wolf
@ 2016-07-14 19:30 ` Eric Blake
0 siblings, 0 replies; 37+ messages in thread
From: Eric Blake @ 2016-07-14 19:30 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]
On 07/13/2016 06:50 AM, Kevin Wolf wrote:
> From: Alberto Garcia <berto@igalia.com>
>
> The 'device' field in all BLOCK_JOB_* events and 'block-job-*' command
> is no longer the device name, but the ID of the job. This patch
> updates the documentation to clarify that.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> docs/qmp-events.txt | 12 ++++++++----
> qapi/block-core.json | 35 +++++++++++++++++++++++++----------
> 2 files changed, 33 insertions(+), 14 deletions(-)
>
> +++ b/qapi/block-core.json
> @@ -713,7 +713,8 @@
> #
> # @type: the job type ('stream' for image streaming)
> #
> -# @device: the block device name
> +# @device: The job identifier. Originally the device name but other
> +# values are allowed since QEMU 2.7
> #
> # @len: the maximum progress value
> #
> @@ -1475,7 +1476,9 @@
> #
> # Throttling can be disabled by setting the speed to 0.
> #
> -# @device: the device name
> +# @device: The job identifier. This used to be a device name (hence
> +# the name of the parameter), but since QEMU 2.7 it can have
> +# other values.
It might have been nice to use the same wording everywhere, instead of
writing two different things, but it's not the end of the world, and I'm
fine if we keep it as committed.
--
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] 37+ messages in thread
* [Qemu-devel] [PULL v2 12/34] osdep: Introduce qemu_dup
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (10 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 11/34] blockjob: Update description of the 'device' field in the QMP API Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 13/34] raw-posix: Use qemu_dup Kevin Wolf
` (22 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Fam Zheng <famz@redhat.com>
And use it in qemu_dup_flags.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/qemu/osdep.h | 3 +++
util/osdep.c | 23 +++++++++++++++--------
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index e63da28..7361006 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -278,6 +278,9 @@ int qemu_madvise(void *addr, size_t len, int advice);
int qemu_open(const char *name, int flags, ...);
int qemu_close(int fd);
+#ifndef _WIN32
+int qemu_dup(int fd);
+#endif
#if defined(__HAIKU__) && defined(__i386__)
#define FMT_pid "%ld"
diff --git a/util/osdep.c b/util/osdep.c
index ff004e8..06fb1cf 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -83,14 +83,7 @@ static int qemu_dup_flags(int fd, int flags)
int serrno;
int dup_flags;
-#ifdef F_DUPFD_CLOEXEC
- ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
-#else
- ret = dup(fd);
- if (ret != -1) {
- qemu_set_cloexec(ret);
- }
-#endif
+ ret = qemu_dup(fd);
if (ret == -1) {
goto fail;
}
@@ -129,6 +122,20 @@ fail:
return -1;
}
+int qemu_dup(int fd)
+{
+ int ret;
+#ifdef F_DUPFD_CLOEXEC
+ ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
+#else
+ ret = dup(fd);
+ if (ret != -1) {
+ qemu_set_cloexec(ret);
+ }
+#endif
+ return ret;
+}
+
static int qemu_parse_fdset(const char *param)
{
return qemu_parse_fd(param);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 13/34] raw-posix: Use qemu_dup
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (11 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 12/34] osdep: Introduce qemu_dup Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 14/34] coroutine: use QSIMPLEQ instead of QTAILQ Kevin Wolf
` (21 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Fam Zheng <famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/raw-posix.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index c979ac3..d1c3bd8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -639,15 +639,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
/* dup the original fd */
- /* TODO: use qemu fcntl wrapper */
-#ifdef F_DUPFD_CLOEXEC
- raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0);
-#else
- raw_s->fd = dup(s->fd);
- if (raw_s->fd != -1) {
- qemu_set_cloexec(raw_s->fd);
- }
-#endif
+ raw_s->fd = qemu_dup(s->fd);
if (raw_s->fd >= 0) {
ret = fcntl_setfl(raw_s->fd, raw_s->open_flags);
if (ret) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 14/34] coroutine: use QSIMPLEQ instead of QTAILQ
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (12 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 13/34] raw-posix: Use qemu_dup Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 15/34] test-coroutine: prepare for the next patch Kevin Wolf
` (20 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
CoQueue do not need to remove any element but the head of the list;
processing is always strictly FIFO. Therefore, the simpler singly-linked
QSIMPLEQ can be used instead.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/qemu/coroutine.h | 2 +-
include/qemu/coroutine_int.h | 4 ++--
util/qemu-coroutine-lock.c | 22 +++++++++++-----------
util/qemu-coroutine.c | 2 +-
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 305fe76..63ae7fe 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -102,7 +102,7 @@ bool qemu_in_coroutine(void);
* are built.
*/
typedef struct CoQueue {
- QTAILQ_HEAD(, Coroutine) entries;
+ QSIMPLEQ_HEAD(, Coroutine) entries;
} CoQueue;
/**
diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index 42d6838..581a7f5 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -41,8 +41,8 @@ struct Coroutine {
QSLIST_ENTRY(Coroutine) pool_next;
/* Coroutines that should be woken up when we yield or terminate */
- QTAILQ_HEAD(, Coroutine) co_queue_wakeup;
- QTAILQ_ENTRY(Coroutine) co_queue_next;
+ QSIMPLEQ_HEAD(, Coroutine) co_queue_wakeup;
+ QSIMPLEQ_ENTRY(Coroutine) co_queue_next;
};
Coroutine *qemu_coroutine_new(void);
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index da37ca7..cf53693 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -31,13 +31,13 @@
void qemu_co_queue_init(CoQueue *queue)
{
- QTAILQ_INIT(&queue->entries);
+ QSIMPLEQ_INIT(&queue->entries);
}
void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
{
Coroutine *self = qemu_coroutine_self();
- QTAILQ_INSERT_TAIL(&queue->entries, self, co_queue_next);
+ QSIMPLEQ_INSERT_TAIL(&queue->entries, self, co_queue_next);
qemu_coroutine_yield();
assert(qemu_in_coroutine());
}
@@ -55,8 +55,8 @@ void qemu_co_queue_run_restart(Coroutine *co)
Coroutine *next;
trace_qemu_co_queue_run_restart(co);
- while ((next = QTAILQ_FIRST(&co->co_queue_wakeup))) {
- QTAILQ_REMOVE(&co->co_queue_wakeup, next, co_queue_next);
+ while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) {
+ QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next);
qemu_coroutine_enter(next, NULL);
}
}
@@ -66,13 +66,13 @@ static bool qemu_co_queue_do_restart(CoQueue *queue, bool single)
Coroutine *self = qemu_coroutine_self();
Coroutine *next;
- if (QTAILQ_EMPTY(&queue->entries)) {
+ if (QSIMPLEQ_EMPTY(&queue->entries)) {
return false;
}
- while ((next = QTAILQ_FIRST(&queue->entries)) != NULL) {
- QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
- QTAILQ_INSERT_TAIL(&self->co_queue_wakeup, next, co_queue_next);
+ while ((next = QSIMPLEQ_FIRST(&queue->entries)) != NULL) {
+ QSIMPLEQ_REMOVE_HEAD(&queue->entries, co_queue_next);
+ QSIMPLEQ_INSERT_TAIL(&self->co_queue_wakeup, next, co_queue_next);
trace_qemu_co_queue_next(next);
if (single) {
break;
@@ -97,19 +97,19 @@ bool qemu_co_enter_next(CoQueue *queue)
{
Coroutine *next;
- next = QTAILQ_FIRST(&queue->entries);
+ next = QSIMPLEQ_FIRST(&queue->entries);
if (!next) {
return false;
}
- QTAILQ_REMOVE(&queue->entries, next, co_queue_next);
+ QSIMPLEQ_REMOVE_HEAD(&queue->entries, co_queue_next);
qemu_coroutine_enter(next, NULL);
return true;
}
bool qemu_co_queue_empty(CoQueue *queue)
{
- return QTAILQ_FIRST(&queue->entries) == NULL;
+ return QSIMPLEQ_FIRST(&queue->entries) == NULL;
}
void qemu_co_mutex_init(CoMutex *mutex)
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index 5816702..b7cb636 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -76,7 +76,7 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
}
co->entry = entry;
- QTAILQ_INIT(&co->co_queue_wakeup);
+ QSIMPLEQ_INIT(&co->co_queue_wakeup);
return co;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 15/34] test-coroutine: prepare for the next patch
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (13 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 14/34] coroutine: use QSIMPLEQ instead of QTAILQ Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 16/34] coroutine: move entry argument to qemu_coroutine_create Kevin Wolf
` (19 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
The next patch moves the coroutine argument from first-enter to
creation time. In this case, coroutine has not been initialized
yet when the coroutine is created, so change to a pointer.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/test-coroutine.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c
index 215b92e..5171174 100644
--- a/tests/test-coroutine.c
+++ b/tests/test-coroutine.c
@@ -40,7 +40,8 @@ static void test_in_coroutine(void)
static void coroutine_fn verify_self(void *opaque)
{
- g_assert(qemu_coroutine_self() == opaque);
+ Coroutine **p_co = opaque;
+ g_assert(qemu_coroutine_self() == *p_co);
}
static void test_self(void)
@@ -48,7 +49,7 @@ static void test_self(void)
Coroutine *coroutine;
coroutine = qemu_coroutine_create(verify_self);
- qemu_coroutine_enter(coroutine, coroutine);
+ qemu_coroutine_enter(coroutine, &coroutine);
}
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 16/34] coroutine: move entry argument to qemu_coroutine_create
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (14 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 15/34] test-coroutine: prepare for the next patch Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 17/34] block/qdev: Allow node name for drive properties Kevin Wolf
` (18 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
In practice the entry argument is always known at creation time, and
it is confusing that sometimes qemu_coroutine_enter is used with a
non-NULL argument to re-enter a coroutine (this happens in
block/sheepdog.c and tests/test-coroutine.c). So pass the opaque value
at creation time, for consistency with e.g. aio_bh_new.
Mostly done with the following semantic patch:
@ entry1 @
expression entry, arg, co;
@@
- co = qemu_coroutine_create(entry);
+ co = qemu_coroutine_create(entry, arg);
...
- qemu_coroutine_enter(co, arg);
+ qemu_coroutine_enter(co);
@ entry2 @
expression entry, arg;
identifier co;
@@
- Coroutine *co = qemu_coroutine_create(entry);
+ Coroutine *co = qemu_coroutine_create(entry, arg);
...
- qemu_coroutine_enter(co, arg);
+ qemu_coroutine_enter(co);
@ entry3 @
expression entry, arg;
@@
- qemu_coroutine_enter(qemu_coroutine_create(entry), arg);
+ qemu_coroutine_enter(qemu_coroutine_create(entry, arg));
@ reentry @
expression co;
@@
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);
except for the aforementioned few places where the semantic patch
stumbled (as expected) and for test_co_queue, which would otherwise
produce an uninitialized variable warning.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 4 +--
block/backup.c | 4 +--
block/blkdebug.c | 4 +--
block/blkreplay.c | 2 +-
block/block-backend.c | 8 +++---
block/commit.c | 4 +--
block/gluster.c | 2 +-
block/io.c | 45 ++++++++++++++++----------------
block/iscsi.c | 4 +--
block/linux-aio.c | 2 +-
block/mirror.c | 6 ++---
block/nbd-client.c | 6 ++---
block/nfs.c | 2 +-
block/qcow.c | 4 +--
block/qcow2.c | 4 +--
block/qed.c | 4 +--
block/sheepdog.c | 14 +++++-----
block/ssh.c | 2 +-
block/stream.c | 4 +--
block/vmdk.c | 4 +--
blockjob.c | 2 +-
hw/9pfs/9p.c | 4 +--
hw/9pfs/coth.c | 4 +--
include/qemu/coroutine.h | 8 +++---
include/qemu/main-loop.h | 4 +--
io/channel.c | 2 +-
migration/migration.c | 4 +--
nbd/server.c | 12 ++++-----
qemu-io-cmds.c | 4 +--
tests/test-blockjob-txn.c | 4 +--
tests/test-coroutine.c | 62 ++++++++++++++++++++++-----------------------
tests/test-thread-pool.c | 4 +--
thread-pool.c | 2 +-
util/qemu-coroutine-io.c | 2 +-
util/qemu-coroutine-lock.c | 4 +--
util/qemu-coroutine-sleep.c | 2 +-
util/qemu-coroutine.c | 8 +++---
37 files changed, 130 insertions(+), 131 deletions(-)
diff --git a/block.c b/block.c
index 823ff1d..67894e0 100644
--- a/block.c
+++ b/block.c
@@ -329,8 +329,8 @@ int bdrv_create(BlockDriver *drv, const char* filename,
/* Fast-path if already in coroutine context */
bdrv_create_co_entry(&cco);
} else {
- co = qemu_coroutine_create(bdrv_create_co_entry);
- qemu_coroutine_enter(co, &cco);
+ co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
+ qemu_coroutine_enter(co);
while (cco.ret == NOT_DONE) {
aio_poll(qemu_get_aio_context(), true);
}
diff --git a/block/backup.c b/block/backup.c
index dce6614..2c05323 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -576,9 +576,9 @@ void backup_start(const char *job_id, BlockDriverState *bs,
bdrv_op_block_all(target, job->common.blocker);
job->common.len = len;
- job->common.co = qemu_coroutine_create(backup_run);
+ job->common.co = qemu_coroutine_create(backup_run, job);
block_job_txn_add_job(txn, &job->common);
- qemu_coroutine_enter(job->common.co, job);
+ qemu_coroutine_enter(job->common.co);
return;
error:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index bbaa33f..fb29283 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -621,7 +621,7 @@ static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag)
QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, next) {
if (!strcmp(r->tag, tag)) {
- qemu_coroutine_enter(r->co, NULL);
+ qemu_coroutine_enter(r->co);
return 0;
}
}
@@ -647,7 +647,7 @@ static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
}
QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, r_next) {
if (!strcmp(r->tag, tag)) {
- qemu_coroutine_enter(r->co, NULL);
+ qemu_coroutine_enter(r->co);
ret = 0;
}
}
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 70650e4..3368c8c 100755
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -65,7 +65,7 @@ static int64_t blkreplay_getlength(BlockDriverState *bs)
static void blkreplay_bh_cb(void *opaque)
{
Request *req = opaque;
- qemu_coroutine_enter(req->co, NULL);
+ qemu_coroutine_enter(req->co);
qemu_bh_delete(req->bh);
g_free(req);
}
diff --git a/block/block-backend.c b/block/block-backend.c
index a862f65..0d7b801 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -836,8 +836,8 @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
.ret = NOT_DONE,
};
- co = qemu_coroutine_create(co_entry);
- qemu_coroutine_enter(co, &rwco);
+ co = qemu_coroutine_create(co_entry, &rwco);
+ qemu_coroutine_enter(co);
aio_context = blk_get_aio_context(blk);
while (rwco.ret == NOT_DONE) {
@@ -950,8 +950,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
acb->bh = NULL;
acb->has_returned = false;
- co = qemu_coroutine_create(co_entry);
- qemu_coroutine_enter(co, acb);
+ co = qemu_coroutine_create(co_entry, acb);
+ qemu_coroutine_enter(co);
acb->has_returned = true;
if (acb->rwco.ret != NOT_DONE) {
diff --git a/block/commit.c b/block/commit.c
index 23368fa..8b534d7 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -278,10 +278,10 @@ void commit_start(const char *job_id, BlockDriverState *bs,
s->backing_file_str = g_strdup(backing_file_str);
s->on_error = on_error;
- s->common.co = qemu_coroutine_create(commit_run);
+ s->common.co = qemu_coroutine_create(commit_run, s);
trace_commit_start(bs, base, top, s, s->common.co, opaque);
- qemu_coroutine_enter(s->common.co, s);
+ qemu_coroutine_enter(s->common.co);
}
diff --git a/block/gluster.c b/block/gluster.c
index 16f7778..406c1e6 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -233,7 +233,7 @@ static void qemu_gluster_complete_aio(void *opaque)
qemu_bh_delete(acb->bh);
acb->bh = NULL;
- qemu_coroutine_enter(acb->coroutine, NULL);
+ qemu_coroutine_enter(acb->coroutine);
}
/*
diff --git a/block/io.c b/block/io.c
index 7086908..2887394 100644
--- a/block/io.c
+++ b/block/io.c
@@ -195,7 +195,7 @@ static void bdrv_co_drain_bh_cb(void *opaque)
qemu_bh_delete(data->bh);
bdrv_drain_poll(data->bs);
data->done = true;
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);
}
static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
@@ -599,8 +599,8 @@ static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
} else {
AioContext *aio_context = bdrv_get_aio_context(child->bs);
- co = qemu_coroutine_create(bdrv_rw_co_entry);
- qemu_coroutine_enter(co, &rwco);
+ co = qemu_coroutine_create(bdrv_rw_co_entry, &rwco);
+ qemu_coroutine_enter(co);
while (rwco.ret == NOT_DONE) {
aio_poll(aio_context, true);
}
@@ -799,7 +799,7 @@ static void bdrv_co_io_em_complete(void *opaque, int ret)
CoroutineIOCompletion *co = opaque;
co->ret = ret;
- qemu_coroutine_enter(co->coroutine, NULL);
+ qemu_coroutine_enter(co->coroutine);
}
static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
@@ -1752,8 +1752,9 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
} else {
AioContext *aio_context = bdrv_get_aio_context(bs);
- co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry);
- qemu_coroutine_enter(co, &data);
+ co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry,
+ &data);
+ qemu_coroutine_enter(co);
while (!data.done) {
aio_poll(aio_context, true);
}
@@ -1901,9 +1902,9 @@ bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
.is_read = is_read,
.ret = -EINPROGRESS,
};
- Coroutine *co = qemu_coroutine_create(bdrv_co_rw_vmstate_entry);
+ Coroutine *co = qemu_coroutine_create(bdrv_co_rw_vmstate_entry, &data);
- qemu_coroutine_enter(co, &data);
+ qemu_coroutine_enter(co);
while (data.ret == -EINPROGRESS) {
aio_poll(bdrv_get_aio_context(bs), true);
}
@@ -2113,8 +2114,8 @@ static BlockAIOCB *bdrv_co_aio_rw_vector(BdrvChild *child,
acb->req.flags = flags;
acb->is_write = is_write;
- co = qemu_coroutine_create(bdrv_co_do_rw);
- qemu_coroutine_enter(co, acb);
+ co = qemu_coroutine_create(bdrv_co_do_rw, acb);
+ qemu_coroutine_enter(co);
bdrv_co_maybe_schedule_bh(acb);
return &acb->common;
@@ -2141,8 +2142,8 @@ BlockAIOCB *bdrv_aio_flush(BlockDriverState *bs,
acb->need_bh = true;
acb->req.error = -EINPROGRESS;
- co = qemu_coroutine_create(bdrv_aio_flush_co_entry);
- qemu_coroutine_enter(co, acb);
+ co = qemu_coroutine_create(bdrv_aio_flush_co_entry, acb);
+ qemu_coroutine_enter(co);
bdrv_co_maybe_schedule_bh(acb);
return &acb->common;
@@ -2171,8 +2172,8 @@ BlockAIOCB *bdrv_aio_discard(BlockDriverState *bs,
acb->req.error = -EINPROGRESS;
acb->req.sector = sector_num;
acb->req.nb_sectors = nb_sectors;
- co = qemu_coroutine_create(bdrv_aio_discard_co_entry);
- qemu_coroutine_enter(co, acb);
+ co = qemu_coroutine_create(bdrv_aio_discard_co_entry, acb);
+ qemu_coroutine_enter(co);
bdrv_co_maybe_schedule_bh(acb);
return &acb->common;
@@ -2313,8 +2314,8 @@ int bdrv_flush(BlockDriverState *bs)
} else {
AioContext *aio_context = bdrv_get_aio_context(bs);
- co = qemu_coroutine_create(bdrv_flush_co_entry);
- qemu_coroutine_enter(co, &flush_co);
+ co = qemu_coroutine_create(bdrv_flush_co_entry, &flush_co);
+ qemu_coroutine_enter(co);
while (flush_co.ret == NOT_DONE) {
aio_poll(aio_context, true);
}
@@ -2442,8 +2443,8 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
} else {
AioContext *aio_context = bdrv_get_aio_context(bs);
- co = qemu_coroutine_create(bdrv_discard_co_entry);
- qemu_coroutine_enter(co, &rwco);
+ co = qemu_coroutine_create(bdrv_discard_co_entry, &rwco);
+ qemu_coroutine_enter(co);
while (rwco.ret == NOT_DONE) {
aio_poll(aio_context, true);
}
@@ -2505,9 +2506,9 @@ int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
/* Fast-path if already in coroutine context */
bdrv_co_ioctl_entry(&data);
} else {
- Coroutine *co = qemu_coroutine_create(bdrv_co_ioctl_entry);
+ Coroutine *co = qemu_coroutine_create(bdrv_co_ioctl_entry, &data);
- qemu_coroutine_enter(co, &data);
+ qemu_coroutine_enter(co);
while (data.ret == -EINPROGRESS) {
aio_poll(bdrv_get_aio_context(bs), true);
}
@@ -2535,8 +2536,8 @@ BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
acb->req.error = -EINPROGRESS;
acb->req.req = req;
acb->req.buf = buf;
- co = qemu_coroutine_create(bdrv_co_aio_ioctl_entry);
- qemu_coroutine_enter(co, acb);
+ co = qemu_coroutine_create(bdrv_co_aio_ioctl_entry, acb);
+ qemu_coroutine_enter(co);
bdrv_co_maybe_schedule_bh(acb);
return &acb->common;
diff --git a/block/iscsi.c b/block/iscsi.c
index 434cb37..cf1e9e7 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -152,7 +152,7 @@ static void iscsi_co_generic_bh_cb(void *opaque)
struct IscsiTask *iTask = opaque;
iTask->complete = 1;
qemu_bh_delete(iTask->bh);
- qemu_coroutine_enter(iTask->co, NULL);
+ qemu_coroutine_enter(iTask->co);
}
static void iscsi_retry_timer_expired(void *opaque)
@@ -160,7 +160,7 @@ static void iscsi_retry_timer_expired(void *opaque)
struct IscsiTask *iTask = opaque;
iTask->complete = 1;
if (iTask->co) {
- qemu_coroutine_enter(iTask->co, NULL);
+ qemu_coroutine_enter(iTask->co);
}
}
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 7df8651..5c104bd 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -94,7 +94,7 @@ static void qemu_laio_process_completion(struct qemu_laiocb *laiocb)
laiocb->ret = ret;
if (laiocb->co) {
- qemu_coroutine_enter(laiocb->co, NULL);
+ qemu_coroutine_enter(laiocb->co);
} else {
laiocb->common.cb(laiocb->common.opaque, ret);
qemu_aio_unref(laiocb);
diff --git a/block/mirror.c b/block/mirror.c
index 705fbc0..71e5ad4 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -121,7 +121,7 @@ static void mirror_iteration_done(MirrorOp *op, int ret)
g_free(op);
if (s->waiting_for_io) {
- qemu_coroutine_enter(s->common.co, NULL);
+ qemu_coroutine_enter(s->common.co);
}
}
@@ -901,9 +901,9 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
bdrv_op_block_all(target, s->common.blocker);
- s->common.co = qemu_coroutine_create(mirror_run);
+ s->common.co = qemu_coroutine_create(mirror_run, s);
trace_mirror_start(bs, s, s->common.co, opaque);
- qemu_coroutine_enter(s->common.co, s);
+ qemu_coroutine_enter(s->common.co);
}
void mirror_start(const char *job_id, BlockDriverState *bs,
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 420bce8..4cc408d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -38,7 +38,7 @@ static void nbd_recv_coroutines_enter_all(NbdClientSession *s)
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
if (s->recv_coroutine[i]) {
- qemu_coroutine_enter(s->recv_coroutine[i], NULL);
+ qemu_coroutine_enter(s->recv_coroutine[i]);
}
}
}
@@ -99,7 +99,7 @@ static void nbd_reply_ready(void *opaque)
}
if (s->recv_coroutine[i]) {
- qemu_coroutine_enter(s->recv_coroutine[i], NULL);
+ qemu_coroutine_enter(s->recv_coroutine[i]);
return;
}
@@ -111,7 +111,7 @@ static void nbd_restart_write(void *opaque)
{
BlockDriverState *bs = opaque;
- qemu_coroutine_enter(nbd_get_client_session(bs)->send_coroutine, NULL);
+ qemu_coroutine_enter(nbd_get_client_session(bs)->send_coroutine);
}
static int nbd_co_send_request(BlockDriverState *bs,
diff --git a/block/nfs.c b/block/nfs.c
index 15d6832..8602a44 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -104,7 +104,7 @@ static void nfs_co_generic_bh_cb(void *opaque)
NFSRPC *task = opaque;
task->complete = 1;
qemu_bh_delete(task->bh);
- qemu_coroutine_enter(task->co, NULL);
+ qemu_coroutine_enter(task->co);
}
static void
diff --git a/block/qcow.c b/block/qcow.c
index ac849bd..0c7b75b 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -948,8 +948,8 @@ static int qcow_write(BlockDriverState *bs, int64_t sector_num,
.nb_sectors = nb_sectors,
.ret = -EINPROGRESS,
};
- co = qemu_coroutine_create(qcow_write_co_entry);
- qemu_coroutine_enter(co, &data);
+ co = qemu_coroutine_create(qcow_write_co_entry, &data);
+ qemu_coroutine_enter(co);
while (data.ret == -EINPROGRESS) {
aio_poll(aio_context, true);
}
diff --git a/block/qcow2.c b/block/qcow2.c
index a5ea19b..a6bca73 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2570,8 +2570,8 @@ static int qcow2_write(BlockDriverState *bs, int64_t sector_num,
.nb_sectors = nb_sectors,
.ret = -EINPROGRESS,
};
- co = qemu_coroutine_create(qcow2_write_co_entry);
- qemu_coroutine_enter(co, &data);
+ co = qemu_coroutine_create(qcow2_write_co_entry, &data);
+ qemu_coroutine_enter(co);
while (data.ret == -EINPROGRESS) {
aio_poll(aio_context, true);
}
diff --git a/block/qed.c b/block/qed.c
index f619d82..426f3cb 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -708,7 +708,7 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l
}
if (cb->co) {
- qemu_coroutine_enter(cb->co, NULL);
+ qemu_coroutine_enter(cb->co);
}
}
@@ -1425,7 +1425,7 @@ static void coroutine_fn qed_co_pwrite_zeroes_cb(void *opaque, int ret)
cb->done = true;
cb->ret = ret;
if (cb->co) {
- qemu_coroutine_enter(cb->co, NULL);
+ qemu_coroutine_enter(cb->co);
}
}
diff --git a/block/sheepdog.c b/block/sheepdog.c
index ef5d044..e739c56 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -495,7 +495,7 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
{
- qemu_coroutine_enter(acb->coroutine, NULL);
+ qemu_coroutine_enter(acb->coroutine);
qemu_aio_unref(acb);
}
@@ -636,7 +636,7 @@ static void restart_co_req(void *opaque)
{
Coroutine *co = opaque;
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);
}
typedef struct SheepdogReqCo {
@@ -726,8 +726,8 @@ static int do_req(int sockfd, AioContext *aio_context, SheepdogReq *hdr,
if (qemu_in_coroutine()) {
do_co_req(&srco);
} else {
- co = qemu_coroutine_create(do_co_req);
- qemu_coroutine_enter(co, &srco);
+ co = qemu_coroutine_create(do_co_req, &srco);
+ qemu_coroutine_enter(co);
while (!srco.finished) {
aio_poll(aio_context, true);
}
@@ -925,17 +925,17 @@ static void co_read_response(void *opaque)
BDRVSheepdogState *s = opaque;
if (!s->co_recv) {
- s->co_recv = qemu_coroutine_create(aio_read_response);
+ s->co_recv = qemu_coroutine_create(aio_read_response, opaque);
}
- qemu_coroutine_enter(s->co_recv, opaque);
+ qemu_coroutine_enter(s->co_recv);
}
static void co_write_request(void *opaque)
{
BDRVSheepdogState *s = opaque;
- qemu_coroutine_enter(s->co_send, NULL);
+ qemu_coroutine_enter(s->co_send);
}
/*
diff --git a/block/ssh.c b/block/ssh.c
index 06928ed..bcbb0e4 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -777,7 +777,7 @@ static void restart_coroutine(void *opaque)
DPRINTF("co=%p", co);
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);
}
static coroutine_fn void set_fd_handler(BDRVSSHState *s, BlockDriverState *bs)
diff --git a/block/stream.c b/block/stream.c
index 54c8cd8..2e7c654 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -235,7 +235,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
s->backing_file_str = g_strdup(backing_file_str);
s->on_error = on_error;
- s->common.co = qemu_coroutine_create(stream_run);
+ s->common.co = qemu_coroutine_create(stream_run, s);
trace_stream_start(bs, base, s, s->common.co, opaque);
- qemu_coroutine_enter(s->common.co, s);
+ qemu_coroutine_enter(s->common.co);
}
diff --git a/block/vmdk.c b/block/vmdk.c
index d73f431..c9914f7 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1686,8 +1686,8 @@ static int vmdk_write_compressed(BlockDriverState *bs,
.nb_sectors = nb_sectors,
.ret = -EINPROGRESS,
};
- co = qemu_coroutine_create(vmdk_co_write_compressed);
- qemu_coroutine_enter(co, &data);
+ co = qemu_coroutine_create(vmdk_co_write_compressed, &data);
+ qemu_coroutine_enter(co);
while (data.ret == -EINPROGRESS) {
aio_poll(aio_context, true);
}
diff --git a/blockjob.c b/blockjob.c
index 3b9cec7..6816b78 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -375,7 +375,7 @@ void block_job_resume(BlockJob *job)
void block_job_enter(BlockJob *job)
{
if (job->co && !job->busy) {
- qemu_coroutine_enter(job->co, NULL);
+ qemu_coroutine_enter(job->co);
}
}
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 9acff92..b6b02b4 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3278,8 +3278,8 @@ void pdu_submit(V9fsPDU *pdu)
if (is_ro_export(&s->ctx) && !is_read_only_op(pdu)) {
handler = v9fs_fs_ro;
}
- co = qemu_coroutine_create(handler);
- qemu_coroutine_enter(co, pdu);
+ co = qemu_coroutine_create(handler, pdu);
+ qemu_coroutine_enter(co);
}
/* Returns 0 on success, 1 on failure. */
diff --git a/hw/9pfs/coth.c b/hw/9pfs/coth.c
index 9b1151b..89018de 100644
--- a/hw/9pfs/coth.c
+++ b/hw/9pfs/coth.c
@@ -22,14 +22,14 @@
static void coroutine_enter_cb(void *opaque, int ret)
{
Coroutine *co = opaque;
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);
}
/* Called from worker thread. */
static int coroutine_enter_func(void *arg)
{
Coroutine *co = arg;
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);
return 0;
}
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 63ae7fe..ac8d4c9 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -61,16 +61,14 @@ typedef void coroutine_fn CoroutineEntry(void *opaque);
* Create a new coroutine
*
* Use qemu_coroutine_enter() to actually transfer control to the coroutine.
+ * The opaque argument is passed as the argument to the entry point.
*/
-Coroutine *qemu_coroutine_create(CoroutineEntry *entry);
+Coroutine *qemu_coroutine_create(CoroutineEntry *entry, void *opaque);
/**
* Transfer control to a coroutine
- *
- * The opaque argument is passed as the argument to the entry point when
- * entering the coroutine for the first time. It is subsequently ignored.
*/
-void qemu_coroutine_enter(Coroutine *coroutine, void *opaque);
+void qemu_coroutine_enter(Coroutine *coroutine);
/**
* Transfer control back to a coroutine's caller
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 3fa7cfe..470f600 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -64,11 +64,11 @@ int qemu_init_main_loop(Error **errp);
*
* void enter_co_bh(void *opaque) {
* QEMUCoroutine *co = opaque;
- * qemu_coroutine_enter(co, NULL);
+ * qemu_coroutine_enter(co);
* }
*
* ...
- * QEMUCoroutine *co = qemu_coroutine_create(coroutine_entry);
+ * QEMUCoroutine *co = qemu_coroutine_create(coroutine_entry, NULL);
* QEMUBH *start_bh = qemu_bh_new(enter_co_bh, co);
* qemu_bh_schedule(start_bh);
* while (...) {
diff --git a/io/channel.c b/io/channel.c
index 692eb17..923c465 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -218,7 +218,7 @@ static gboolean qio_channel_yield_enter(QIOChannel *ioc,
gpointer opaque)
{
QIOChannelYieldData *data = opaque;
- qemu_coroutine_enter(data->co, NULL);
+ qemu_coroutine_enter(data->co);
return FALSE;
}
diff --git a/migration/migration.c b/migration/migration.c
index a560136..c4e0193 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -418,11 +418,11 @@ static void process_incoming_migration_co(void *opaque)
void migration_fd_process_incoming(QEMUFile *f)
{
- Coroutine *co = qemu_coroutine_create(process_incoming_migration_co);
+ Coroutine *co = qemu_coroutine_create(process_incoming_migration_co, f);
migrate_decompress_threads_create();
qemu_file_set_blocking(f, false);
- qemu_coroutine_enter(co, f);
+ qemu_coroutine_enter(co);
}
diff --git a/nbd/server.c b/nbd/server.c
index a677e26..fbc82de 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -106,7 +106,7 @@ static gboolean nbd_negotiate_continue(QIOChannel *ioc,
GIOCondition condition,
void *opaque)
{
- qemu_coroutine_enter(opaque, NULL);
+ qemu_coroutine_enter(opaque);
return TRUE;
}
@@ -1230,9 +1230,9 @@ static void nbd_read(void *opaque)
NBDClient *client = opaque;
if (client->recv_coroutine) {
- qemu_coroutine_enter(client->recv_coroutine, NULL);
+ qemu_coroutine_enter(client->recv_coroutine);
} else {
- qemu_coroutine_enter(qemu_coroutine_create(nbd_trip), client);
+ qemu_coroutine_enter(qemu_coroutine_create(nbd_trip, client));
}
}
@@ -1240,7 +1240,7 @@ static void nbd_restart_write(void *opaque)
{
NBDClient *client = opaque;
- qemu_coroutine_enter(client->send_coroutine, NULL);
+ qemu_coroutine_enter(client->send_coroutine);
}
static void nbd_set_handlers(NBDClient *client)
@@ -1324,6 +1324,6 @@ void nbd_client_new(NBDExport *exp,
client->close = close_fn;
data->client = client;
- data->co = qemu_coroutine_create(nbd_co_client_start);
- qemu_coroutine_enter(data->co, data);
+ data->co = qemu_coroutine_create(nbd_co_client_start, data);
+ qemu_coroutine_enter(data->co);
}
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 09e879f..40fe2eb 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -483,8 +483,8 @@ static int do_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
return -ERANGE;
}
- co = qemu_coroutine_create(co_pwrite_zeroes_entry);
- qemu_coroutine_enter(co, &data);
+ co = qemu_coroutine_create(co_pwrite_zeroes_entry, &data);
+ qemu_coroutine_enter(co);
while (!data.done) {
aio_poll(blk_get_aio_context(blk), true);
}
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index 50e232a..d049cba 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -103,10 +103,10 @@ static BlockJob *test_block_job_start(unsigned int iterations,
s->use_timer = use_timer;
s->rc = rc;
s->result = result;
- s->common.co = qemu_coroutine_create(test_block_job_run);
+ s->common.co = qemu_coroutine_create(test_block_job_run, s);
data->job = s;
data->result = result;
- qemu_coroutine_enter(s->common.co, s);
+ qemu_coroutine_enter(s->common.co);
return &s->common;
}
diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c
index 5171174..ee5e06d 100644
--- a/tests/test-coroutine.c
+++ b/tests/test-coroutine.c
@@ -30,8 +30,8 @@ static void test_in_coroutine(void)
g_assert(!qemu_in_coroutine());
- coroutine = qemu_coroutine_create(verify_in_coroutine);
- qemu_coroutine_enter(coroutine, NULL);
+ coroutine = qemu_coroutine_create(verify_in_coroutine, NULL);
+ qemu_coroutine_enter(coroutine);
}
/*
@@ -48,8 +48,8 @@ static void test_self(void)
{
Coroutine *coroutine;
- coroutine = qemu_coroutine_create(verify_self);
- qemu_coroutine_enter(coroutine, &coroutine);
+ coroutine = qemu_coroutine_create(verify_self, &coroutine);
+ qemu_coroutine_enter(coroutine);
}
/*
@@ -71,8 +71,8 @@ static void coroutine_fn nest(void *opaque)
if (nd->n_enter < nd->max) {
Coroutine *child;
- child = qemu_coroutine_create(nest);
- qemu_coroutine_enter(child, nd);
+ child = qemu_coroutine_create(nest, nd);
+ qemu_coroutine_enter(child);
}
nd->n_return++;
@@ -87,8 +87,8 @@ static void test_nesting(void)
.max = 128,
};
- root = qemu_coroutine_create(nest);
- qemu_coroutine_enter(root, &nd);
+ root = qemu_coroutine_create(nest, &nd);
+ qemu_coroutine_enter(root);
/* Must enter and return from max nesting level */
g_assert_cmpint(nd.n_enter, ==, nd.max);
@@ -116,9 +116,9 @@ static void test_yield(void)
bool done = false;
int i = -1; /* one extra time to return from coroutine */
- coroutine = qemu_coroutine_create(yield_5_times);
+ coroutine = qemu_coroutine_create(yield_5_times, &done);
while (!done) {
- qemu_coroutine_enter(coroutine, &done);
+ qemu_coroutine_enter(coroutine);
i++;
}
g_assert_cmpint(i, ==, 5); /* coroutine must yield 5 times */
@@ -132,7 +132,7 @@ static void coroutine_fn c2_fn(void *opaque)
static void coroutine_fn c1_fn(void *opaque)
{
Coroutine *c2 = opaque;
- qemu_coroutine_enter(c2, NULL);
+ qemu_coroutine_enter(c2);
}
static void test_co_queue(void)
@@ -140,12 +140,12 @@ static void test_co_queue(void)
Coroutine *c1;
Coroutine *c2;
- c1 = qemu_coroutine_create(c1_fn);
- c2 = qemu_coroutine_create(c2_fn);
+ c2 = qemu_coroutine_create(c2_fn, NULL);
+ c1 = qemu_coroutine_create(c1_fn, c2);
- qemu_coroutine_enter(c1, c2);
+ qemu_coroutine_enter(c1);
memset(c1, 0xff, sizeof(Coroutine));
- qemu_coroutine_enter(c2, NULL);
+ qemu_coroutine_enter(c2);
}
/*
@@ -165,14 +165,14 @@ static void test_lifecycle(void)
bool done = false;
/* Create, enter, and return from coroutine */
- coroutine = qemu_coroutine_create(set_and_exit);
- qemu_coroutine_enter(coroutine, &done);
+ coroutine = qemu_coroutine_create(set_and_exit, &done);
+ qemu_coroutine_enter(coroutine);
g_assert(done); /* expect done to be true (first time) */
/* Repeat to check that no state affects this test */
done = false;
- coroutine = qemu_coroutine_create(set_and_exit);
- qemu_coroutine_enter(coroutine, &done);
+ coroutine = qemu_coroutine_create(set_and_exit, &done);
+ qemu_coroutine_enter(coroutine);
g_assert(done); /* expect done to be true (second time) */
}
@@ -206,12 +206,12 @@ static void do_order_test(void)
{
Coroutine *co;
- co = qemu_coroutine_create(co_order_test);
+ co = qemu_coroutine_create(co_order_test, NULL);
record_push(1, 1);
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);
record_push(1, 2);
g_assert(!qemu_in_coroutine());
- qemu_coroutine_enter(co, NULL);
+ qemu_coroutine_enter(co);
record_push(1, 3);
g_assert(!qemu_in_coroutine());
}
@@ -248,8 +248,8 @@ static void perf_lifecycle(void)
g_test_timer_start();
for (i = 0; i < max; i++) {
- coroutine = qemu_coroutine_create(empty_coroutine);
- qemu_coroutine_enter(coroutine, NULL);
+ coroutine = qemu_coroutine_create(empty_coroutine, NULL);
+ qemu_coroutine_enter(coroutine);
}
duration = g_test_timer_elapsed();
@@ -272,8 +272,8 @@ static void perf_nesting(void)
.n_return = 0,
.max = maxnesting,
};
- root = qemu_coroutine_create(nest);
- qemu_coroutine_enter(root, &nd);
+ root = qemu_coroutine_create(nest, &nd);
+ qemu_coroutine_enter(root);
}
duration = g_test_timer_elapsed();
@@ -302,11 +302,11 @@ static void perf_yield(void)
maxcycles = 100000000;
i = maxcycles;
- Coroutine *coroutine = qemu_coroutine_create(yield_loop);
+ Coroutine *coroutine = qemu_coroutine_create(yield_loop, &i);
g_test_timer_start();
while (i > 0) {
- qemu_coroutine_enter(coroutine, &i);
+ qemu_coroutine_enter(coroutine);
}
duration = g_test_timer_elapsed();
@@ -352,9 +352,9 @@ static void perf_cost(void)
g_test_timer_start();
while (i++ < maxcycles) {
- co = qemu_coroutine_create(perf_cost_func);
- qemu_coroutine_enter(co, &i);
- qemu_coroutine_enter(co, NULL);
+ co = qemu_coroutine_create(perf_cost_func, &i);
+ qemu_coroutine_enter(co);
+ qemu_coroutine_enter(co);
}
duration = g_test_timer_elapsed();
ops = (long)(maxcycles / (duration * 1000));
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index b0e1f32..8dbf66a 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -91,9 +91,9 @@ static void co_test_cb(void *opaque)
static void test_submit_co(void)
{
WorkerTestData data;
- Coroutine *co = qemu_coroutine_create(co_test_cb);
+ Coroutine *co = qemu_coroutine_create(co_test_cb, &data);
- qemu_coroutine_enter(co, &data);
+ qemu_coroutine_enter(co);
/* Back here once the worker has started. */
diff --git a/thread-pool.c b/thread-pool.c
index 03ba0b0..6fba913 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -267,7 +267,7 @@ static void thread_pool_co_cb(void *opaque, int ret)
ThreadPoolCo *co = opaque;
co->ret = ret;
- qemu_coroutine_enter(co->co, NULL);
+ qemu_coroutine_enter(co->co);
}
int coroutine_fn thread_pool_submit_co(ThreadPool *pool, ThreadPoolFunc *func,
diff --git a/util/qemu-coroutine-io.c b/util/qemu-coroutine-io.c
index 91b9357..44a8969 100644
--- a/util/qemu-coroutine-io.c
+++ b/util/qemu-coroutine-io.c
@@ -75,7 +75,7 @@ static void fd_coroutine_enter(void *opaque)
{
FDYieldUntilData *data = opaque;
qemu_set_fd_handler(data->fd, NULL, NULL, NULL);
- qemu_coroutine_enter(data->co, NULL);
+ qemu_coroutine_enter(data->co);
}
void coroutine_fn yield_until_fd_readable(int fd)
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index cf53693..22aa9ab 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -57,7 +57,7 @@ void qemu_co_queue_run_restart(Coroutine *co)
trace_qemu_co_queue_run_restart(co);
while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) {
QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next);
- qemu_coroutine_enter(next, NULL);
+ qemu_coroutine_enter(next);
}
}
@@ -103,7 +103,7 @@ bool qemu_co_enter_next(CoQueue *queue)
}
QSIMPLEQ_REMOVE_HEAD(&queue->entries, co_queue_next);
- qemu_coroutine_enter(next, NULL);
+ qemu_coroutine_enter(next);
return true;
}
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 6966831..25de3ed 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -25,7 +25,7 @@ static void co_sleep_cb(void *opaque)
{
CoSleepCB *sleep_cb = opaque;
- qemu_coroutine_enter(sleep_cb->co, NULL);
+ qemu_coroutine_enter(sleep_cb->co);
}
void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUClockType type,
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index b7cb636..89f21a9 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -42,7 +42,7 @@ static void coroutine_pool_cleanup(Notifier *n, void *value)
}
}
-Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
+Coroutine *qemu_coroutine_create(CoroutineEntry *entry, void *opaque)
{
Coroutine *co = NULL;
@@ -76,6 +76,7 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
}
co->entry = entry;
+ co->entry_arg = opaque;
QSIMPLEQ_INIT(&co->co_queue_wakeup);
return co;
}
@@ -100,12 +101,12 @@ static void coroutine_delete(Coroutine *co)
qemu_coroutine_delete(co);
}
-void qemu_coroutine_enter(Coroutine *co, void *opaque)
+void qemu_coroutine_enter(Coroutine *co)
{
Coroutine *self = qemu_coroutine_self();
CoroutineAction ret;
- trace_qemu_coroutine_enter(self, co, opaque);
+ trace_qemu_coroutine_enter(self, co, co->entry_arg);
if (co->caller) {
fprintf(stderr, "Co-routine re-entered recursively\n");
@@ -113,7 +114,6 @@ void qemu_coroutine_enter(Coroutine *co, void *opaque)
}
co->caller = self;
- co->entry_arg = opaque;
ret = qemu_coroutine_switch(self, co, COROUTINE_ENTER);
qemu_co_queue_run_restart(co);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 17/34] block/qdev: Allow node name for drive properties
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (15 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 16/34] coroutine: move entry argument to qemu_coroutine_create Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 18/34] block/qdev: Allow configuring WCE with qdev properties Kevin Wolf
` (17 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
If a node name instead of a BlockBackend name is specified as the driver
for a guest device, an anonymous BlockBackend is created now.
The order of operations in release_drive() must be reversed in order to
avoid a use-after-free bug because now blk_detach_dev() frees the last
reference if an anonymous BlockBackend is used.
usb-storage uses a hack where it forwards its BlockBackend as a property
to another device that it internally creates. This hack must be updated
so that it doesn't drop its original BB before it can be passed to the
other device. This used to work because we always had the monitor
reference around, but with node-names the device reference is the only
one now.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
hw/core/qdev-properties-system.c | 39 +++++++++++++++++++++++++++++++++------
hw/usb/dev-storage.c | 5 ++++-
2 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 65d9fa9..ab1bc5e 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -72,12 +72,21 @@ static void parse_drive(DeviceState *dev, const char *str, void **ptr,
const char *propname, Error **errp)
{
BlockBackend *blk;
+ bool blk_created = false;
blk = blk_by_name(str);
if (!blk) {
+ BlockDriverState *bs = bdrv_lookup_bs(NULL, str, NULL);
+ if (bs) {
+ blk = blk_new();
+ blk_insert_bs(blk, bs);
+ blk_created = true;
+ }
+ }
+ if (!blk) {
error_setg(errp, "Property '%s.%s' can't find value '%s'",
object_get_typename(OBJECT(dev)), propname, str);
- return;
+ goto fail;
}
if (blk_attach_dev(blk, dev) < 0) {
DriveInfo *dinfo = blk_legacy_dinfo(blk);
@@ -91,9 +100,16 @@ static void parse_drive(DeviceState *dev, const char *str, void **ptr,
error_setg(errp, "Drive '%s' is already in use by another device",
str);
}
- return;
+ goto fail;
}
+
*ptr = blk;
+
+fail:
+ if (blk_created) {
+ /* If we need to keep a reference, blk_attach_dev() took it */
+ blk_unref(blk);
+ }
}
static void release_drive(Object *obj, const char *name, void *opaque)
@@ -103,8 +119,8 @@ static void release_drive(Object *obj, const char *name, void *opaque)
BlockBackend **ptr = qdev_get_prop_ptr(dev, prop);
if (*ptr) {
- blk_detach_dev(*ptr, dev);
blockdev_auto_del(*ptr);
+ blk_detach_dev(*ptr, dev);
}
}
@@ -127,7 +143,7 @@ static void set_drive(Object *obj, Visitor *v, const char *name, void *opaque,
PropertyInfo qdev_prop_drive = {
.name = "str",
- .description = "ID of a drive to use as a backend",
+ .description = "Node name or ID of a block device to use as a backend",
.get = get_drive,
.set = set_drive,
.release = release_drive,
@@ -362,8 +378,19 @@ PropertyInfo qdev_prop_vlan = {
void qdev_prop_set_drive(DeviceState *dev, const char *name,
BlockBackend *value, Error **errp)
{
- object_property_set_str(OBJECT(dev), value ? blk_name(value) : "",
- name, errp);
+ const char *ref = "";
+
+ if (value) {
+ ref = blk_name(value);
+ if (!*ref) {
+ BlockDriverState *bs = blk_bs(value);
+ if (bs) {
+ ref = bdrv_get_node_name(bs);
+ }
+ }
+ }
+
+ object_property_set_str(OBJECT(dev), ref, name, errp);
}
void qdev_prop_set_chr(DeviceState *dev, const char *name,
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 4d605b8..78038a2 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -609,10 +609,12 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
* a SCSI bus that can serve only a single device, which it
* creates automatically. But first it needs to detach from its
* blockdev, or else scsi_bus_legacy_add_drive() dies when it
- * attaches again.
+ * attaches again. We also need to take another reference so that
+ * blk_detach_dev() doesn't free blk while we still need it.
*
* The hack is probably a bad idea.
*/
+ blk_ref(blk);
blk_detach_dev(blk, &s->dev.qdev);
s->conf.blk = NULL;
@@ -623,6 +625,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable,
s->conf.bootindex, dev->serial,
&err);
+ blk_unref(blk);
if (!scsi_dev) {
error_propagate(errp, err);
return;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 18/34] block/qdev: Allow configuring WCE with qdev properties
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (16 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 17/34] block/qdev: Allow node name for drive properties Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 19/34] commit: Fix use of error handling policy Kevin Wolf
` (16 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
As cache.writeback is a BlockBackend property and as such more related
to the guest device than the BlockDriverState, we already removed it
from the blockdev-add interface. This patch adds the new way to set it,
as a qdev property of the corresponding guest device.
For example: -drive if=none,file=test.img,node-name=img
-device ide-hd,drive=img,write-cache=off
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
hw/block/block.c | 16 ++++++++++++++++
hw/block/nvme.c | 1 +
hw/block/virtio-blk.c | 1 +
hw/ide/qdev.c | 1 +
hw/scsi/scsi-disk.c | 1 +
hw/usb/dev-storage.c | 1 +
include/hw/block/block.h | 5 ++++-
7 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/hw/block/block.c b/hw/block/block.c
index 97a59d4..396b0d5 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -51,6 +51,22 @@ void blkconf_blocksizes(BlockConf *conf)
}
}
+void blkconf_apply_backend_options(BlockConf *conf)
+{
+ BlockBackend *blk = conf->blk;
+ bool wce;
+
+ switch (conf->wce) {
+ case ON_OFF_AUTO_ON: wce = true; break;
+ case ON_OFF_AUTO_OFF: wce = false; break;
+ case ON_OFF_AUTO_AUTO: wce = blk_enable_write_cache(blk); break;
+ default:
+ abort();
+ }
+
+ blk_set_enable_write_cache(blk, wce);
+}
+
void blkconf_geometry(BlockConf *conf, int *ptrans,
unsigned cyls_max, unsigned heads_max, unsigned secs_max,
Error **errp)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 280d54d..2ded247 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -803,6 +803,7 @@ static int nvme_init(PCIDevice *pci_dev)
return -1;
}
blkconf_blocksizes(&n->conf);
+ blkconf_apply_backend_options(&n->conf);
pci_conf = pci_dev->config;
pci_conf[PCI_INTERRUPT_PIN] = 1;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index ae86e94..ecd8ea3 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -897,6 +897,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
}
blkconf_serial(&conf->conf, &conf->serial);
+ blkconf_apply_backend_options(&conf->conf);
s->original_wce = blk_enable_write_cache(conf->conf.blk);
blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, &err);
if (err) {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index d07b44e..33619f4 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -180,6 +180,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
return -1;
}
}
+ blkconf_apply_backend_options(&dev->conf);
if (ide_init_drive(s, dev->conf.blk, kind,
dev->version, dev->serial, dev->model, dev->wwn,
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 36f8a85..8c26a4e 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2309,6 +2309,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
return;
}
}
+ blkconf_apply_backend_options(&dev->conf);
if (s->qdev.conf.discard_granularity == -1) {
s->qdev.conf.discard_granularity =
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 78038a2..c607f76 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -603,6 +603,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
blkconf_serial(&s->conf, &dev->serial);
blkconf_blocksizes(&s->conf);
+ blkconf_apply_backend_options(&s->conf);
/*
* Hack alert: this pretends to be a block device, but it's really
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 87c87ed..245ac99 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -25,6 +25,7 @@ typedef struct BlockConf {
uint32_t discard_granularity;
/* geometry, not all devices use this */
uint32_t cyls, heads, secs;
+ OnOffAuto wce;
} BlockConf;
static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -49,7 +50,8 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \
DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \
DEFINE_PROP_UINT32("discard_granularity", _state, \
- _conf.discard_granularity, -1)
+ _conf.discard_granularity, -1), \
+ DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, ON_OFF_AUTO_AUTO)
#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
@@ -63,6 +65,7 @@ void blkconf_geometry(BlockConf *conf, int *trans,
unsigned cyls_max, unsigned heads_max, unsigned secs_max,
Error **errp);
void blkconf_blocksizes(BlockConf *conf);
+void blkconf_apply_backend_options(BlockConf *conf);
/* Hard disk geometry */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 19/34] commit: Fix use of error handling policy
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (17 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 18/34] block/qdev: Allow configuring WCE with qdev properties Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 20/34] block/qdev: Allow configuring rerror/werror with qdev properties Kevin Wolf
` (15 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Commit implemented the 'enospc' policy as 'ignore' if the error was not
ENOSPC. The QAPI documentation promises that it's treated as 'stop'.
Using the common block job error handling function fixes this and also
adds the missing QMP event.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/commit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 8b534d7..5d11eb6 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -171,9 +171,9 @@ wait:
bytes_written += n * BDRV_SECTOR_SIZE;
}
if (ret < 0) {
- if (s->on_error == BLOCKDEV_ON_ERROR_STOP ||
- s->on_error == BLOCKDEV_ON_ERROR_REPORT||
- (s->on_error == BLOCKDEV_ON_ERROR_ENOSPC && ret == -ENOSPC)) {
+ BlockErrorAction action =
+ block_job_error_action(&s->common, false, s->on_error, -ret);
+ if (action == BLOCK_ERROR_ACTION_REPORT) {
goto out;
} else {
n = 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 20/34] block/qdev: Allow configuring rerror/werror with qdev properties
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (18 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 19/34] commit: Fix use of error handling policy Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 21/34] qemu-iotests: Test setting WCE with qdev Kevin Wolf
` (14 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
The rerror/werror policies are implemented in the devices, so that's
where they should be configured. In comparison to the old options in
-drive, the qdev properties are only added to those devices that
actually support them.
If the option isn't given (or "auto" is specified), the setting of the
BlockBackend is used for compatibility with the old options. For block
jobs, "auto" is the same as "enospc".
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/block-backend.c | 1 +
blockjob.c | 1 +
hw/block/block.c | 12 ++++++++++++
hw/block/virtio-blk.c | 1 +
hw/core/qdev-properties.c | 13 +++++++++++++
hw/ide/qdev.c | 1 +
hw/scsi/scsi-disk.c | 1 +
include/hw/block/block.h | 8 ++++++++
include/hw/qdev-properties.h | 4 ++++
qapi/block-core.json | 4 +++-
10 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 0d7b801..f9cea1b 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1173,6 +1173,7 @@ BlockErrorAction blk_get_error_action(BlockBackend *blk, bool is_read,
return BLOCK_ERROR_ACTION_REPORT;
case BLOCKDEV_ON_ERROR_IGNORE:
return BLOCK_ERROR_ACTION_IGNORE;
+ case BLOCKDEV_ON_ERROR_AUTO:
default:
abort();
}
diff --git a/blockjob.c b/blockjob.c
index 6816b78..a5ba3be 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -553,6 +553,7 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
switch (on_err) {
case BLOCKDEV_ON_ERROR_ENOSPC:
+ case BLOCKDEV_ON_ERROR_AUTO:
action = (error == ENOSPC) ?
BLOCK_ERROR_ACTION_STOP : BLOCK_ERROR_ACTION_REPORT;
break;
diff --git a/hw/block/block.c b/hw/block/block.c
index 396b0d5..8dc9d84 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -54,6 +54,7 @@ void blkconf_blocksizes(BlockConf *conf)
void blkconf_apply_backend_options(BlockConf *conf)
{
BlockBackend *blk = conf->blk;
+ BlockdevOnError rerror, werror;
bool wce;
switch (conf->wce) {
@@ -64,7 +65,18 @@ void blkconf_apply_backend_options(BlockConf *conf)
abort();
}
+ rerror = conf->rerror;
+ if (rerror == BLOCKDEV_ON_ERROR_AUTO) {
+ rerror = blk_get_on_error(blk, true);
+ }
+
+ werror = conf->werror;
+ if (werror == BLOCKDEV_ON_ERROR_AUTO) {
+ werror = blk_get_on_error(blk, false);
+ }
+
blk_set_enable_write_cache(blk, wce);
+ blk_set_on_error(blk, rerror, werror);
}
void blkconf_geometry(BlockConf *conf, int *ptrans,
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index ecd8ea3..357ff90 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -960,6 +960,7 @@ static void virtio_blk_instance_init(Object *obj)
static Property virtio_blk_properties[] = {
DEFINE_BLOCK_PROPERTIES(VirtIOBlock, conf.conf),
+ DEFINE_BLOCK_ERROR_PROPERTIES(VirtIOBlock, conf.conf),
DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, conf.conf),
DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial),
DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true),
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3c20c8e..14e544a 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -539,6 +539,19 @@ PropertyInfo qdev_prop_losttickpolicy = {
.set = set_enum,
};
+/* --- Block device error handling policy --- */
+
+QEMU_BUILD_BUG_ON(sizeof(BlockdevOnError) != sizeof(int));
+
+PropertyInfo qdev_prop_blockdev_on_error = {
+ .name = "BlockdevOnError",
+ .description = "Error handling policy, "
+ "report/ignore/enospc/stop/auto",
+ .enum_table = BlockdevOnError_lookup,
+ .get = get_enum,
+ .set = set_enum,
+};
+
/* --- BIOS CHS translation */
QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) != sizeof(int));
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 33619f4..67c76bf 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -264,6 +264,7 @@ static int ide_drive_initfn(IDEDevice *dev)
#define DEFINE_IDE_DEV_PROPERTIES() \
DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf), \
+ DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf), \
DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 8c26a4e..8dbfc10 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2849,6 +2849,7 @@ static const TypeInfo scsi_disk_base_info = {
#define DEFINE_SCSI_DISK_PROPERTIES() \
DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf), \
+ DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
DEFINE_PROP_STRING("ver", SCSIDiskState, version), \
DEFINE_PROP_STRING("serial", SCSIDiskState, serial), \
DEFINE_PROP_STRING("vendor", SCSIDiskState, vendor), \
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 245ac99..df9d207 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -26,6 +26,8 @@ typedef struct BlockConf {
/* geometry, not all devices use this */
uint32_t cyls, heads, secs;
OnOffAuto wce;
+ BlockdevOnError rerror;
+ BlockdevOnError werror;
} BlockConf;
static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -58,6 +60,12 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf) \
+ DEFINE_PROP_BLOCKDEV_ON_ERROR("rerror", _state, _conf.rerror, \
+ BLOCKDEV_ON_ERROR_AUTO), \
+ DEFINE_PROP_BLOCKDEV_ON_ERROR("werror", _state, _conf.werror, \
+ BLOCKDEV_ON_ERROR_AUTO)
+
/* Configuration helpers */
void blkconf_serial(BlockConf *conf, char **serial);
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 034b75a..2a9d2f9 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -20,6 +20,7 @@ extern PropertyInfo qdev_prop_ptr;
extern PropertyInfo qdev_prop_macaddr;
extern PropertyInfo qdev_prop_on_off_auto;
extern PropertyInfo qdev_prop_losttickpolicy;
+extern PropertyInfo qdev_prop_blockdev_on_error;
extern PropertyInfo qdev_prop_bios_chs_trans;
extern PropertyInfo qdev_prop_fdc_drive_type;
extern PropertyInfo qdev_prop_drive;
@@ -161,6 +162,9 @@ extern PropertyInfo qdev_prop_arraylen;
#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
LostTickPolicy)
+#define DEFINE_PROP_BLOCKDEV_ON_ERROR(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blockdev_on_error, \
+ BlockdevOnError)
#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f) \
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3f77dac..0d30187 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -664,10 +664,12 @@
# @stop: for guest operations, stop the virtual machine;
# for jobs, pause the job
#
+# @auto: inherit the error handling policy of the backend (since: 2.7)
+#
# Since: 1.3
##
{ 'enum': 'BlockdevOnError',
- 'data': ['report', 'ignore', 'enospc', 'stop'] }
+ 'data': ['report', 'ignore', 'enospc', 'stop', 'auto'] }
##
# @MirrorSyncMode:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 21/34] qemu-iotests: Test setting WCE with qdev
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (19 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 20/34] block/qdev: Allow configuring rerror/werror with qdev properties Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 22/34] block: Remove BB options from blockdev-add Kevin Wolf
` (13 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/157 | 88 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/157.out | 22 ++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 111 insertions(+)
create mode 100755 tests/qemu-iotests/157
create mode 100644 tests/qemu-iotests/157.out
diff --git a/tests/qemu-iotests/157 b/tests/qemu-iotests/157
new file mode 100755
index 0000000..2699168
--- /dev/null
+++ b/tests/qemu-iotests/157
@@ -0,0 +1,88 @@
+#!/bin/bash
+#
+# Test command line configuration of block devices with qdev
+#
+# Copyright (C) 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+ echo Testing: "$@"
+ (
+ if ! test -t 0; then
+ while read cmd; do
+ echo $cmd
+ done
+ fi
+ echo quit
+ ) | $QEMU -nodefaults -nographic -monitor stdio -serial none "$@"
+ echo
+}
+
+function run_qemu()
+{
+ do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_generated_node_ids
+}
+
+
+size=128M
+drive="if=none,file=$TEST_IMG,driver=$IMGFMT"
+
+_make_test_img $size
+
+echo
+echo "=== Setting WCE with qdev and with manually created BB ==="
+echo
+
+# The qdev option takes precedence, but if it isn't given or 'auto', the BB
+# option is used instead.
+
+for cache in "writeback" "writethrough"; do
+ for wce in "" ",write-cache=auto" ",write-cache=on" ",write-cache=off"; do
+ echo "info block" \
+ | run_qemu -drive "$drive,cache=$cache" \
+ -device "virtio-blk,drive=none0$wce" \
+ | grep -e "Testing" -e "Cache mode"
+ done
+done
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/157.out b/tests/qemu-iotests/157.out
new file mode 100644
index 0000000..5aa9c0e
--- /dev/null
+++ b/tests/qemu-iotests/157.out
@@ -0,0 +1,22 @@
+QA output created by 157
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+
+=== Setting WCE with qdev and with manually created BB ===
+
+Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback -device virtio-blk,drive=none0
+ Cache mode: writeback
+Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback -device virtio-blk,drive=none0,write-cache=auto
+ Cache mode: writeback
+Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback -device virtio-blk,drive=none0,write-cache=on
+ Cache mode: writeback
+Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback -device virtio-blk,drive=none0,write-cache=off
+ Cache mode: writethrough
+Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device virtio-blk,drive=none0
+ Cache mode: writethrough
+Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device virtio-blk,drive=none0,write-cache=auto
+ Cache mode: writethrough
+Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device virtio-blk,drive=none0,write-cache=on
+ Cache mode: writeback
+Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device virtio-blk,drive=none0,write-cache=off
+ Cache mode: writethrough
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1c6fcb6..3a3973e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -156,3 +156,4 @@
154 rw auto backing quick
155 rw auto
156 rw auto quick
+157 auto
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 22/34] block: Remove BB options from blockdev-add
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (20 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 21/34] qemu-iotests: Test setting WCE with qdev Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 23/34] qemu-img: Use strerror() for generic resize error Kevin Wolf
` (12 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
werror/rerror are now available as qdev options. The stats-* options are
removed without an existing replacement; they should probably be
configurable with a separate QMP command like I/O throttling settings.
Removing id is left for another day because this involves updating
qemu-iotests cases to use node-name for everything. Before we can do
that, however, all QMP commands must support node-name.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-core.json | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0d30187..3444a9b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2081,20 +2081,8 @@
# @discard: #optional discard-related options (default: ignore)
# @cache: #optional cache-related options
# @aio: #optional AIO backend (default: threads)
-# @rerror: #optional how to handle read errors on the device
-# (default: report)
-# @werror: #optional how to handle write errors on the device
-# (default: enospc)
# @read-only: #optional whether the block device should be read-only
# (default: false)
-# @stats-account-invalid: #optional whether to include invalid
-# operations when computing last access statistics
-# (default: true) (Since 2.5)
-# @stats-account-failed: #optional whether to include failed
-# operations when computing latency and last
-# access statistics (default: true) (Since 2.5)
-# @stats-intervals: #optional list of intervals for collecting I/O
-# statistics, in seconds (default: none) (Since 2.5)
# @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
# (default: off)
#
@@ -2104,17 +2092,13 @@
##
{ 'union': 'BlockdevOptions',
'base': { 'driver': 'BlockdevDriver',
+# TODO 'id' is a BB-level option, remove it
'*id': 'str',
'*node-name': 'str',
'*discard': 'BlockdevDiscardOptions',
'*cache': 'BlockdevCacheOptions',
'*aio': 'BlockdevAioOptions',
- '*rerror': 'BlockdevOnError',
- '*werror': 'BlockdevOnError',
'*read-only': 'bool',
- '*stats-account-invalid': 'bool',
- '*stats-account-failed': 'bool',
- '*stats-intervals': ['int'],
'*detect-zeroes': 'BlockdevDetectZeroesOptions' },
'discriminator': 'driver',
'data': {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 23/34] qemu-img: Use strerror() for generic resize error
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (21 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 22/34] block: Remove BB options from blockdev-add Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 24/34] qcow2: Avoid making the L1 table too big Kevin Wolf
` (11 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Emitting the plain error number is not very helpful. Use strerror()
instead.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20160615153630.2116-2-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index 969edce..2e40e1f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3283,7 +3283,7 @@ static int img_resize(int argc, char **argv)
error_report("Image is read-only");
break;
default:
- error_report("Error resizing image (%d)", -ret);
+ error_report("Error resizing image: %s", strerror(-ret));
break;
}
out:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 24/34] qcow2: Avoid making the L1 table too big
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (22 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 23/34] qemu-img: Use strerror() for generic resize error Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 25/34] qemu-io: Use correct range limitations Kevin Wolf
` (10 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
We refuse to open images whose L1 table we deem "too big". Consequently,
we should not produce such images ourselves.
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20160615153630.2116-3-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
[mreitz: Added QEMU_BUILD_BUG_ON()]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-cluster.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 6b92ce9..00c16dc 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -65,7 +65,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
}
}
- if (new_l1_size > INT_MAX / sizeof(uint64_t)) {
+ QEMU_BUILD_BUG_ON(QCOW_MAX_L1_SIZE > INT_MAX);
+ if (new_l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) {
return -EFBIG;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 25/34] qemu-io: Use correct range limitations
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (23 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 24/34] qcow2: Avoid making the L1 table too big Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 26/34] qcow2: Fix qcow2_get_cluster_offset() Kevin Wolf
` (9 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
create_iovec() has a comment lamenting the lack of SIZE_T_MAX. Since
there actually is a SIZE_MAX, use it.
Two places use INT_MAX for checking the upper bound of a sector count
that is used as an argument for a blk_*() function (blk_discard() and
blk_write_compressed(), respectively). BDRV_REQUEST_MAX_SECTORS should
be used instead.
And finally, do_co_pwrite_zeroes() used to similarly check that the
sector count does not exceed INT_MAX. However, this function is now
backed by blk_co_pwrite_zeroes() which takes bytes as an argument
instead of sectors. Therefore, it should be the byte count that does not
exceed INT_MAX, not the sector count.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-io-cmds.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 40fe2eb..6e29edb 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -389,9 +389,9 @@ create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char **argv, int nr_iov,
goto fail;
}
- /* should be SIZE_T_MAX, but that doesn't exist */
- if (len > INT_MAX) {
- printf("Argument '%s' exceeds maximum size %d\n", arg, INT_MAX);
+ if (len > SIZE_MAX) {
+ printf("Argument '%s' exceeds maximum size %llu\n", arg,
+ (unsigned long long)SIZE_MAX);
goto fail;
}
@@ -479,7 +479,7 @@ static int do_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
.done = false,
};
- if (count >> BDRV_SECTOR_BITS > INT_MAX) {
+ if (count > INT_MAX) {
return -ERANGE;
}
@@ -500,7 +500,7 @@ static int do_write_compressed(BlockBackend *blk, char *buf, int64_t offset,
{
int ret;
- if (count >> 9 > INT_MAX) {
+ if (count >> 9 > BDRV_REQUEST_MAX_SECTORS) {
return -ERANGE;
}
@@ -1688,9 +1688,9 @@ static int discard_f(BlockBackend *blk, int argc, char **argv)
if (count < 0) {
print_cvtnum_err(count, argv[optind]);
return 0;
- } else if (count >> BDRV_SECTOR_BITS > INT_MAX) {
+ } else if (count >> BDRV_SECTOR_BITS > BDRV_REQUEST_MAX_SECTORS) {
printf("length cannot exceed %"PRIu64", given %s\n",
- (uint64_t)INT_MAX << BDRV_SECTOR_BITS,
+ (uint64_t)BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS,
argv[optind]);
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 26/34] qcow2: Fix qcow2_get_cluster_offset()
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (24 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 25/34] qemu-io: Use correct range limitations Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 27/34] Improve block job rate limiting for small bandwidth values Kevin Wolf
` (8 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Recently, qcow2_get_cluster_offset() has been changed to work with bytes
instead of sectors. This invalidated some assertions and introduced a
possible integer multiplication overflow.
This could be reproduced using e.g.
$ qemu-img create -f qcow2 -o cluster_size=1M blub.qcow2 8G
Formatting 'foo.qcow2', fmt=qcow2 size=8589934592 encryption=off
cluster_size=1048576 lazy_refcounts=off refcount_bits=16
$ qemu-io -c map blub.qcow2
qemu-io: qemu/block/qcow2-cluster.c:504: qcow2_get_cluster_offset:
Assertion `bytes_needed <= INT_MAX' failed.
[1] 20775 abort (core dumped) qemu-io -c map foo.qcow2
This patch removes the now wrong assertion, adding comments and more
assertions to prove its correctness (and fixing the overflow which would
become apparent with the original assertion removed).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20160620142623.24471-3-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-cluster.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 00c16dc..f941835 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -483,8 +483,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int l2_index;
uint64_t l1_index, l2_offset, *l2_table;
int l1_bits, c;
- unsigned int offset_in_cluster, nb_clusters;
- uint64_t bytes_available, bytes_needed;
+ unsigned int offset_in_cluster;
+ uint64_t bytes_available, bytes_needed, nb_clusters;
int ret;
offset_in_cluster = offset_into_cluster(s, offset);
@@ -500,7 +500,6 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
if (bytes_needed > bytes_available) {
bytes_needed = bytes_available;
}
- assert(bytes_needed <= INT_MAX);
*cluster_offset = 0;
@@ -537,8 +536,11 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
l2_index = (offset >> s->cluster_bits) & (s->l2_size - 1);
*cluster_offset = be64_to_cpu(l2_table[l2_index]);
- /* nb_needed <= INT_MAX, thus nb_clusters <= INT_MAX, too */
nb_clusters = size_to_clusters(s, bytes_needed);
+ /* bytes_needed <= *bytes + offset_in_cluster, both of which are unsigned
+ * integers; the minimum cluster size is 512, so this assertion is always
+ * true */
+ assert(nb_clusters <= INT_MAX);
ret = qcow2_get_cluster_type(*cluster_offset);
switch (ret) {
@@ -585,13 +587,17 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
- bytes_available = (c * s->cluster_size);
+ bytes_available = (int64_t)c * s->cluster_size;
out:
if (bytes_available > bytes_needed) {
bytes_available = bytes_needed;
}
+ /* bytes_available <= bytes_needed <= *bytes + offset_in_cluster;
+ * subtracting offset_in_cluster will therefore definitely yield something
+ * not exceeding UINT_MAX */
+ assert(bytes_available - offset_in_cluster <= UINT_MAX);
*bytes = bytes_available - offset_in_cluster;
return ret;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 27/34] Improve block job rate limiting for small bandwidth values
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (25 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 26/34] qcow2: Fix qcow2_get_cluster_offset() Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 28/34] vmdk: fix metadata write regression Kevin Wolf
` (7 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Sascha Silbe <silbe@linux.vnet.ibm.com>
ratelimit_calculate_delay() previously reset the accounting every time
slice, no matter how much data had been processed before. This had (at
least) two consequences:
1. The minimum speed is rather large, e.g. 5 MiB/s for commit and stream.
Not sure if there are real-world use cases where this would be a
problem. Mirroring and backup over a slow link (e.g. DSL) would
come to mind, though.
2. Tests for block job operations (e.g. cancel) were rather racy
All block jobs currently use a time slice of 100ms. That's a
reasonable value to get smooth output during regular
operation. However this also meant that the state of block jobs
changed every 100ms, no matter how low the configured limit was. On
busy hosts, qemu often transferred additional chunks until the test
case had a chance to cancel the job.
Fix the block job rate limit code to delay for more than one time
slice to address the above issues. To make it easier to handle
oversized chunks we switch the semantics from returning a delay
_before_ the current request to a delay _after_ the current
request. If necessary, this delay consists of multiple time slice
units.
Since the mirror job sends multiple chunks in one go even if the rate
limit was exceeded in between, we need to keep track of the start of
the current time slice so we can correctly re-compute the delay for
the updated amount of data.
The minimum bandwidth now is 1 data unit per time slice. The block
jobs are currently passing the amount of data transferred in sectors
and using 100ms time slices, so this translates to 5120
bytes/second. With chunk sizes usually being O(512KiB), tests have
plenty of time (O(100s)) to operate on block jobs. The chance of a
race condition now is fairly remote, except possibly on insanely
loaded systems.
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Message-id: 1467127721-9564-2-git-send-email-silbe@linux.vnet.ibm.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/commit.c | 13 +++++--------
block/mirror.c | 4 +++-
block/stream.c | 12 ++++--------
include/qemu/ratelimit.h | 43 ++++++++++++++++++++++++++++++++++---------
4 files changed, 46 insertions(+), 26 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 5d11eb6..553e18d 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -113,6 +113,7 @@ static void coroutine_fn commit_run(void *opaque)
CommitBlockJob *s = opaque;
CommitCompleteData *data;
int64_t sector_num, end;
+ uint64_t delay_ns = 0;
int ret = 0;
int n = 0;
void *buf = NULL;
@@ -142,10 +143,8 @@ static void coroutine_fn commit_run(void *opaque)
buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE);
for (sector_num = 0; sector_num < end; sector_num += n) {
- uint64_t delay_ns = 0;
bool copy;
-wait:
/* Note that even when no rate limit is applied we need to yield
* with no pending I/O here so that bdrv_drain_all() returns.
*/
@@ -161,12 +160,6 @@ wait:
copy = (ret == 1);
trace_commit_one_iteration(s, sector_num, n, ret);
if (copy) {
- if (s->common.speed) {
- delay_ns = ratelimit_calculate_delay(&s->limit, n);
- if (delay_ns > 0) {
- goto wait;
- }
- }
ret = commit_populate(s->top, s->base, sector_num, n, buf);
bytes_written += n * BDRV_SECTOR_SIZE;
}
@@ -182,6 +175,10 @@ wait:
}
/* Publish progress */
s->common.offset += n * BDRV_SECTOR_SIZE;
+
+ if (copy && s->common.speed) {
+ delay_ns = ratelimit_calculate_delay(&s->limit, n);
+ }
}
ret = 0;
diff --git a/block/mirror.c b/block/mirror.c
index 71e5ad4..b1e633e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -422,7 +422,9 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
assert(io_sectors);
sector_num += io_sectors;
nb_chunks -= DIV_ROUND_UP(io_sectors, sectors_per_chunk);
- delay_ns += ratelimit_calculate_delay(&s->limit, io_sectors);
+ if (s->common.speed) {
+ delay_ns = ratelimit_calculate_delay(&s->limit, io_sectors);
+ }
}
return delay_ns;
}
diff --git a/block/stream.c b/block/stream.c
index 2e7c654..3187481 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -95,6 +95,7 @@ static void coroutine_fn stream_run(void *opaque)
BlockDriverState *base = s->base;
int64_t sector_num = 0;
int64_t end = -1;
+ uint64_t delay_ns = 0;
int error = 0;
int ret = 0;
int n = 0;
@@ -123,10 +124,8 @@ static void coroutine_fn stream_run(void *opaque)
}
for (sector_num = 0; sector_num < end; sector_num += n) {
- uint64_t delay_ns = 0;
bool copy;
-wait:
/* Note that even when no rate limit is applied we need to yield
* with no pending I/O here so that bdrv_drain_all() returns.
*/
@@ -156,12 +155,6 @@ wait:
}
trace_stream_one_iteration(s, sector_num, n, ret);
if (copy) {
- if (s->common.speed) {
- delay_ns = ratelimit_calculate_delay(&s->limit, n);
- if (delay_ns > 0) {
- goto wait;
- }
- }
ret = stream_populate(blk, sector_num, n, buf);
}
if (ret < 0) {
@@ -182,6 +175,9 @@ wait:
/* Publish progress */
s->common.offset += n * BDRV_SECTOR_SIZE;
+ if (copy && s->common.speed) {
+ delay_ns = ratelimit_calculate_delay(&s->limit, n);
+ }
}
if (!base) {
diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h
index 1e3cb13..8da1232 100644
--- a/include/qemu/ratelimit.h
+++ b/include/qemu/ratelimit.h
@@ -15,34 +15,59 @@
#define QEMU_RATELIMIT_H
typedef struct {
- int64_t next_slice_time;
+ int64_t slice_start_time;
+ int64_t slice_end_time;
uint64_t slice_quota;
uint64_t slice_ns;
uint64_t dispatched;
} RateLimit;
+/** Calculate and return delay for next request in ns
+ *
+ * Record that we sent @p n data units. If we may send more data units
+ * in the current time slice, return 0 (i.e. no delay). Otherwise
+ * return the amount of time (in ns) until the start of the next time
+ * slice that will permit sending the next chunk of data.
+ *
+ * Recording sent data units even after exceeding the quota is
+ * permitted; the time slice will be extended accordingly.
+ */
static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
{
int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+ uint64_t delay_slices;
- if (limit->next_slice_time < now) {
- limit->next_slice_time = now + limit->slice_ns;
+ assert(limit->slice_quota && limit->slice_ns);
+
+ if (limit->slice_end_time < now) {
+ /* Previous, possibly extended, time slice finished; reset the
+ * accounting. */
+ limit->slice_start_time = now;
+ limit->slice_end_time = now + limit->slice_ns;
limit->dispatched = 0;
}
- if (limit->dispatched == 0 || limit->dispatched + n <= limit->slice_quota) {
- limit->dispatched += n;
+
+ limit->dispatched += n;
+ if (limit->dispatched < limit->slice_quota) {
+ /* We may send further data within the current time slice, no
+ * need to delay the next request. */
return 0;
- } else {
- limit->dispatched = n;
- return limit->next_slice_time - now;
}
+
+ /* Quota exceeded. Calculate the next time slice we may start
+ * sending data again. */
+ delay_slices = (limit->dispatched + limit->slice_quota - 1) /
+ limit->slice_quota;
+ limit->slice_end_time = limit->slice_start_time +
+ delay_slices * limit->slice_ns;
+ return limit->slice_end_time - now;
}
static inline void ratelimit_set_speed(RateLimit *limit, uint64_t speed,
uint64_t slice_ns)
{
limit->slice_ns = slice_ns;
- limit->slice_quota = ((double)speed * slice_ns)/1000000000ULL;
+ limit->slice_quota = MAX(((double)speed * slice_ns) / 1000000000ULL, 1);
}
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 28/34] vmdk: fix metadata write regression
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (26 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 27/34] Improve block job rate limiting for small bandwidth values Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 29/34] blockdev: Fix regression with the default naming of throttling groups Kevin Wolf
` (6 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Reda Sallahi <fullmanet@gmail.com>
Commit "cdeaf1f vmdk: add bdrv_co_write_zeroes" causes a regression on
writes. It writes metadata after every write instead of doing it only once
for each cluster.
vmdk_pwritev() writes metadata whenever m_data is set as valid so this patch
sets m_data as valid only when we have a new cluster which hasn't been
allocated before or a zero grain.
Signed-off-by: Reda Sallahi <fullmanet@gmail.com>
Message-id: 20160707084249.29084-1-fullmanet@gmail.com
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/vmdk.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index c9914f7..46d474e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1202,13 +1202,6 @@ static int get_cluster_offset(BlockDriverState *bs,
l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
cluster_sector = le32_to_cpu(l2_table[l2_index]);
- if (m_data) {
- m_data->valid = 1;
- m_data->l1_index = l1_index;
- m_data->l2_index = l2_index;
- m_data->l2_offset = l2_offset;
- m_data->l2_cache_entry = &l2_table[l2_index];
- }
if (extent->has_zero_grain && cluster_sector == VMDK_GTE_ZEROED) {
zeroed = true;
}
@@ -1231,6 +1224,13 @@ static int get_cluster_offset(BlockDriverState *bs,
if (ret) {
return ret;
}
+ if (m_data) {
+ m_data->valid = 1;
+ m_data->l1_index = l1_index;
+ m_data->l2_index = l2_index;
+ m_data->l2_offset = l2_offset;
+ m_data->l2_cache_entry = &l2_table[l2_index];
+ }
}
*cluster_offset = cluster_sector << BDRV_SECTOR_BITS;
return VMDK_OK;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 29/34] blockdev: Fix regression with the default naming of throttling groups
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (27 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 28/34] vmdk: fix metadata write regression Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 30/34] qemu-iotests: Test " Kevin Wolf
` (5 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
When I/O limits are set for a block device, the name of the throttling
group is taken from the BlockBackend if the user doesn't specify one.
Commit efaa7c4eeb7490c6f37f3 moved the naming of the BlockBackend in
blockdev_init() to the end of the function, after I/O limits are set.
The consequence is that the throttling group gets an empty name.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-stable@nongnu.org
Message-id: af5cd58bd2c4b9f6c57f260d9cfe586b9fb7d34d.1467986342.git.berto@igalia.com
[mreitz: Use existing "id" variable instead of new "blk_id"]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index aa23dc2..384ad3b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -512,6 +512,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
writethrough = !qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true);
+ id = qemu_opts_id(opts);
+
qdict_extract_subqdict(bs_opts, &interval_dict, "stats-intervals.");
qdict_array_split(interval_dict, &interval_list);
@@ -616,7 +618,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
/* disk I/O throttling */
if (throttle_enabled(&cfg)) {
if (!throttling_group) {
- throttling_group = blk_name(blk);
+ throttling_group = id;
}
blk_io_limits_enable(blk, throttling_group);
blk_set_io_limits(blk, &cfg);
@@ -625,7 +627,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
blk_set_enable_write_cache(blk, !writethrough);
blk_set_on_error(blk, on_read_error, on_write_error);
- if (!monitor_add_blk(blk, qemu_opts_id(opts), errp)) {
+ if (!monitor_add_blk(blk, id, errp)) {
blk_unref(blk);
blk = NULL;
goto err_no_bs_opts;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 30/34] qemu-iotests: Test naming of throttling groups
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (28 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 29/34] blockdev: Fix regression with the default naming of throttling groups Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 31/34] hmp: use snapshot name to determine whether a snapshot is 'fully available' Kevin Wolf
` (4 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Alberto Garcia <berto@igalia.com>
Throttling groups are named using the 'group' parameter of the
block_set_io_throttle command and the throttling.group command-line
option. If that parameter is unspecified the groups get the name of
the block device.
This patch adds a new test to check the naming of throttling groups.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: d87d02823a6b91609509d8bb18e2f5dbd9a6102c.1467986342.git.berto@igalia.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/093 | 98 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/093.out | 4 +-
2 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index ce8e13c..ffcb271 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -184,5 +184,103 @@ class ThrottleTestCase(iotests.QMPTestCase):
class ThrottleTestCoroutine(ThrottleTestCase):
test_img = "null-co://"
+class ThrottleTestGroupNames(iotests.QMPTestCase):
+ test_img = "null-aio://"
+ max_drives = 3
+
+ def setUp(self):
+ self.vm = iotests.VM()
+ for i in range(0, self.max_drives):
+ self.vm.add_drive(self.test_img, "throttling.iops-total=100")
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+
+ def set_io_throttle(self, device, params):
+ params["device"] = device
+ result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params)
+ self.assert_qmp(result, 'return', {})
+
+ def verify_name(self, device, name):
+ result = self.vm.qmp("query-block")
+ for r in result["return"]:
+ if r["device"] == device:
+ info = r["inserted"]
+ if name:
+ self.assertEqual(info["group"], name)
+ else:
+ self.assertFalse(info.has_key('group'))
+ return
+
+ raise Exception("No group information found for '%s'" % device)
+
+ def test_group_naming(self):
+ params = {"bps": 0,
+ "bps_rd": 0,
+ "bps_wr": 0,
+ "iops": 0,
+ "iops_rd": 0,
+ "iops_wr": 0}
+
+ # Check the drives added using the command line.
+ # The default throttling group name is the device name.
+ for i in range(self.max_drives):
+ devname = "drive%d" % i
+ self.verify_name(devname, devname)
+
+ # Clear throttling settings => the group name is gone.
+ for i in range(self.max_drives):
+ devname = "drive%d" % i
+ self.set_io_throttle(devname, params)
+ self.verify_name(devname, None)
+
+ # Set throttling settings using block_set_io_throttle and
+ # check the default group names.
+ params["iops"] = 10
+ for i in range(self.max_drives):
+ devname = "drive%d" % i
+ self.set_io_throttle(devname, params)
+ self.verify_name(devname, devname)
+
+ # Set a custom group name for each device
+ for i in range(3):
+ devname = "drive%d" % i
+ groupname = "group%d" % i
+ params['group'] = groupname
+ self.set_io_throttle(devname, params)
+ self.verify_name(devname, groupname)
+
+ # Put drive0 in group1 and check that all other devices remain
+ # unchanged
+ params['group'] = 'group1'
+ self.set_io_throttle('drive0', params)
+ self.verify_name('drive0', 'group1')
+ for i in range(1, self.max_drives):
+ devname = "drive%d" % i
+ groupname = "group%d" % i
+ self.verify_name(devname, groupname)
+
+ # Put drive0 in group2 and check that all other devices remain
+ # unchanged
+ params['group'] = 'group2'
+ self.set_io_throttle('drive0', params)
+ self.verify_name('drive0', 'group2')
+ for i in range(1, self.max_drives):
+ devname = "drive%d" % i
+ groupname = "group%d" % i
+ self.verify_name(devname, groupname)
+
+ # Clear throttling settings from drive0 check that all other
+ # devices remain unchanged
+ params["iops"] = 0
+ self.set_io_throttle('drive0', params)
+ self.verify_name('drive0', None)
+ for i in range(1, self.max_drives):
+ devname = "drive%d" % i
+ groupname = "group%d" % i
+ self.verify_name(devname, groupname)
+
+
if __name__ == '__main__':
iotests.main(supported_fmts=["raw"])
diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out
index 89968f3..914e373 100644
--- a/tests/qemu-iotests/093.out
+++ b/tests/qemu-iotests/093.out
@@ -1,5 +1,5 @@
-....
+.....
----------------------------------------------------------------------
-Ran 4 tests
+Ran 5 tests
OK
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 31/34] hmp: use snapshot name to determine whether a snapshot is 'fully available'
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (29 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 30/34] qemu-iotests: Test " Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 32/34] hmp: show all of snapshot info on every block dev in output of 'info snapshots' Kevin Wolf
` (3 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Lin Ma <lma@suse.com>
Currently qemu uses snapshot id to determine whether a snapshot is fully
available, It causes incorrect output in some scenario.
For instance:
(qemu) info block
drive_image1 (#block113): /opt/vms/SLES12-SP1-JeOS-x86_64-GM/disk0.qcow2
(qcow2)
Cache mode: writeback
drive_image2 (#block349): /opt/vms/SLES12-SP1-JeOS-x86_64-GM/disk1.qcow2
(qcow2)
Cache mode: writeback
(qemu)
(qemu) info snapshots
There is no snapshot available.
(qemu)
(qemu) snapshot_blkdev_internal drive_image1 snap1
(qemu)
(qemu) info snapshots
There is no suitable snapshot available
(qemu)
(qemu) savevm checkpoint-1
(qemu)
(qemu) info snapshots
ID TAG VM SIZE DATE VM CLOCK
1 snap1 0 2016-05-22 16:57:31 00:01:30.567
(qemu)
$ qemu-img snapshot -l disk0.qcow2
Snapshot list:
ID TAG VM SIZE DATE VM CLOCK
1 snap1 0 2016-05-22 16:57:31 00:01:30.567
2 checkpoint-1 165M 2016-05-22 16:58:07 00:02:06.813
$ qemu-img snapshot -l disk1.qcow2
Snapshot list:
ID TAG VM SIZE DATE VM CLOCK
1 checkpoint-1 0 2016-05-22 16:58:07 00:02:06.813
The patch uses snapshot name instead of snapshot id to determine whether a
snapshot is fully available and uses '--' instead of snapshot id in output
because the snapshot id is not guaranteed to be the same on all images.
For instance:
(qemu) info snapshots
List of snapshots present on all disks:
ID TAG VM SIZE DATE VM CLOCK
-- checkpoint-1 165M 2016-05-22 16:58:07 00:02:06.813
Signed-off-by: Lin Ma <lma@suse.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 1467869164-26688-2-git-send-email-lma@suse.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
migration/savevm.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 38b85ee..a8f22da 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2230,7 +2230,7 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
available_snapshots = g_new0(int, nb_sns);
total = 0;
for (i = 0; i < nb_sns; i++) {
- if (bdrv_all_find_snapshot(sn_tab[i].id_str, &bs1) == 0) {
+ if (bdrv_all_find_snapshot(sn_tab[i].name, &bs1) == 0) {
available_snapshots[total] = i;
total++;
}
@@ -2241,6 +2241,10 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "\n");
for (i = 0; i < total; i++) {
sn = &sn_tab[available_snapshots[i]];
+ /* The ID is not guaranteed to be the same on all images, so
+ * overwrite it.
+ */
+ pstrcpy(sn->id_str, sizeof(sn->id_str), "--");
bdrv_snapshot_dump((fprintf_function)monitor_printf, mon, sn);
monitor_printf(mon, "\n");
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 32/34] hmp: show all of snapshot info on every block dev in output of 'info snapshots'
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (30 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 31/34] hmp: use snapshot name to determine whether a snapshot is 'fully available' Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 33/34] vvfat: Fix qcow write target driver specification Kevin Wolf
` (2 subsequent siblings)
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Lin Ma <lma@suse.com>
Currently, the output of 'info snapshots' shows fully available snapshots.
It's opaque, hides some snapshot information to users. It's not convenient
if users want to know more about all of snapshot information on every block
device via monitor.
Follow Kevin's and Max's proposals, The patch makes the output more detailed:
(qemu) info snapshots
List of snapshots present on all disks:
ID TAG VM SIZE DATE VM CLOCK
-- checkpoint-1 165M 2016-05-22 16:58:07 00:02:06.813
List of partial (non-loadable) snapshots on 'drive_image1':
ID TAG VM SIZE DATE VM CLOCK
1 snap1 0 2016-05-22 16:57:31 00:01:30.567
Signed-off-by: Lin Ma <lma@suse.com>
Message-id: 1467869164-26688-3-git-send-email-lma@suse.com
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
migration/savevm.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 90 insertions(+), 7 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index a8f22da..33a2911 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2200,12 +2200,31 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs, *bs1;
+ BdrvNextIterator it1;
QEMUSnapshotInfo *sn_tab, *sn;
+ bool no_snapshot = true;
int nb_sns, i;
int total;
- int *available_snapshots;
+ int *global_snapshots;
AioContext *aio_context;
+ typedef struct SnapshotEntry {
+ QEMUSnapshotInfo sn;
+ QTAILQ_ENTRY(SnapshotEntry) next;
+ } SnapshotEntry;
+
+ typedef struct ImageEntry {
+ const char *imagename;
+ QTAILQ_ENTRY(ImageEntry) next;
+ QTAILQ_HEAD(, SnapshotEntry) snapshots;
+ } ImageEntry;
+
+ QTAILQ_HEAD(, ImageEntry) image_list =
+ QTAILQ_HEAD_INITIALIZER(image_list);
+
+ ImageEntry *image_entry, *next_ie;
+ SnapshotEntry *snapshot_entry;
+
bs = bdrv_all_find_vmstate_bs();
if (!bs) {
monitor_printf(mon, "No available block device supports snapshots\n");
@@ -2222,25 +2241,65 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
return;
}
- if (nb_sns == 0) {
+ for (bs1 = bdrv_first(&it1); bs1; bs1 = bdrv_next(&it1)) {
+ int bs1_nb_sns = 0;
+ ImageEntry *ie;
+ SnapshotEntry *se;
+ AioContext *ctx = bdrv_get_aio_context(bs1);
+
+ aio_context_acquire(ctx);
+ if (bdrv_can_snapshot(bs1)) {
+ sn = NULL;
+ bs1_nb_sns = bdrv_snapshot_list(bs1, &sn);
+ if (bs1_nb_sns > 0) {
+ no_snapshot = false;
+ ie = g_new0(ImageEntry, 1);
+ ie->imagename = bdrv_get_device_name(bs1);
+ QTAILQ_INIT(&ie->snapshots);
+ QTAILQ_INSERT_TAIL(&image_list, ie, next);
+ for (i = 0; i < bs1_nb_sns; i++) {
+ se = g_new0(SnapshotEntry, 1);
+ se->sn = sn[i];
+ QTAILQ_INSERT_TAIL(&ie->snapshots, se, next);
+ }
+ }
+ g_free(sn);
+ }
+ aio_context_release(ctx);
+ }
+
+ if (no_snapshot) {
monitor_printf(mon, "There is no snapshot available.\n");
return;
}
- available_snapshots = g_new0(int, nb_sns);
+ global_snapshots = g_new0(int, nb_sns);
total = 0;
for (i = 0; i < nb_sns; i++) {
+ SnapshotEntry *next_sn;
if (bdrv_all_find_snapshot(sn_tab[i].name, &bs1) == 0) {
- available_snapshots[total] = i;
+ global_snapshots[total] = i;
total++;
+ QTAILQ_FOREACH(image_entry, &image_list, next) {
+ QTAILQ_FOREACH_SAFE(snapshot_entry, &image_entry->snapshots,
+ next, next_sn) {
+ if (!strcmp(sn_tab[i].name, snapshot_entry->sn.name)) {
+ QTAILQ_REMOVE(&image_entry->snapshots, snapshot_entry,
+ next);
+ g_free(snapshot_entry);
+ }
+ }
+ }
}
}
+ monitor_printf(mon, "List of snapshots present on all disks:\n");
+
if (total > 0) {
bdrv_snapshot_dump((fprintf_function)monitor_printf, mon, NULL);
monitor_printf(mon, "\n");
for (i = 0; i < total; i++) {
- sn = &sn_tab[available_snapshots[i]];
+ sn = &sn_tab[global_snapshots[i]];
/* The ID is not guaranteed to be the same on all images, so
* overwrite it.
*/
@@ -2249,11 +2308,35 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "\n");
}
} else {
- monitor_printf(mon, "There is no suitable snapshot available\n");
+ monitor_printf(mon, "None\n");
}
+ QTAILQ_FOREACH(image_entry, &image_list, next) {
+ if (QTAILQ_EMPTY(&image_entry->snapshots)) {
+ continue;
+ }
+ monitor_printf(mon,
+ "\nList of partial (non-loadable) snapshots on '%s':\n",
+ image_entry->imagename);
+ bdrv_snapshot_dump((fprintf_function)monitor_printf, mon, NULL);
+ monitor_printf(mon, "\n");
+ QTAILQ_FOREACH(snapshot_entry, &image_entry->snapshots, next) {
+ bdrv_snapshot_dump((fprintf_function)monitor_printf, mon,
+ &snapshot_entry->sn);
+ monitor_printf(mon, "\n");
+ }
+ }
+
+ QTAILQ_FOREACH_SAFE(image_entry, &image_list, next, next_ie) {
+ SnapshotEntry *next_sn;
+ QTAILQ_FOREACH_SAFE(snapshot_entry, &image_entry->snapshots, next,
+ next_sn) {
+ g_free(snapshot_entry);
+ }
+ g_free(image_entry);
+ }
g_free(sn_tab);
- g_free(available_snapshots);
+ g_free(global_snapshots);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 33/34] vvfat: Fix qcow write target driver specification
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (31 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 32/34] hmp: show all of snapshot info on every block dev in output of 'info snapshots' Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 34/34] iotests: Make 157 actually format-agnostic Kevin Wolf
2016-07-14 12:10 ` [Qemu-devel] [PULL v2 00/34] Block layer patches Peter Maydell
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
First, bdrv_open_child() expects all options for the child to be
prefixed by the child's name (and a separating dot). Second,
bdrv_open_child() does not take ownership of the QDict passed to it but
only extracts all options for the child, so if a QDict is created for
the sole purpose of passing it to bdrv_open_child(), it needs to be
freed afterwards.
This patch makes vvfat adhere to both of these rules.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20160711135452.11304-1-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/vvfat.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index c3f24c6..ba2620f 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3018,9 +3018,10 @@ static int enable_write_target(BlockDriverState *bs, Error **errp)
}
options = qdict_new();
- qdict_put(options, "driver", qstring_from_str("qcow"));
+ qdict_put(options, "write-target.driver", qstring_from_str("qcow"));
s->qcow = bdrv_open_child(s->qcow_filename, options, "write-target", bs,
&child_vvfat_qcow, false, errp);
+ QDECREF(options);
if (!s->qcow) {
ret = -EINVAL;
goto err;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* [Qemu-devel] [PULL v2 34/34] iotests: Make 157 actually format-agnostic
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (32 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 33/34] vvfat: Fix qcow write target driver specification Kevin Wolf
@ 2016-07-13 12:50 ` Kevin Wolf
2016-07-14 12:10 ` [Qemu-devel] [PULL v2 00/34] Block layer patches Peter Maydell
34 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2016-07-13 12:50 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
iotest 157 pretends not to care about the image format used, but in fact
it does due to the format name not being filtered in its output. This
patch adds filtering and changes the reference output accordingly.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20160711132246.3152-1-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/157 | 3 ++-
tests/qemu-iotests/157.out | 16 ++++++++--------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/tests/qemu-iotests/157 b/tests/qemu-iotests/157
index 2699168..8d939cb 100755
--- a/tests/qemu-iotests/157
+++ b/tests/qemu-iotests/157
@@ -57,7 +57,8 @@ function do_run_qemu()
function run_qemu()
{
- do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_generated_node_ids
+ do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_imgfmt \
+ | _filter_qemu | _filter_generated_node_ids
}
diff --git a/tests/qemu-iotests/157.out b/tests/qemu-iotests/157.out
index 5aa9c0e..77a9c03 100644
--- a/tests/qemu-iotests/157.out
+++ b/tests/qemu-iotests/157.out
@@ -3,20 +3,20 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
=== Setting WCE with qdev and with manually created BB ===
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback -device virtio-blk,drive=none0
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writeback -device virtio-blk,drive=none0
Cache mode: writeback
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback -device virtio-blk,drive=none0,write-cache=auto
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writeback -device virtio-blk,drive=none0,write-cache=auto
Cache mode: writeback
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback -device virtio-blk,drive=none0,write-cache=on
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writeback -device virtio-blk,drive=none0,write-cache=on
Cache mode: writeback
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writeback -device virtio-blk,drive=none0,write-cache=off
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writeback -device virtio-blk,drive=none0,write-cache=off
Cache mode: writethrough
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device virtio-blk,drive=none0
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writethrough -device virtio-blk,drive=none0
Cache mode: writethrough
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device virtio-blk,drive=none0,write-cache=auto
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writethrough -device virtio-blk,drive=none0,write-cache=auto
Cache mode: writethrough
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device virtio-blk,drive=none0,write-cache=on
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writethrough -device virtio-blk,drive=none0,write-cache=on
Cache mode: writeback
-Testing: -drive if=none,file=TEST_DIR/t.qcow2,driver=qcow2,cache=writethrough -device virtio-blk,drive=none0,write-cache=off
+Testing: -drive if=none,file=TEST_DIR/t.IMGFMT,driver=IMGFMT,cache=writethrough -device virtio-blk,drive=none0,write-cache=off
Cache mode: writethrough
*** done
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [Qemu-devel] [PULL v2 00/34] Block layer patches
2016-07-13 12:50 [Qemu-devel] [PULL v2 00/34] Block layer patches Kevin Wolf
` (33 preceding siblings ...)
2016-07-13 12:50 ` [Qemu-devel] [PULL v2 34/34] iotests: Make 157 actually format-agnostic Kevin Wolf
@ 2016-07-14 12:10 ` Peter Maydell
34 siblings, 0 replies; 37+ messages in thread
From: Peter Maydell @ 2016-07-14 12:10 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Qemu-block, QEMU Developers
On 13 July 2016 at 13:50, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit ca3d87d4c84032f19478010b5604cac88b045c25:
>
> Merge remote-tracking branch 'remotes/armbru/tags/pull-include-2016-07-12' into staging (2016-07-12 16:04:36 +0100)
>
> are available in the git repository at:
>
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 543d7a42baf39c09db754ba9eca1d386e5958110:
>
> Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2016-07-13' into queue-block (2016-07-13 13:45:55 +0200)
>
> ----------------------------------------------------------------
>
> Block layer patches
>
> ----------------------------------------------------------------
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 37+ messages in thread