From fd7f8c65377ee918479e43b38d44f54f13aa6548 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 25 Apr 2012 16:51:00 +0100 Subject: [PATCH 1/6] block: use Error mechanism instead of -errno for block_job_create() The block job API uses -errno return values internally and we convert these to Error in the QMP functions. This is ugly because the Error should be created at the point where we still have all the relevant information. More importantly, it is hard to add new error cases to this case since we quickly run out of -errno values without losing information. Go ahead and use Error directly and don't convert later. Signed-off-by: Stefan Hajnoczi Acked-by: Kevin Wolf Signed-off-by: Luiz Capitulino --- block.c | 4 +++- block/stream.c | 11 +++++------ block_int.h | 11 +++++++---- blockdev.c | 16 +++++----------- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/block.c b/block.c index fe74dddb13..2b72a0f3b0 100644 --- a/block.c +++ b/block.c @@ -4083,11 +4083,13 @@ out: } void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs, - BlockDriverCompletionFunc *cb, void *opaque) + BlockDriverCompletionFunc *cb, void *opaque, + Error **errp) { BlockJob *job; if (bs->job || bdrv_in_use(bs)) { + error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); return NULL; } bdrv_set_in_use(bs, 1); diff --git a/block/stream.c b/block/stream.c index 0efe1adfd5..7002dc8573 100644 --- a/block/stream.c +++ b/block/stream.c @@ -280,16 +280,16 @@ static BlockJobType stream_job_type = { .set_speed = stream_set_speed, }; -int stream_start(BlockDriverState *bs, BlockDriverState *base, - const char *base_id, BlockDriverCompletionFunc *cb, - void *opaque) +void stream_start(BlockDriverState *bs, BlockDriverState *base, + const char *base_id, BlockDriverCompletionFunc *cb, + void *opaque, Error **errp) { StreamBlockJob *s; Coroutine *co; - s = block_job_create(&stream_job_type, bs, cb, opaque); + s = block_job_create(&stream_job_type, bs, cb, opaque, errp); if (!s) { - return -EBUSY; /* bs must already be in use */ + return; } s->base = base; @@ -300,5 +300,4 @@ int stream_start(BlockDriverState *bs, BlockDriverState *base, co = qemu_coroutine_create(stream_run); trace_stream_start(bs, base, s, co, opaque); qemu_coroutine_enter(co, s); - return 0; } diff --git a/block_int.h b/block_int.h index 0acb49f100..e70a33e0c2 100644 --- a/block_int.h +++ b/block_int.h @@ -346,6 +346,7 @@ int is_windows_drive(const char *filename); * @bs: The block * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. + * @errp: Error object. * * Create a new long-running block device job and return it. The job * will call @cb asynchronously when the job completes. Note that @@ -357,7 +358,8 @@ int is_windows_drive(const char *filename); * called from a wrapper that is specific to the job type. */ void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs, - BlockDriverCompletionFunc *cb, void *opaque); + BlockDriverCompletionFunc *cb, void *opaque, + Error **errp); /** * block_job_complete: @@ -417,6 +419,7 @@ void block_job_cancel_sync(BlockJob *job); * backing file if the job completes. Ignored if @base is %NULL. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. + * @errp: Error object. * * Start a streaming operation on @bs. Clusters that are unallocated * in @bs, but allocated in any image between @base and @bs (both @@ -424,8 +427,8 @@ void block_job_cancel_sync(BlockJob *job); * streaming job, the backing file of @bs will be changed to * @base_id in the written image and to @base in the live BlockDriverState. */ -int stream_start(BlockDriverState *bs, BlockDriverState *base, - const char *base_id, BlockDriverCompletionFunc *cb, - void *opaque); +void stream_start(BlockDriverState *bs, BlockDriverState *base, + const char *base_id, BlockDriverCompletionFunc *cb, + void *opaque, Error **errp); #endif /* BLOCK_INT_H */ diff --git a/blockdev.c b/blockdev.c index 0c2440e249..a41147749a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1095,7 +1095,7 @@ void qmp_block_stream(const char *device, bool has_base, { BlockDriverState *bs; BlockDriverState *base_bs = NULL; - int ret; + Error *local_err = NULL; bs = bdrv_find(device); if (!bs) { @@ -1111,16 +1111,10 @@ void qmp_block_stream(const char *device, bool has_base, } } - ret = stream_start(bs, base_bs, base, block_stream_cb, bs); - if (ret < 0) { - switch (ret) { - case -EBUSY: - error_set(errp, QERR_DEVICE_IN_USE, device); - return; - default: - error_set(errp, QERR_NOT_SUPPORTED); - return; - } + stream_start(bs, base_bs, base, block_stream_cb, bs, &local_err); + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + return; } /* Grab a reference so hotplug does not delete the BlockDriverState from From 9e6636c72d8d6f0605e23ed820c8487686882b12 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 25 Apr 2012 16:51:01 +0100 Subject: [PATCH 2/6] block: use Error mechanism instead of -errno for block_job_set_speed() There are at least two different errors that can occur in block_job_set_speed(): the job might not support setting speeds or the value might be invalid. Use the Error mechanism to report the error where it occurs. Signed-off-by: Stefan Hajnoczi Acked-by: Kevin Wolf Signed-off-by: Luiz Capitulino --- block.c | 17 ++++++++++------- block/stream.c | 6 +++--- block_int.h | 5 +++-- blockdev.c | 4 +--- qapi-schema.json | 1 + 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/block.c b/block.c index 2b72a0f3b0..dc02736d7e 100644 --- a/block.c +++ b/block.c @@ -4114,18 +4114,21 @@ void block_job_complete(BlockJob *job, int ret) bdrv_set_in_use(bs, 0); } -int block_job_set_speed(BlockJob *job, int64_t value) +void block_job_set_speed(BlockJob *job, int64_t value, Error **errp) { - int rc; + Error *local_err = NULL; if (!job->job_type->set_speed) { - return -ENOTSUP; + error_set(errp, QERR_NOT_SUPPORTED); + return; } - rc = job->job_type->set_speed(job, value); - if (rc == 0) { - job->speed = value; + job->job_type->set_speed(job, value, &local_err); + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + return; } - return rc; + + job->speed = value; } void block_job_cancel(BlockJob *job) diff --git a/block/stream.c b/block/stream.c index 7002dc8573..06bc70a9b4 100644 --- a/block/stream.c +++ b/block/stream.c @@ -263,15 +263,15 @@ retry: block_job_complete(&s->common, ret); } -static int stream_set_speed(BlockJob *job, int64_t value) +static void stream_set_speed(BlockJob *job, int64_t value, Error **errp) { StreamBlockJob *s = container_of(job, StreamBlockJob, common); if (value < 0) { - return -EINVAL; + error_set(errp, QERR_INVALID_PARAMETER, "value"); + return; } ratelimit_set_speed(&s->limit, value / BDRV_SECTOR_SIZE); - return 0; } static BlockJobType stream_job_type = { diff --git a/block_int.h b/block_int.h index e70a33e0c2..e042676be3 100644 --- a/block_int.h +++ b/block_int.h @@ -79,7 +79,7 @@ typedef struct BlockJobType { const char *job_type; /** Optional callback for job types that support setting a speed limit */ - int (*set_speed)(BlockJob *job, int64_t value); + void (*set_speed)(BlockJob *job, int64_t value, Error **errp); } BlockJobType; /** @@ -375,11 +375,12 @@ void block_job_complete(BlockJob *job, int ret); * block_job_set_speed: * @job: The job to set the speed for. * @speed: The new value + * @errp: Error object. * * Set a rate-limiting parameter for the job; the actual meaning may * vary depending on the job type. */ -int block_job_set_speed(BlockJob *job, int64_t value); +void block_job_set_speed(BlockJob *job, int64_t value, Error **errp); /** * block_job_cancel: diff --git a/blockdev.c b/blockdev.c index a41147749a..70733308c9 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1145,9 +1145,7 @@ void qmp_block_job_set_speed(const char *device, int64_t value, Error **errp) return; } - if (block_job_set_speed(job, value) < 0) { - error_set(errp, QERR_NOT_SUPPORTED); - } + block_job_set_speed(job, value, errp); } void qmp_block_job_cancel(const char *device, Error **errp) diff --git a/qapi-schema.json b/qapi-schema.json index 64998959db..49f1e16bd7 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1596,6 +1596,7 @@ # # Returns: Nothing on success # If the job type does not support throttling, NotSupported +# If the speed value is invalid, InvalidParameter # If streaming is not active on this device, DeviceNotActive # # Since: 1.1 From 882ec7ce531091bc0f3ffc6ac71943cf383f86a6 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 25 Apr 2012 16:51:02 +0100 Subject: [PATCH 3/6] block: change block-job-set-speed argument from 'value' to 'speed' Signed-off-by: Stefan Hajnoczi Acked-by: Kevin Wolf Signed-off-by: Luiz Capitulino --- block.c | 6 +++--- block/stream.c | 8 ++++---- block_int.h | 4 ++-- blockdev.c | 4 ++-- hmp-commands.hx | 4 ++-- qapi-schema.json | 4 ++-- qmp-commands.hx | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index dc02736d7e..1ab6e525c2 100644 --- a/block.c +++ b/block.c @@ -4114,7 +4114,7 @@ void block_job_complete(BlockJob *job, int ret) bdrv_set_in_use(bs, 0); } -void block_job_set_speed(BlockJob *job, int64_t value, Error **errp) +void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) { Error *local_err = NULL; @@ -4122,13 +4122,13 @@ void block_job_set_speed(BlockJob *job, int64_t value, Error **errp) error_set(errp, QERR_NOT_SUPPORTED); return; } - job->job_type->set_speed(job, value, &local_err); + job->job_type->set_speed(job, speed, &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); return; } - job->speed = value; + job->speed = speed; } void block_job_cancel(BlockJob *job) diff --git a/block/stream.c b/block/stream.c index 06bc70a9b4..b66242a7b8 100644 --- a/block/stream.c +++ b/block/stream.c @@ -263,15 +263,15 @@ retry: block_job_complete(&s->common, ret); } -static void stream_set_speed(BlockJob *job, int64_t value, Error **errp) +static void stream_set_speed(BlockJob *job, int64_t speed, Error **errp) { StreamBlockJob *s = container_of(job, StreamBlockJob, common); - if (value < 0) { - error_set(errp, QERR_INVALID_PARAMETER, "value"); + if (speed < 0) { + error_set(errp, QERR_INVALID_PARAMETER, "speed"); return; } - ratelimit_set_speed(&s->limit, value / BDRV_SECTOR_SIZE); + ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE); } static BlockJobType stream_job_type = { diff --git a/block_int.h b/block_int.h index e042676be3..624b2e634c 100644 --- a/block_int.h +++ b/block_int.h @@ -79,7 +79,7 @@ typedef struct BlockJobType { const char *job_type; /** Optional callback for job types that support setting a speed limit */ - void (*set_speed)(BlockJob *job, int64_t value, Error **errp); + void (*set_speed)(BlockJob *job, int64_t speed, Error **errp); } BlockJobType; /** @@ -380,7 +380,7 @@ void block_job_complete(BlockJob *job, int ret); * Set a rate-limiting parameter for the job; the actual meaning may * vary depending on the job type. */ -void block_job_set_speed(BlockJob *job, int64_t value, Error **errp); +void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp); /** * block_job_cancel: diff --git a/blockdev.c b/blockdev.c index 70733308c9..80b62c3b29 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1136,7 +1136,7 @@ static BlockJob *find_block_job(const char *device) return bs->job; } -void qmp_block_job_set_speed(const char *device, int64_t value, Error **errp) +void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp) { BlockJob *job = find_block_job(device); @@ -1145,7 +1145,7 @@ void qmp_block_job_set_speed(const char *device, int64_t value, Error **errp) return; } - block_job_set_speed(job, value, errp); + block_job_set_speed(job, speed, errp); } void qmp_block_job_cancel(const char *device, Error **errp) diff --git a/hmp-commands.hx b/hmp-commands.hx index 461fa597d4..8a929f0967 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -85,8 +85,8 @@ ETEXI { .name = "block_job_set_speed", - .args_type = "device:B,value:o", - .params = "device value", + .args_type = "device:B,speed:o", + .params = "device speed", .help = "set maximum speed for a background block operation", .mhandler.cmd = hmp_block_job_set_speed, }, diff --git a/qapi-schema.json b/qapi-schema.json index 49f1e16bd7..d56fcb639b 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1592,7 +1592,7 @@ # # @device: the device name # -# @value: the maximum speed, in bytes per second +# @speed: the maximum speed, in bytes per second # # Returns: Nothing on success # If the job type does not support throttling, NotSupported @@ -1602,7 +1602,7 @@ # Since: 1.1 ## { 'command': 'block-job-set-speed', - 'data': { 'device': 'str', 'value': 'int' } } + 'data': { 'device': 'str', 'speed': 'int' } } ## # @block-job-cancel: diff --git a/qmp-commands.hx b/qmp-commands.hx index f97233223d..b07ed59ccc 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -694,7 +694,7 @@ EQMP { .name = "block-job-set-speed", - .args_type = "device:B,value:o", + .args_type = "device:B,speed:o", .mhandler.cmd_new = qmp_marshal_input_block_job_set_speed, }, From c83c66c3b58893a4dc056e272822beb88fe9ec7f Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 25 Apr 2012 16:51:03 +0100 Subject: [PATCH 4/6] block: add 'speed' optional parameter to block-stream Allow streaming operations to be started with an initial speed limit. This eliminates the window of time between starting streaming and issuing block-job-set-speed. Users should use the new optional 'speed' parameter instead so that speed limits are in effect immediately when the job starts. Signed-off-by: Stefan Hajnoczi Acked-by: Kevin Wolf Signed-off-by: Luiz Capitulino --- block.c | 18 ++++++++++++++++-- block/stream.c | 5 +++-- block_int.h | 9 ++++++--- blockdev.c | 6 ++++-- hmp-commands.hx | 4 ++-- hmp.c | 4 +++- qapi-schema.json | 9 +++++++-- qmp-commands.hx | 2 +- 8 files changed, 42 insertions(+), 15 deletions(-) diff --git a/block.c b/block.c index 1ab6e525c2..43c794c4d7 100644 --- a/block.c +++ b/block.c @@ -4083,8 +4083,8 @@ out: } void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs, - BlockDriverCompletionFunc *cb, void *opaque, - Error **errp) + int64_t speed, BlockDriverCompletionFunc *cb, + void *opaque, Error **errp) { BlockJob *job; @@ -4100,6 +4100,20 @@ void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs, job->cb = cb; job->opaque = opaque; bs->job = job; + + /* Only set speed when necessary to avoid NotSupported error */ + if (speed != 0) { + Error *local_err = NULL; + + block_job_set_speed(job, speed, &local_err); + if (error_is_set(&local_err)) { + bs->job = NULL; + g_free(job); + bdrv_set_in_use(bs, 0); + error_propagate(errp, local_err); + return NULL; + } + } return job; } diff --git a/block/stream.c b/block/stream.c index b66242a7b8..6724af2764 100644 --- a/block/stream.c +++ b/block/stream.c @@ -281,13 +281,14 @@ static BlockJobType stream_job_type = { }; void stream_start(BlockDriverState *bs, BlockDriverState *base, - const char *base_id, BlockDriverCompletionFunc *cb, + const char *base_id, int64_t speed, + BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { StreamBlockJob *s; Coroutine *co; - s = block_job_create(&stream_job_type, bs, cb, opaque, errp); + s = block_job_create(&stream_job_type, bs, speed, cb, opaque, errp); if (!s) { return; } diff --git a/block_int.h b/block_int.h index 624b2e634c..086832aab9 100644 --- a/block_int.h +++ b/block_int.h @@ -344,6 +344,7 @@ int is_windows_drive(const char *filename); * block_job_create: * @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. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. * @errp: Error object. @@ -358,8 +359,8 @@ int is_windows_drive(const char *filename); * called from a wrapper that is specific to the job type. */ void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs, - BlockDriverCompletionFunc *cb, void *opaque, - Error **errp); + int64_t speed, BlockDriverCompletionFunc *cb, + void *opaque, Error **errp); /** * block_job_complete: @@ -418,6 +419,7 @@ void block_job_cancel_sync(BlockJob *job); * 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. + * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. * @errp: Error object. @@ -429,7 +431,8 @@ void block_job_cancel_sync(BlockJob *job); * @base_id in the written image and to @base in the live BlockDriverState. */ void stream_start(BlockDriverState *bs, BlockDriverState *base, - const char *base_id, BlockDriverCompletionFunc *cb, + const char *base_id, int64_t speed, + BlockDriverCompletionFunc *cb, void *opaque, Error **errp); #endif /* BLOCK_INT_H */ diff --git a/blockdev.c b/blockdev.c index 80b62c3b29..d25ffea926 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1091,7 +1091,8 @@ static void block_stream_cb(void *opaque, int ret) } void qmp_block_stream(const char *device, bool has_base, - const char *base, Error **errp) + const char *base, bool has_speed, + int64_t speed, Error **errp) { BlockDriverState *bs; BlockDriverState *base_bs = NULL; @@ -1111,7 +1112,8 @@ void qmp_block_stream(const char *device, bool has_base, } } - stream_start(bs, base_bs, base, block_stream_cb, bs, &local_err); + stream_start(bs, base_bs, base, has_speed ? speed : 0, + block_stream_cb, bs, &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); return; diff --git a/hmp-commands.hx b/hmp-commands.hx index 8a929f0967..18cb415ac4 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -71,8 +71,8 @@ ETEXI { .name = "block_stream", - .args_type = "device:B,base:s?", - .params = "device [base]", + .args_type = "device:B,speed:o?,base:s?", + .params = "device [speed [base]]", .help = "copy data from a backing file into a block device", .mhandler.cmd = hmp_block_stream, }, diff --git a/hmp.c b/hmp.c index f3e5163f1e..eb96618e1e 100644 --- a/hmp.c +++ b/hmp.c @@ -835,8 +835,10 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict) Error *error = NULL; const char *device = qdict_get_str(qdict, "device"); 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, &error); + qmp_block_stream(device, base != NULL, base, + qdict_haskey(qdict, "speed"), speed, &error); hmp_handle_error(mon, &error); } diff --git a/qapi-schema.json b/qapi-schema.json index d56fcb639b..9193fb9968 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1571,15 +1571,19 @@ # # @base: #optional the common backing file name # +# @speed: #optional the maximum speed, in bytes per second +# # Returns: Nothing on success # If streaming is already active on this device, DeviceInUse # If @device does not exist, DeviceNotFound # If image streaming is not supported by this device, NotSupported # If @base does not exist, BaseNotFound +# If @speed is invalid, InvalidParameter # # Since: 1.1 ## -{ 'command': 'block-stream', 'data': { 'device': 'str', '*base': 'str' } } +{ 'command': 'block-stream', 'data': { 'device': 'str', '*base': 'str', + '*speed': 'int' } } ## # @block-job-set-speed: @@ -1592,7 +1596,8 @@ # # @device: the device name # -# @speed: the maximum speed, in bytes per second +# @speed: the maximum speed, in bytes per second, or 0 for unlimited. +# Defaults to 0. # # Returns: Nothing on success # If the job type does not support throttling, NotSupported diff --git a/qmp-commands.hx b/qmp-commands.hx index b07ed59ccc..c810c74c11 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -688,7 +688,7 @@ EQMP { .name = "block-stream", - .args_type = "device:B,base:s?", + .args_type = "device:B,base:s?,speed:o?", .mhandler.cmd_new = qmp_marshal_input_block_stream, }, From e425306a27ef02b6ef19408d588974953f0f28c0 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 25 Apr 2012 16:51:04 +0100 Subject: [PATCH 5/6] qemu-iotests: add block-stream speed value test case Add tests to exercise the InvalidParameter 'speed' error code path, as well as the regular success case for setting the speed. The block-stream 'speed' parameter allows the speed limit of the job to be applied immediately when the job starts instead of issuing a separate block-job-set-speed command later. If the parameter has an invalid value we expect to get an error and the job is not created. It turns out that cancelling a block job is a common operation in these test cases, let's extract a cancel_and_wait() function instead of duplicating the QMP commands. Signed-off-by: Stefan Hajnoczi Acked-by: Kevin Wolf Signed-off-by: Luiz Capitulino --- tests/qemu-iotests/030 | 85 +++++++++++++++++++++++++++++++------- tests/qemu-iotests/030.out | 4 +- 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index 978fd82224..38abc2ce77 100755 --- a/tests/qemu-iotests/030 +++ b/tests/qemu-iotests/030 @@ -32,6 +32,21 @@ class ImageStreamingTestCase(iotests.QMPTestCase): result = self.vm.qmp('query-block-jobs') self.assert_qmp(result, 'return', []) + def cancel_and_wait(self, drive='drive0'): + '''Cancel a block job and wait for it to finish''' + result = self.vm.qmp('block-job-cancel', device=drive) + self.assert_qmp(result, 'return', {}) + + cancelled = False + while not cancelled: + for event in self.vm.get_qmp_events(wait=True): + if event['event'] == 'BLOCK_JOB_CANCELLED': + self.assert_qmp(event, 'data/type', 'stream') + self.assert_qmp(event, 'data/device', drive) + cancelled = True + + self.assert_no_active_streams() + class TestSingleDrive(ImageStreamingTestCase): image_len = 1 * 1024 * 1024 # MB @@ -97,21 +112,8 @@ class TestStreamStop(ImageStreamingTestCase): events = self.vm.get_qmp_events(wait=False) self.assertEqual(events, [], 'unexpected QMP event: %s' % events) - self.vm.qmp('block-job-cancel', device='drive0') - self.assert_qmp(result, 'return', {}) + self.cancel_and_wait() - cancelled = False - while not cancelled: - for event in self.vm.get_qmp_events(wait=True): - if event['event'] == 'BLOCK_JOB_CANCELLED': - self.assert_qmp(event, 'data/type', 'stream') - self.assert_qmp(event, 'data/device', 'drive0') - cancelled = True - - self.assert_no_active_streams() - -# This is a short performance test which is not run by default. -# Invoke "IMGFMT=qed ./030 TestSetSpeed.perf_test_set_speed" class TestSetSpeed(ImageStreamingTestCase): image_len = 80 * 1024 * 1024 # MB @@ -126,13 +128,15 @@ class TestSetSpeed(ImageStreamingTestCase): os.remove(test_img) os.remove(backing_img) - def perf_test_set_speed(self): + # This is a short performance test which is not run by default. + # Invoke "IMGFMT=qed ./030 TestSetSpeed.perf_test_throughput" + def perf_test_throughput(self): self.assert_no_active_streams() result = self.vm.qmp('block-stream', device='drive0') self.assert_qmp(result, 'return', {}) - result = self.vm.qmp('block-job-set-speed', device='drive0', value=8 * 1024 * 1024) + result = self.vm.qmp('block-job-set-speed', device='drive0', speed=8 * 1024 * 1024) self.assert_qmp(result, 'return', {}) completed = False @@ -147,5 +151,54 @@ class TestSetSpeed(ImageStreamingTestCase): self.assert_no_active_streams() + def test_set_speed(self): + self.assert_no_active_streams() + + result = self.vm.qmp('block-stream', device='drive0') + self.assert_qmp(result, 'return', {}) + + # Default speed is 0 + result = self.vm.qmp('query-block-jobs') + self.assert_qmp(result, 'return[0]/device', 'drive0') + self.assert_qmp(result, 'return[0]/speed', 0) + + result = self.vm.qmp('block-job-set-speed', device='drive0', speed=8 * 1024 * 1024) + self.assert_qmp(result, 'return', {}) + + # Ensure the speed we set was accepted + result = self.vm.qmp('query-block-jobs') + self.assert_qmp(result, 'return[0]/device', 'drive0') + self.assert_qmp(result, 'return[0]/speed', 8 * 1024 * 1024) + + self.cancel_and_wait() + + # Check setting speed in block-stream works + result = self.vm.qmp('block-stream', device='drive0', speed=4 * 1024 * 1024) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('query-block-jobs') + self.assert_qmp(result, 'return[0]/device', 'drive0') + self.assert_qmp(result, 'return[0]/speed', 4 * 1024 * 1024) + + self.cancel_and_wait() + + def test_set_speed_invalid(self): + self.assert_no_active_streams() + + result = self.vm.qmp('block-stream', device='drive0', speed=-1) + self.assert_qmp(result, 'error/class', 'InvalidParameter') + self.assert_qmp(result, 'error/data/name', 'speed') + + self.assert_no_active_streams() + + result = self.vm.qmp('block-stream', device='drive0') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1) + self.assert_qmp(result, 'error/class', 'InvalidParameter') + self.assert_qmp(result, 'error/data/name', 'speed') + + self.cancel_and_wait() + if __name__ == '__main__': iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out index 8d7e996700..914e3737bd 100644 --- a/tests/qemu-iotests/030.out +++ b/tests/qemu-iotests/030.out @@ -1,5 +1,5 @@ -... +..... ---------------------------------------------------------------------- -Ran 3 tests +Ran 5 tests OK From b3c83a2265261594d0a24507a17ad2f5c83eea81 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Thu, 26 Apr 2012 17:15:02 -0300 Subject: [PATCH 6/6] qapi: fix qmp_balloon() conversion Commit d72f326431 forgot to convert a call from qerror_report() to error_set(). Fix it. Signed-off-by: Luiz Capitulino Reviewed-by: Michael Roth --- balloon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/balloon.c b/balloon.c index 0166744aa8..aa354f7554 100644 --- a/balloon.c +++ b/balloon.c @@ -108,7 +108,7 @@ void qmp_balloon(int64_t value, Error **errp) } if (value <= 0) { - qerror_report(QERR_INVALID_PARAMETER_VALUE, "target", "a size"); + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "target", "a size"); return; }