From 29338003c93c3e81ea00f7b45baeee379fa83aa5 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:28:52 +0300 Subject: [PATCH 01/34] stream: Fix prototype of stream_start() '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 Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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 47b9aac71b..2a27a1946b 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); /** From 9df229c3caf6559a37c8760ef6e1485e66bbae41 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:28:53 +0300 Subject: [PATCH 02/34] blockjob: Update description of the 'id' field 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 Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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 8d96049555..6e3dbd257b 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 205da9df4e..ce0e27c124 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 f7f5687cf4..97b86f109f 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 d08652aaa5..6586c9fa62 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" From ffb1f10cd1118393627e1bd2dad0a68152d2e539 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:28:54 +0300 Subject: [PATCH 03/34] blockjob: Add block_job_get() 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 Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- blockjob.c | 13 +++++++++++++ include/block/blockjob.h | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/blockjob.c b/blockjob.c index ce0e27c124..ca2291b5c5 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 97b86f109f..934bf1ce2d 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -211,6 +211,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. From 3ddf3efefa364505ee44582873612dd8f6abb838 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:28:55 +0300 Subject: [PATCH 04/34] block: Use block_job_get() in find_block_job() 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 Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- blockdev.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/blockdev.c b/blockdev.c index 0f8065c4a5..e649521e0c 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; + + assert(id != NULL); *aio_context = NULL; - blk = blk_by_name(device); - if (!blk) { - goto notfound; + job = block_job_get(id); + + if (!job) { + error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE, + "Block job '%s' not found", id); + return NULL; } - *aio_context = blk_get_aio_context(blk); + *aio_context = blk_get_aio_context(job->blk); aio_context_acquire(*aio_context); - if (!blk_is_available(blk)) { - goto notfound; - } - bs = blk_bs(blk); - - if (!bs->job) { - goto notfound; - } - - return bs->job; - -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) From 7f0317cfc8da620cdb38cb5cfec5f82b8dd05403 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:28:56 +0300 Subject: [PATCH 05/34] blockjob: Add 'job_id' parameter to block_job_create() 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 Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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 f87f8d539b..19c587c926 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 379efb7c92..137bb03297 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 6e3dbd257b..08d88cad8a 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 c0efbda34e..e4319d37e5 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 ca2291b5c5..511c0db123 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 934bf1ce2d..4ddb4ae2e1 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 d3030f1566..50e232a709 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; From 71aa98678c2b5616de5453d55e12f8ea810fbefb Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:28:57 +0300 Subject: [PATCH 06/34] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' 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 Reviewed-by: Kevin Wolf Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- 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 08d88cad8a..702a686145 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 e649521e0c..2008690045 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 0cf5baa069..edf9c7d5c6 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 2a27a1946b..ad44fb959e 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 ac8f5f61d4..18cdd5b85d 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 6937e83cbd..6a2ec8f94f 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 From 70559d499c84b9c7b1874821f970a15d52460d64 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:28:58 +0300 Subject: [PATCH 07/34] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup' 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 Reviewed-by: Kevin Wolf Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- 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 19c587c926..dce66142f3 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 2008690045..920d9878aa 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 edf9c7d5c6..62eca7077a 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 ad44fb959e..a0b9f926b5 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 18cdd5b85d..ee44ce4326 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 6a2ec8f94f..a032089d36 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) From 2323322ed060749b52864836f6fcb1a906baf95d Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:28:59 +0300 Subject: [PATCH 08/34] stream: Add 'job-id' parameter to 'block-stream' 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 Reviewed-by: Kevin Wolf Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- 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 e4319d37e5..54c8cd8f57 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 920d9878aa..d6f1d4d575 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 62eca7077a..3ca79c3ea3 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 a0b9f926b5..db364bb4c9 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 ee44ce4326..94f4733e5e 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 a032089d36..d61ea2047c 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) From fd62c609edb32ddaafbe84c466dd21603577a46d Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:29:00 +0300 Subject: [PATCH 09/34] commit: Add 'job-id' parameter to 'block-commit' 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 Reviewed-by: Kevin Wolf Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- 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 137bb03297..23368fa65b 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 702a686145..705fbc0052 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 d6f1d4d575..aa23dc23ac 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 db364bb4c9..805414619d 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 94f4733e5e..1e4b16d882 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 ea5970bdd3..a162f34130 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 d61ea2047c..c46c65ce2d 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 From a5d5a3bdbd4bd2ffb27f825dd8a39e1fbaf11ad3 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:29:01 +0300 Subject: [PATCH 10/34] qemu-img: Set the ID of the block job in img_commit() 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 Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- blockjob.c | 6 ------ qemu-img.c | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/blockjob.c b/blockjob.c index 511c0db123..3b9cec7440 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 a162f34130..969edce186 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; From 6aae5be6a77e36968c0f6cc2ab147918904087b7 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 5 Jul 2016 17:29:02 +0300 Subject: [PATCH 11/34] blockjob: Update description of the 'device' field in the QMP API 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 Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- 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 fa7574d671..7967ec4c5a 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 1e4b16d882..3f77dac07d 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 # From 761d1ddf25988eeb110a612b0a42945fa1b561a5 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 22 Jun 2016 20:53:19 +0800 Subject: [PATCH 12/34] osdep: Introduce qemu_dup And use it in qemu_dup_flags. Signed-off-by: Fam Zheng Reviewed-by: Eric Blake Reviewed-by: John Snow Signed-off-by: Kevin Wolf --- 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 e63da2831a..7361006c50 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 ff004e8074..06fb1cfda6 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); From 5af7045bd0d45cff5c8eb0a3b14b900d9bd24998 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 22 Jun 2016 20:53:20 +0800 Subject: [PATCH 13/34] raw-posix: Use qemu_dup Signed-off-by: Fam Zheng Reviewed-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: John Snow Signed-off-by: Kevin Wolf --- 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 c979ac3fd1..d1c3bd8e47 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) { From 7d9c8581370738fb3877a724ac061e6a8cd00121 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Jul 2016 19:09:59 +0200 Subject: [PATCH 14/34] coroutine: use QSIMPLEQ instead of QTAILQ 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 Signed-off-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- 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 305fe76c29..63ae7fee75 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 42d6838401..581a7f5140 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 da37ca7f95..cf53693fe2 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 5816702cc5..b7cb636e4d 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; } From 7e70cdba9f220bef3f3481c663c066c2b80469aa Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Jul 2016 19:10:00 +0200 Subject: [PATCH 15/34] test-coroutine: prepare for the next patch 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 Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- 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 215b92e636..51711744c8 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); } /* From 0b8b8753e4d94901627b3e86431230f2319215c4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Jul 2016 19:10:01 +0200 Subject: [PATCH 16/34] coroutine: move entry argument to qemu_coroutine_create 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 Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- 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 823ff1d7ab..67894e0719 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 dce66142f3..2c0532314f 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 bbaa33fdd8..fb29283f80 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 70650e4be1..3368c8c98d 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 a862f6577b..0d7b801107 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 23368fa65b..8b534d7c95 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 16f7778a50..406c1e6357 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 708690898f..2887394633 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 434cb37107..cf1e9e7f66 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 7df8651581..5c104bd3cd 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 705fbc0052..71e5ad4377 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 420bce89f3..4cc408d206 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 15d6832c4c..8602a44211 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 ac849bd47c..0c7b75bc76 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 a5ea19b0b6..a6bca735e5 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 f619d82c6e..426f3cb447 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 ef5d044ab9..e739c56f08 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 06928ed939..bcbb0e4223 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 54c8cd8f57..2e7c6547d2 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 d73f4314ba..c9914f718f 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 3b9cec7440..6816b78fcb 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 9acff9293c..b6b02b46a9 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 9b1151b60e..89018de6bf 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 63ae7fee75..ac8d4c9cc8 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 3fa7cfe574..470f600bbc 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 692eb179b3..923c4651ca 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 a56013662d..c4e019305c 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 a677e266ff..fbc82de932 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 09e879f872..40fe2ebf7d 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 50e232a709..d049cba8a3 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 51711744c8..ee5e06d327 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 b0e1f3290f..8dbf66a44a 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 03ba0b02a4..6fba913529 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 91b9357d4a..44a8969a69 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 cf53693fe2..22aa9abb30 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 6966831d37..25de3ed3dd 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 b7cb636e4d..89f21a9cec 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); From 8daea510951dd309a44cea8de415c685c43851cf Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 21 Jun 2016 20:46:05 +0200 Subject: [PATCH 17/34] block/qdev: Allow node name for drive properties 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 Reviewed-by: Max Reitz --- 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 65d9fa9f53..ab1bc5e945 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 4d605b8a6a..78038a2470 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; From f6166a06ffdb1cd5dd80adf2d82c35c3bda88239 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 23 Jun 2016 15:12:35 +0200 Subject: [PATCH 18/34] block/qdev: Allow configuring WCE with qdev properties 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 Reviewed-by: Max Reitz --- 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 97a59d4fa2..396b0d5de1 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 280d54d8eb..2ded2475ee 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 ae86e944ea..ecd8ea34b6 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 d07b44eed5..33619f4022 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 36f8a85a70..8c26a4e179 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 78038a2470..c607f7606d 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 87c87ed92a..245ac99148 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 */ From 1e8fb7f1ee1ba902ab06cb8d54eca006c3b45f42 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 29 Jun 2016 17:38:57 +0200 Subject: [PATCH 19/34] commit: Fix use of error handling policy 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 Reviewed-by: Max Reitz --- block/commit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block/commit.c b/block/commit.c index 8b534d7c95..5d11eb6103 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; From 8c39825218227c0d63709708602d34c4355bad56 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 29 Jun 2016 17:41:35 +0200 Subject: [PATCH 20/34] block/qdev: Allow configuring rerror/werror with qdev properties 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 Reviewed-by: Max Reitz --- 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 0d7b801107..f9cea1b304 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 6816b78fcb..a5ba3bee52 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 396b0d5de1..8dc9d84a39 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 ecd8ea34b6..357ff9081e 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 3c20c8e4b2..14e544ab17 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 33619f4022..67c76bfcd6 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 8c26a4e179..8dbfc10b78 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 245ac99148..df9d207d81 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 034b75acc5..2a9d2f90e6 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 3f77dac07d..0d30187691 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: From 62ed9fa991488b5ed6b8a2cbcb64182d7a5378f5 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 30 Jun 2016 15:29:35 +0200 Subject: [PATCH 21/34] qemu-iotests: Test setting WCE with qdev Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- 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 0000000000..2699168f1d --- /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 . +# + +# 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 0000000000..5aa9c0e2e4 --- /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 1c6fcb6018..3a3973e963 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 From 35fedb7b0e5766fc62a2c0bdce023b50dc5e3ffc Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 30 Jun 2016 15:52:37 +0200 Subject: [PATCH 22/34] block: Remove BB options from blockdev-add 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 Reviewed-by: Max Reitz --- 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 0d30187691..3444a9bc3e 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': { From bcf23482ae00e040dbef46c44ff914bf788a0937 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 15 Jun 2016 17:36:29 +0200 Subject: [PATCH 23/34] qemu-img: Use strerror() for generic resize error Emitting the plain error number is not very helpful. Use strerror() instead. Signed-off-by: Max Reitz Message-id: 20160615153630.2116-2-mreitz@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- qemu-img.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index 969edce186..2e40e1fc84 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: From 84c26520d3c1c9ff4a10455748139463278816d5 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 15 Jun 2016 17:36:30 +0200 Subject: [PATCH 24/34] qcow2: Avoid making the L1 table too big 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 Message-id: 20160615153630.2116-3-mreitz@redhat.com Reviewed-by: Eric Blake [mreitz: Added QEMU_BUILD_BUG_ON()] Signed-off-by: Max Reitz --- 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 6b92ce9429..00c16dcba5 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; } From a367467995d0528fe591d87ca2e437c7b7d7951b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 20 Jun 2016 16:26:22 +0200 Subject: [PATCH 25/34] qemu-io: Use correct range limitations 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 --- 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 40fe2ebf7d..6e29edb1fd 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; } From c834cba90521576224c30b15ebb4d6aeab7b42c4 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 20 Jun 2016 16:26:23 +0200 Subject: [PATCH 26/34] qcow2: Fix qcow2_get_cluster_offset() 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 Message-id: 20160620142623.24471-3-mreitz@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- 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 00c16dcba5..f94183529c 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; From f14a39ccb922ee123741ae2cf70a10eef9a650fc Mon Sep 17 00:00:00 2001 From: Sascha Silbe Date: Tue, 28 Jun 2016 17:28:41 +0200 Subject: [PATCH 27/34] Improve block job rate limiting for small bandwidth values 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 Message-id: 1467127721-9564-2-git-send-email-silbe@linux.vnet.ibm.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- 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 5d11eb6103..553e18da52 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 71e5ad4377..b1e633ecad 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 2e7c6547d2..31874817c2 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 1e3cb13b28..8da1232574 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 From 524089bce43fd1cd3daaca979872451efa2cf7c6 Mon Sep 17 00:00:00 2001 From: Reda Sallahi Date: Thu, 7 Jul 2016 10:42:49 +0200 Subject: [PATCH 28/34] vmdk: fix metadata write regression 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 Message-id: 20160707084249.29084-1-fullmanet@gmail.com Reviewed-by: Fam Zheng Signed-off-by: Max Reitz --- block/vmdk.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index c9914f718f..46d474e442 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; From ff356ee4da0a2e691a7ab0165d47279f868977c4 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 8 Jul 2016 17:03:00 +0300 Subject: [PATCH 29/34] blockdev: Fix regression with the default naming of throttling groups 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 Reported-by: Stefan Hajnoczi Cc: Max Reitz 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 --- blockdev.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index aa23dc23ac..384ad3bba6 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; From 435d5ee6cd3fd7aa0f16748c03648ed97b493c2b Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 8 Jul 2016 17:03:01 +0300 Subject: [PATCH 30/34] qemu-iotests: Test naming of throttling groups 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 Message-id: d87d02823a6b91609509d8bb18e2f5dbd9a6102c.1467986342.git.berto@igalia.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- 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 ce8e13cb49..ffcb271b36 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 89968f35d7..914e3737bd 100644 --- a/tests/qemu-iotests/093.out +++ b/tests/qemu-iotests/093.out @@ -1,5 +1,5 @@ -.... +..... ---------------------------------------------------------------------- -Ran 4 tests +Ran 5 tests OK From 3a1ee711904f12f601fffca31a1050d39f833487 Mon Sep 17 00:00:00 2001 From: Lin Ma Date: Thu, 7 Jul 2016 13:26:03 +0800 Subject: [PATCH 31/34] hmp: use snapshot name to determine whether a snapshot is 'fully available' 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 Reviewed-by: Max Reitz Message-id: 1467869164-26688-2-git-send-email-lma@suse.com Signed-off-by: Max Reitz --- migration/savevm.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/migration/savevm.c b/migration/savevm.c index 38b85ee77b..a8f22da731 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"); } From 0c204cc810af90ca2a449d08d9d39ec8b760d9b4 Mon Sep 17 00:00:00 2001 From: Lin Ma Date: Thu, 7 Jul 2016 13:26:04 +0800 Subject: [PATCH 32/34] hmp: show all of snapshot info on every block dev in output of 'info snapshots' 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 Message-id: 1467869164-26688-3-git-send-email-lma@suse.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- migration/savevm.c | 97 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 90 insertions(+), 7 deletions(-) diff --git a/migration/savevm.c b/migration/savevm.c index a8f22da731..33a2911ec2 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); } From c4b48bfdc59caf0a69b7e0a40c9ea6d3d7848bc3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 11 Jul 2016 15:54:52 +0200 Subject: [PATCH 33/34] vvfat: Fix qcow write target driver specification 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 Message-id: 20160711135452.11304-1-mreitz@redhat.com Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Signed-off-by: Max Reitz --- block/vvfat.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/vvfat.c b/block/vvfat.c index c3f24c6eb0..ba2620f3c2 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; From 42190dcc70b34d59c81d72f1f8ff884aa27dd851 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 11 Jul 2016 15:22:46 +0200 Subject: [PATCH 34/34] iotests: Make 157 actually format-agnostic 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 Message-id: 20160711132246.3152-1-mreitz@redhat.com Reviewed-by: John Snow Signed-off-by: Max Reitz --- 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 2699168f1d..8d939cb747 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 5aa9c0e2e4..77a9c03d2c 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