Add an 'id' field to block jobs
v2 cover Add an 'id' field to block jobs
Hello all,
Block jobs are currently identified by the name of the block backend of the BDS where the job was started.
This series adds a new 'id' field that is specific to the block job and guaranteed to be unique and always present.
The patches can be summarized as follows:
- 1: Fix the prototype of stream_start(), that is going to be touched by a later patch in the series.
- 2-5: Split BlockJob's 'id' field into 'id' and 'device' and adjust the internal APIs to allow using the job ID.
- 6-9: Add parameters to allow setting the job ID to all QMP commands that create block jobs (block-stream, block-commit, ...)
- 10-14: Add parameters to allow using the job ID to all QMP command that manipulate block jobs (cancel, pause, complete, ...).
- 15: Expose the ID field in BlockJobInfo and all BLOCK_JOB_* events.
Things to do:
- Add new tests specific to job IDs.
- Add job ID support to HMP commands.
Berto
v2:
- Rebased on top of the current master
- Patch 2
- Now that job->id no longer holds the device name, replace "The active block job for device '%s' cannot be completed" with "The active block job '%s' cannot be completed" in mirror_complete() and block_job_complete().
- Patch 4
- Keep using ERROR_CLASS_DEVICE_NOT_ACTIVE in find_block_job() and change error path. [Max]
- Patch 6
- Fix merge conflicts after 274fcce.
- Patch 9
- Fix subject in commit message [Max]
- Patch 15
- Update iotests 109 and 156 [Max]
v1: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg02438.html
- Initial release
git backport-diff against v1:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch [down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/15:[----] [--] 'stream: Fix prototype of stream_start()'
002/15:[0009] [FC] 'blockjob: Decouple the ID from the device name in the BlockJob struct' 003/15:[----] [-C] 'blockjob: Add block_job_get()'
004/15:[0031] [FC] 'block: Simplify find_block_job() and make it accept a job ID' 005/15:[----] [-C] 'blockjob: Add 'job_id' parameter to block_job_create()' 006/15:[0003] [FC] 'mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'' 007/15:[----] [-C] 'backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup'' 008/15:[----] [--] 'stream: Add 'job-id' parameter to 'block-stream'' 009/15:[down] 'commit: Add 'job-id' parameter to 'block-commit'' 010/15:[----] [--] 'blockjob: Add 'id' parameter to 'block-job-set-speed'' 011/15:[----] [--] 'blockjob: Add 'id' parameter to 'block-job-cancel'' 012/15:[----] [--] 'blockjob: Add 'id' parameter to 'block-job-pause'' 013/15:[----] [--] 'blockjob: Add 'id' parameter to 'block-job-resume'' 014/15:[----] [--] 'blockjob: Add 'id' parameter to 'block-job-complete'' 015/15:[0095] [FC] 'blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events'
Alberto Garcia (15):
stream: Fix prototype of stream_start()
blockjob: Decouple the ID from the device name in the BlockJob struct
blockjob: Add block_job_get()
block: Simplify find_block_job() and make it accept a job ID
blockjob: Add 'job_id' parameter to block_job_create()
mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'
backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup'
stream: Add 'job-id' parameter to 'block-stream'
commit: Add 'job-id' parameter to 'block-commit'
blockjob: Add 'id' parameter to 'block-job-set-speed'
blockjob: Add 'id' parameter to 'block-job-cancel'
blockjob: Add 'id' parameter to 'block-job-pause'
blockjob: Add 'id' parameter to 'block-job-resume'
blockjob: Add 'id' parameter to 'block-job-complete'
blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events
block/backup.c | 9 +-- block/commit.c | 7 +- block/mirror.c | 22 +++--- block/stream.c | 12 ++-- blockdev.c | 175 +++++++++++++++++++++++++++------------------ blockjob.c | 46 ++++++++++-- docs/qmp-events.txt | 4 ++ hmp.c | 16 ++--- include/block/block_int.h | 47 +++++++----- include/block/blockjob.h | 30 ++++++-- include/qapi/qmp/qerror.h | 3 - include/qemu/id.h | 1 + qapi/block-core.json | 91 ++++++++++++++++++----- qemu-img.c | 2 +- qmp-commands.hx | 32 +++++---- tests/qemu-iotests/095 | 2 +- tests/qemu-iotests/095.out | 2 +- tests/qemu-iotests/109 | 2 +- tests/qemu-iotests/109.out | 88 +++++++++++------------ tests/qemu-iotests/124 | 3 +- tests/qemu-iotests/141 | 6 +- tests/qemu-iotests/141.out | 14 ++-- tests/qemu-iotests/144 | 1 + tests/qemu-iotests/144.out | 4 +- tests/qemu-iotests/156 | 1 + tests/qemu-iotests/156.out | 4 +- tests/test-blockjob-txn.c | 2 +- util/id.c | 1 + 28 files changed, 396 insertions(+), 231 deletions(-)
Signature · 1 line
2.8.1
PATCH v2 1/15 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()).
--- include/block/block_int.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
include/block/block_int.h +6 -5
@@ -625,8 +625,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.
@@ -637,11 +637,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);/**
Signature · 1 line
2.8.1
Re: [Qemu-devel] [PATCH v2 01/15] stream: Fix prototype of stream_start()
Am 22.06.2016 um 14:24 hat Alberto Garcia geschrieben:
Quoted 11 lines
'stream-start' has a parameter called 'backing-file', which is the string to be written to
bs->backingwhen 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 <[email protected]>Reviewed-by Max Reitz <[email protected]>
PATCH v2 2/15 blockjob: Decouple the ID from the device name in the BlockJob struct
In reply to Alberto GarciaBlock jobs are identified by the name of the BlockBackend of the BDS where the job was started.
We want block jobs to have unique, arbitrary identifiers that are not tied to a block device, so this patch decouples the ID from the device name in the BlockJob structure.
The ID is generated automatically for the moment, in later patches we'll allow the user to set it.
--- block/mirror.c | 3 ++- blockjob.c | 18 +++++++++++------- include/block/blockjob.h | 12 ++++++++---- include/qapi/qmp/qerror.h | 3 --- include/qemu/id.h | 1 + util/id.c | 1 + 6 files changed, 23 insertions(+), 15 deletions(-)
block/mirror.c +2 -1
@@ -755,7 +755,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 ; }
blockjob.c +11 -7
@@ -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"
@@ -125,7 +126,8 @@ 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->device =g_strdup (bdrv_get_device_name (bs)); + job->id =id_generate (ID_JOB ); job->blk = blk; job->cb = cb; job->opaque = opaque;
@@ -168,6 +170,7 @@ void block_job_unref(BlockJob *job) block_job_detach_aio_context, job);blk_unref (job->blk);error_free (job->blocker); +g_free (job->device);g_free (job->id);QLIST_REMOVE (job, job_list);g_free (job);
@@ -289,7 +292,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 ; }
@@ -464,7 +468,7 @@ BlockJobInfo *block_job_query(BlockJob *job) {BlockJobInfo *info =g_new0 (BlockJobInfo,1 ); info->type =g_strdup (BlockJobType_lookup[job->driver->job_type]); - info->device =g_strdup (job->id); + info->device =g_strdup (job->device); info->len = job->len; info->busy = job->busy; info->paused = job->pause_count >0 ;
@@ -486,7 +490,7 @@ static void block_job_iostatus_set_err(BlockJob *job, int error)void block_job_event_cancelled (BlockJob *job) { qapi_event_send_block_job_cancelled(job->driver->job_type, - job->id, + job->device, job->len, job->offset, job->speed,
@@ -496,7 +500,7 @@ void block_job_event_cancelled(BlockJob *job)void block_job_event_completed (BlockJob *job,const char *msg) { qapi_event_send_block_job_completed(job->driver->job_type, - job->id, + job->device, job->len, job->offset, job->speed,
@@ -510,7 +514,7 @@ void block_job_event_ready(BlockJob *job) job->ready = true;qapi_event_send_block_job_ready (job->driver->job_type, - job->id, + job->device, job->len, job->offset, job->speed, &error_abort);
@@ -538,7 +542,7 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,default :abort (); } -qapi_event_send_block_job_error (job->id, +qapi_event_send_block_job_error (job->device, is_read ?IO_OPERATION_TYPE_READ :IO_OPERATION_TYPE_WRITE , action, &error_abort);
include/block/blockjob.h +8 -4
@@ -106,14 +106,18 @@ 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;/** +* BlockBackend name of the BDS owning the job at the time when +* the job is started. For compatibility with clients that don't +* support the ID field. +*/ +char *device; + +/** * The coroutine that executes the job. If not NULL, it is * reentered when busy is false and the job is cancelled. */
include/qapi/qmp/qerror.h +0 -3
@@ -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"
include/qemu/id.h +1 -0
@@ -4,6 +4,7 @@typedef enum IdSubSystems {ID_QDEV ,ID_BLOCK , +ID_JOB ,ID_MAX /* last element, used as array size */ }IdSubSystems ;
util/id.c +1 -0
@@ -34,6 +34,7 @@ bool id_wellformed(const char *id)static const char *const id_subsys_str[ID_MAX ] = { [ID_QDEV ] ="qdev" , [ID_BLOCK ] ="block" , + [ID_JOB ] ="job" , };/*
Signature · 1 line
2.8.1
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
Am 22.06.2016 um 14:24 hat Alberto Garcia geschrieben:
Quoted 2 lines
Block jobs are identified by the name of the BlockBackend of the BDS where the job was started.
Let me rephrase that: Block jobs are identified by an ID which defaults to the name of the BlockBackend where the job was started and can't currently be set by the user.
Quoted 6 lines
We want block jobs to have unique, arbitrary identifiers that are not tied to a block device, so this patch decouples the ID from the device name in the BlockJob structure.
The ID is generated automatically for the moment, in later patches we'll allow the user to set it.
This approach requires us to keep track of both a device name and an ID in order to maintain compatibility, and we always need to check both when a job is referenced. This model is already a pain with node-name and BB name, I wouldn't want to introduce more instances.
Why don't we go the easy route and make the ID configurable, but still default to the device name?
include/block/blockjob.h struct BlockJob { · line 118
Quoted 10 lines
Signed-off-by Alberto Garcia <[email protected]>include/block/blockjob.h +5
-0/** +* BlockBackend name of the BDS owning the job at the time when +* the job is started. For compatibility with clients that don't +* support the ID field. +*/ +char *device;
This in particular feels like a step backwards. This addition is tightening the coupling between jobs and devices instead of removing it.
Quoted 37 lines
include/block/blockjob.h +4
-4@@ -464,7 +468,7 @@ BlockJobInfo *block_job_query(BlockJob *job) {BlockJobInfo *info =g_new0 (BlockJobInfo,1 ); info->type =g_strdup (BlockJobType_lookup[job->driver->job_type]); - info->device =g_strdup (job->id); + info->device =g_strdup (job->device); info->len = job->len; info->busy = job->busy; info->paused = job->pause_count >0 ;@@ -486,7 +490,7 @@ static void block_job_iostatus_set_err(BlockJob *job, int error)void block_job_event_cancelled (BlockJob *job) { qapi_event_send_block_job_cancelled(job->driver->job_type, - job->id, + job->device, job->len, job->offset, job->speed,@@ -496,7 +500,7 @@ void block_job_event_cancelled(BlockJob *job)void block_job_event_completed (BlockJob *job,const char *msg) { qapi_event_send_block_job_completed(job->driver->job_type, - job->id, + job->device, job->len, job->offset, job->speed,@@ -510,7 +514,7 @@ void block_job_event_ready(BlockJob *job) job->ready = true;qapi_event_send_block_job_ready (job->driver->job_type, - job->id, + job->device, job->len, job->offset, job->speed, &error_abort);
include/block/blockjob.h BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err, · line 548
Quoted 8 lines
include/block/blockjob.h +1
-1@@ -538,7 +542,7 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,default :abort (); } -qapi_event_send_block_job_error (job->id, +qapi_event_send_block_job_error (job->device, is_read ?IO_OPERATION_TYPE_READ :IO_OPERATION_TYPE_WRITE , action, &error_abort);
These monitor related functions are the only users of the device name. Let's simply redefine the respective QMP fields to refer to the job ID rather than the device (because identifying the job is really what they are used for; once we allow more than one block job per device, putting the device there will be completely useless).
All of these redefinitions are backwards compatible. Only clients which are new enough to set an individual ID can get something that isn't a device name here. And they will be prepared for it.
Kevin
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
Quoted 7 lines · Kevin Wolf
Quoted 6 lines
We want block jobs to have unique, arbitrary identifiers that are not tied to a block device, so this patch decouples the ID from the device name in the BlockJob structure.
The ID is generated automatically for the moment, in later patches we'll allow the user to set it.
Quoted 7 lines
This approach requires us to keep track of both a device name and an ID in order to maintain compatibility, and we always need to check both when a job is referenced. This model is already a pain with node-name and BB name, I wouldn't want to introduce more instances.
Why don't we go the easy route and make the ID configurable, but still default to the device name?
That was my first approach when I started to write the series, but I discarded it for several reasons:
- More confusing API: we'd have many instances of fields and parameters named 'device' holding something that is not a device name or anything similar.
We'd have e.g. block-commit(id, device), and the created job would have a 'device' that is not the user-specified 'device' but the user-specified 'id', which has no relation to devices.
- More risk of collisions: with the current system if no ID is specified a new one is be generated. That's guaranteed to be valid and unique. If we default to the device name we have no such guarantee.
- Potential compatibility break: there's a field that used to hold the name of the device but it no longer does.
I don't think any of them is a showstopper, and as you said if a client is recent enough to understand job IDs it should also expect the device field to hold something different from a device name. But I'm not a big fan of this kind of scenarios where the semantics of a returned value depend on what we can assume that the caller is able to handle.
I thought adding a new 'ID' field was simpler. The device name is still a device name (where it makes sense). The default ID is guaranteed to be valid and guaranteed not to clash with user-defined IDs. The API is (in my opinion) more clear.
The only problems that I can think of:
- BlockJobInfo and the events expose the 'device' field which is superfluous.
- block-job-{pause,resume,...} can take an ID or a device name.
All that said, I'm open to change the implementation, but I'd like to make sure that the problems of keeping the ID and device name merged are considered.
Berto
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
Am 22.06.2016 um 16:42 hat Alberto Garcia geschrieben:
Quoted 19 lines · Kevin Wolf
Quoted 7 lines
Quoted 6 lines
We want block jobs to have unique, arbitrary identifiers that are not tied to a block device, so this patch decouples the ID from the device name in the BlockJob structure.
The ID is generated automatically for the moment, in later patches we'll allow the user to set it.
Quoted 7 lines
This approach requires us to keep track of both a device name and an ID in order to maintain compatibility, and we always need to check both when a job is referenced. This model is already a pain with node-name and BB name, I wouldn't want to introduce more instances.
Why don't we go the easy route and make the ID configurable, but still default to the device name?
That was my first approach when I started to write the series, but I discarded it for several reasons:
Quoted 7 lines
- More confusing API: we'd have many instances of fields and parameters named 'device' holding something that is not a device name or anything similar.
We'd have e.g. block-commit(id, device), and the created job would have a 'device' that is not the user-specified 'device' but the user-specified 'id', which has no relation to devices.
Yes, the 'device' field ends up having the wrong name then. This is ugly, but I think having code to deal with two ways of addressing jobs, in two different namespaces, is a cost too high to be justifiable just for making things look nicer.
I seem to remember that John is going to introduce a new set of job management functions anyway while he generalises block jobs to just background jobs, so in that case the ugliness would be confined to a deprecated legacy interface. I think that's tolerable.
Quoted 3 lines
- More risk of collisions: with the current system if no ID is specified a new one is be generated. That's guaranteed to be valid and unique. If we default to the device name we have no such guarantee.
The expectation is that if a client passes an ID for one job, it will pass IDs for all jobs. As long as you never pass an ID, the interface looks exactly the same as it used to. If you pass it always, you're obviously responsible for choosing unique IDs.
The only vaguely interesting case is if you mix both styles and then create a new job with an explicit ID that you already used for a block device. In this case you get an error, and I'd argue you deserve it.
Quoted 2 lines
- Potential compatibility break: there's a field that used to hold the name of the device but it no longer does.
Old clients never set an explicit ID, so what they get in this field is always the default, which is a device name.
I don't see the compatibility problem?
Quoted 5 lines
I don't think any of them is a showstopper, and as you said if a client is recent enough to understand job IDs it should also expect the device field to hold something different from a device name. But I'm not a big fan of this kind of scenarios where the semantics of a returned value depend on what we can assume that the caller is able to handle.
It's really just another case of making something configurable and letting it default to the previously hardcoded value. Pretty much a normal thing to do.
Quoted 10 lines
I thought adding a new 'ID' field was simpler. The device name is still a device name (where it makes sense). The default ID is guaranteed to be valid and guaranteed not to clash with user-defined IDs. The API is (in my opinion) more clear.
The only problems that I can think of:
- BlockJobInfo and the events expose the 'device' field which is superfluous.
- block-job-{pause,resume,...} can take an ID or a device name.
Yes. There are two parts that I don't like about this.
The first one is that we need additional code to keep track of the device name and to look it up.
The other, more important one is that it couples block jobs more tightly with a BDS:
* What do you with a background job that doesn't have a device name because it doesn't work on a BDS? Will 'device' become optional everywhere? But how is this less problematic for compatibility than returning non-device-name IDs? (To be clear, I don't think either is a real problem, but you can hardly dismiss one and accept the other.)
* And what do you do once we allow more than one job per device? Then the device name isn't suitable for addressing the job any more. And letting the client use the device name after it started the first job, but not any more after it started the second one, feels wrong.
Kevin
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
Quoted 7 lines · Kevin Wolf
Quoted 7 lines
I thought adding a new 'ID' field was simpler. The device name is still a device name (where it makes sense). The default ID is guaranteed to be valid and guaranteed not to clash with user-defined IDs. The API is (in my opinion) more clear.
The only problems that I can think of:
Quoted 8 lines
Quoted 3 lines
- BlockJobInfo and the events expose the 'device' field which is superfluous.
- block-job-{pause,resume,...} can take an ID or a device name.Yes. There are two parts that I don't like about this.
The first one is that we need additional code to keep track of the device name and to look it up.
I think this part is negligible, but ok.
Quoted 10 lines
The other, more important one is that it couples block jobs more tightly with a BDS:
* What do you with a background job that doesn't have a device name because it doesn't work on a BDS? Will 'device' become optional everywhere? But how is this less problematic for compatibility than returning non-device-name IDs? (To be clear, I don't think either is a real problem, but you can hardly dismiss one and accept the other.)
Quoted 4 lines
* And what do you do once we allow more than one job per device? Then the device name isn't suitable for addressing the job any more. And letting the client use the device name after it started the first job, but not any more after it started the second one, feels wrong.
Fair enough. Unless Max, Eric or someone else has something else to add I'll give it a try and see how it looks.
Berto
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
Quoted 29 lines · Alberto Garcia
Quoted 7 lines · Kevin Wolf
Quoted 7 lines
I thought adding a new 'ID' field was simpler. The device name is still a device name (where it makes sense). The default ID is guaranteed to be valid and guaranteed not to clash with user-defined IDs. The API is (in my opinion) more clear.
The only problems that I can think of:
Quoted 8 lines
Quoted 3 lines
- BlockJobInfo and the events expose the 'device' field which is superfluous.
- block-job-{pause,resume,...} can take an ID or a device name.Yes. There are two parts that I don't like about this.
The first one is that we need additional code to keep track of the device name and to look it up.
I think this part is negligible, but ok.
Quoted 9 lines
The other, more important one is that it couples block jobs more tightly with a BDS:
* What do you with a background job that doesn't have a device name because it doesn't work on a BDS? Will 'device' become optional everywhere? But how is this less problematic for compatibility than returning non-device-name IDs? (To be clear, I don't think either is a real problem, but you can hardly dismiss one and accept the other.)
Quoted 7 lines
Quoted 4 lines
* And what do you do once we allow more than one job per device? Then the device name isn't suitable for addressing the job any more. And letting the client use the device name after it started the first job, but not any more after it started the second one, feels wrong.
Fair enough. Unless Max, Eric or someone else has something else to add I'll give it a try and see how it looks.
Sorry for the late response, but then again I don't actually have an opinion either way.
The thing I feel most strongly about is the issue of storing a general ID in a field named "device". However, as Kevin hinted at this becoming irrelevant with John's work on decoupling block jobs from the block layer.
(And it probably will, because we don't want to call e.g. block-job-pause that way then anymore, so John's probably going to add newly named commands anyway, even if they do the same as the old ones. I don't know.)
But this also means that I don't understand the first point of the second part of what you quoted from Kevin here. I think he's referring to what qemu returns e.g. in query-block-jobs. But why should query-block-jobs return non-*block*-jobs? I think it should always omit all background jobs that are not related to the block layer. Or at least that would make sense.
The second point of the second part doesn't really strike with me either. If a client uses the device name to identify a block job, they should be used to a version of qemu that isn't capable of having more than one job per device anyway, so they shouldn't launch more than a single job per device to begin with.
So I don't think any of the pros and cons is a very strong point. I personally feel like having a single ID field is more natural and making the device name its default value is very intuitive, but I take issue with its naming ("device"). So I don't care either way.
(Maybe, aside from John's work, we could get the naming issue out of the way by introducing something like QMP aliases...?)
Max
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
Quoted 36 lines · Max Reitz
Quoted 28 lines
Quoted 7 lines
Quoted 7 lines
I thought adding a new 'ID' field was simpler. The device name is still a device name (where it makes sense). The default ID is guaranteed to be valid and guaranteed not to clash with user-defined IDs. The API is (in my opinion) more clear.
The only problems that I can think of:
Quoted 8 lines
Quoted 3 lines
- BlockJobInfo and the events expose the 'device' field which is superfluous.
- block-job-{pause,resume,...} can take an ID or a device name.Yes. There are two parts that I don't like about this.
The first one is that we need additional code to keep track of the device name and to look it up.
I think this part is negligible, but ok.
Quoted 9 lines
The other, more important one is that it couples block jobs more tightly with a BDS:
* What do you with a background job that doesn't have a device name because it doesn't work on a BDS? Will 'device' become optional everywhere? But how is this less problematic for compatibility than returning non-device-name IDs? (To be clear, I don't think either is a real problem, but you can hardly dismiss one and accept the other.)
Quoted 7 lines
Quoted 4 lines
* And what do you do once we allow more than one job per device? Then the device name isn't suitable for addressing the job any more. And letting the client use the device name after it started the first job, but not any more after it started the second one, feels wrong.
Fair enough. Unless Max, Eric or someone else has something else to add I'll give it a try and see how it looks.
Quoted 7 lines
Sorry for the late response, but then again I don't actually have an opinion either way.
The thing I feel most strongly about is the issue of storing a general ID in a field named "device". However, as Kevin hinted at this becoming irrelevant with John's work on decoupling block jobs from the block layer.
I actually forgot to Cc him, I'm doing it now.
The idea is that I don't want to add anything now that is going to cause headaches later. Adding a new 'id' field to block jobs and keeping 'device' feels more natural to me, but reusing the 'device' field and allowing any ID set by the user requires less changes both to the code and the API.
Berto
Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
Quoted 47 lines · Alberto Garcia
Quoted 36 lines · Max Reitz
Quoted 28 lines
Quoted 7 lines
Quoted 7 lines
I thought adding a new 'ID' field was simpler. The device name is still a device name (where it makes sense). The default ID is guaranteed to be valid and guaranteed not to clash with user-defined IDs. The API is (in my opinion) more clear.
The only problems that I can think of:
Quoted 8 lines
Quoted 3 lines
- BlockJobInfo and the events expose the 'device' field which is superfluous.
- block-job-{pause,resume,...} can take an ID or a device name.Yes. There are two parts that I don't like about this.
The first one is that we need additional code to keep track of the device name and to look it up.
I think this part is negligible, but ok.
Quoted 9 lines
The other, more important one is that it couples block jobs more tightly with a BDS:
* What do you with a background job that doesn't have a device name because it doesn't work on a BDS? Will 'device' become optional everywhere? But how is this less problematic for compatibility than returning non-device-name IDs? (To be clear, I don't think either is a real problem, but you can hardly dismiss one and accept the other.)
Quoted 7 lines
Quoted 4 lines
* And what do you do once we allow more than one job per device? Then the device name isn't suitable for addressing the job any more. And letting the client use the device name after it started the first job, but not any more after it started the second one, feels wrong.
Fair enough. Unless Max, Eric or someone else has something else to add I'll give it a try and see how it looks.
Quoted 7 lines
Sorry for the late response, but then again I don't actually have an opinion either way.
The thing I feel most strongly about is the issue of storing a general ID in a field named "device". However, as Kevin hinted at this becoming irrelevant with John's work on decoupling block jobs from the block layer.
I actually forgot to Cc him, I'm doing it now.
Quoted 8 lines
The idea is that I don't want to add anything now that is going to cause headaches later. Adding a new 'id' field to block jobs and keeping 'device' feels more natural to me, but reusing the 'device' field and allowing any ID set by the user requires less changes both to the code and the API.
Berto
Reviewing everything now, sorry for being MIA, and thank you for keeping me in the loop.
--js
PATCH v2 3/15 blockjob: Add block_job_get()
In reply to Alberto GarciaCurrently 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.
--- blockjob.c | 13 +++++++++++++ include/block/blockjob.h | 10 ++++++++++ 2 files changed, 23 insertions(+)
blockjob.c +13 -0
@@ -61,6 +61,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.
include/block/blockjob.h +10 -0
@@ -218,6 +218,16 @@ struct BlockJob {BlockJob *block_job_next (BlockJob *job);/** +* block_job_get: +* @id: The id of the block job. +* +* Get the block job identified by @id (which must not be %NULL). +* +* Returns the requested job, or %NULL if it doesn't exist. +*/ +BlockJob *block_job_get (const char *id); + +/** * block_job_create: * @job_type: The class object for the newly-created job. * @bs: The block
Signature · 1 line
2.8.1
Re: [Qemu-devel] [PATCH v2 03/15] blockjob: Add block_job_get()
Am 22.06.2016 um 14:25 hat Alberto Garcia geschrieben:
Quoted 8 lines
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 <[email protected]>Reviewed-by Max Reitz <[email protected]>
PATCH v2 4/15 block: Simplify find_block_job() and make it accept a job ID
In reply to Alberto Garciafind_block_job() looks for a block backend with a specified name, checks whether it has a block job and acquires its AioContext. This patch uses block_job_next() and iterate directly over the block jobs.
In addition to that we want to identify jobs primarily by their ID, so this patch updates find_block_job() to allow IDs too. Only one of ID and device name can be specified when looking for a block job.
--- blockdev.c | 77 +++++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 44 insertions(+), 33 deletions(-)
blockdev.c +44 -33
@@ -3704,48 +3704,59 @@ 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, -Error **errp) +/* Get a block job using its ID or device name and acquire its AioContext. +* Only one of ID and device name can be specified. */ +static BlockJob *find_block_job (const char *id,const char *device, +AioContext **aio_context,Error **errp) { -BlockBackend *blk; -BlockDriverState *bs; +BlockJob *job =NULL ; *aio_context =NULL ; - blk =blk_by_name (device); -if (!blk) { - goto notfound; +if ((id && device) || (!id && !device)) { +error_setg (errp,"Only one of ID or device name " +"must be specified when looking for a block job" ); +return NULL ; } - *aio_context =blk_get_aio_context (blk); +if (id) { + job =block_job_get (id); + }else { +BlockJob *j; +for (j =block_job_next (NULL ); j; j =block_job_next (j)) { +if (!strcmp (device, j->device)) { +if (job) { +/* This scenario is currently not possible, but it +* could theoretically happen in the future. */ +error_setg (errp,"More than one job on device '%s', " +"use the job ID instead" , device); +return NULL ; + } + job = j; + } + } + } + +if (!job) { +if (id) { +error_setg (errp,"Block job '%s' not found" , id); + }else { +error_set (errp,ERROR_CLASS_DEVICE_NOT_ACTIVE , +"No active block job on device '%s'" , device); + } +return NULL ; + } + + *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) {AioContext *aio_context; -BlockJob *job =find_block_job (device, &aio_context, errp); +BlockJob *job =find_block_job (NULL , device, &aio_context, errp);if (!job) {return ;
@@ -3759,7 +3770,7 @@ void qmp_block_job_cancel(const char *device, bool has_force, bool force, Error **errp) { AioContext *aio_context; -BlockJob *job =find_block_job (device, &aio_context, errp); +BlockJob *job =find_block_job (NULL , device, &aio_context, errp);if (!job) {return ;
@@ -3784,7 +3795,7 @@ out:void qmp_block_job_pause (const char *device,Error **errp) {AioContext *aio_context; -BlockJob *job =find_block_job (device, &aio_context, errp); +BlockJob *job =find_block_job (NULL , device, &aio_context, errp);if (!job || job->user_paused) {return ;
@@ -3799,7 +3810,7 @@ void qmp_block_job_pause(const char *device, Error **errp)void qmp_block_job_resume (const char *device,Error **errp) {AioContext *aio_context; -BlockJob *job =find_block_job (device, &aio_context, errp); +BlockJob *job =find_block_job (NULL , device, &aio_context, errp);if (!job || !job->user_paused) {return ;
@@ -3815,7 +3826,7 @@ void qmp_block_job_resume(const char *device, Error **errp)void qmp_block_job_complete (const char *device,Error **errp) {AioContext *aio_context; -BlockJob *job =find_block_job (device, &aio_context, errp); +BlockJob *job =find_block_job (NULL , device, &aio_context, errp);if (!job) {return ;
Signature · 1 line
2.8.1
Re: [Qemu-devel] [PATCH v2 04/15] block: Simplify find_block_job() and make it accept a job ID
Am 22.06.2016 um 14:25 hat Alberto Garcia geschrieben:
Quoted 9 lines
find_block_job()looks for a block backend with a specified name, checks whether it has a block job and acquires its AioContext. This patch usesblock_job_next()and iterate directly over the block jobs.In addition to that we want to identify jobs primarily by their ID, so this patch updates
find_block_job()to allow IDs too. Only one of ID and device name can be specified when looking for a block job.Signed-off-by Alberto Garcia <[email protected]>
Nice, this gets rid of another bs->job user.
But as I said in patch 2, it's better to leave out the device thing. We can just update the documentation of the QMP commands so that 'device' contains a job ID rather than a device name and then we can look up IDs unconditionally. This will simplify the function even more.
Kevin
PATCH v2 5/15 blockjob: Add 'job_id' parameter to block_job_create()
In reply to Alberto GarciaJob IDs are generated automatically when a new job is created. This patch adds a new 'job_id' parameter to let the caller provide one instead. In this case the ID is verified to be unique and well-formed.
--- block/backup.c | 3 ++- block/commit.c | 2 +- block/mirror.c | 2 +- block/stream.c | 2 +- blockjob.c | 19 +++++++++++++++---- include/block/blockjob.h | 8 +++++--- tests/test-blockjob-txn.c | 2 +- 7 files changed, 26 insertions(+), 12 deletions(-)
block/backup.c +2 -1
@@ -542,7 +542,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; }
block/commit.c +1 -1
@@ -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 ; }
block/mirror.c +1 -1
@@ -867,7 +867,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 ; }
block/stream.c +1 -1
@@ -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 ; }
blockjob.c +15 -4
@@ -117,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 +129,17 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,return NULL ; } +if (job_id) { +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);
@@ -140,7 +151,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, job->driver = driver; job->device =g_strdup (bdrv_get_device_name (bs)); - job->id =id_generate (ID_JOB ); + job->id = job_id ?g_strdup (job_id) :id_generate (ID_JOB ); job->blk = blk; job->cb = cb; job->opaque = opaque;
include/block/blockjob.h +5 -3
@@ -229,6 +229,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.
@@ -245,9 +247,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:
tests/test-blockjob-txn.c +1 -1
@@ -94,7 +94,7 @@ static BlockJob *test_block_job_start(unsigned int iterations, data =g_new0 (TestBlockJobCBData,1 ); bs =bdrv_new (); - s =block_job_create (&test_block_job_driver, bs,0 , test_block_job_cb, + s =block_job_create (NULL , &test_block_job_driver, bs,0 , test_block_job_cb, data, &error_abort); s->iterations = iterations; s->use_timer = use_timer;
Signature · 1 line
2.8.1
Re: [Qemu-devel] [PATCH v2 05/15] blockjob: Add 'job_id' parameter to block_job_create()
Am 22.06.2016 um 14:25 hat Alberto Garcia geschrieben:
Quoted 8 lines
Job IDs are generated automatically when a new job is created. This patch adds a new '
job_id' parameter to let the caller provide one instead. In this case the ID is verified to be unique and well-formed.Signed-off-by Alberto Garcia <[email protected]>Reviewed-by Max Reitz <[email protected]>
blockjob.c void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, · line 157
Quoted 8 lines
blockjob.c +1
-1@@ -140,7 +151,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, job->driver = driver; job->device =g_strdup (bdrv_get_device_name (bs)); - job->id =id_generate (ID_JOB ); + job->id = job_id ?g_strdup (job_id) :id_generate (ID_JOB ); job->blk = blk; job->cb = cb; job->opaque = opaque;
This hunk will trivially conflict with removing device names. You can keep my R-b while resolving that.
PATCH v2 6/15 mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'
In reply to Alberto GarciaThis 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.
--- block/mirror.c | 15 ++++++++------- blockdev.c | 15 ++++++++------- hmp.c | 2 +- include/block/block_int.h | 6 ++++-- qapi/block-core.json | 10 +++++++--- qmp-commands.hx | 8 +++++--- 6 files changed, 33 insertions(+), 23 deletions(-)
block/mirror.c +8 -7
@@ -837,8 +837,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,
@@ -867,7 +867,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 ; }
@@ -900,8 +900,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,
@@ -919,7 +919,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);
@@ -967,7 +967,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) {
blockdev.c +8 -7
@@ -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,
hmp.c +1 -1
@@ -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,
include/block/block_int.h +4 -2
@@ -680,6 +680,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 have one +* generated automatically. * @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
@@ -701,8 +703,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,
qapi/block-core.json +7 -3
@@ -1108,6 +1108,8 @@ # # Start mirroring a block device's writes to a new destination. # +# @job-id: #optional identifier for the newly-created block job (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 +1162,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 +1245,8 @@ # # Start mirroring a block device's writes to a new destination. # +# @job-id: #optional identifier for the newly-created block job (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 +1283,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',
qmp-commands.hx +5 -3
@@ -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,7 @@ of the source. Arguments: +- "job-id": identifier for the newly-created block job (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 +1721,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 +1736,7 @@ specifies the target of mirror operation. Arguments: +- "job-id": identifier for the newly-created block job (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
Signature · 1 line
2.8.1
PATCH v2 7/15 backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup'
In reply to Alberto GarciaThis 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.
--- block/backup.c | 8 ++++---- blockdev.c | 43 ++++++++++++++++++++++++------------------- hmp.c | 2 +- include/block/block_int.h | 8 +++++--- qapi/block-core.json | 10 +++++++--- qmp-commands.hx | 8 +++++--- 6 files changed, 46 insertions(+), 33 deletions(-)
block/backup.c +4 -4
@@ -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,
@@ -542,7 +542,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;
blockdev.c +24 -19
115 lines not on this page · raw message
hmp.c +1 -1
@@ -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);
include/block/block_int.h +5 -3
@@ -715,6 +715,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs,/* * backup_start: +* @job_id: The id of the newly-created job, or %NULL to have one +* generated automatically. * @bs: Block device to operate on. * @target: Block device to write to. * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
@@ -729,9 +731,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,
qapi/block-core.json +7 -3
@@ -866,6 +866,8 @@ ## # @DriveBackup # +# @job-id: #optional identifier for the newly-created block job (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 +905,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 +914,8 @@ ## # @BlockdevBackup # +# @job-id: #optional identifier for the newly-created block job (Since 2.7) +# # @device: the name of the device which should be copied. # # @target: the name of the backup target device.
@@ -938,7 +942,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',
qmp-commands.hx +5 -3
@@ -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,7 @@ block-job-cancel command. Arguments: +- "job-id": identifier for the newly-created block job (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 +1267,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 +1281,7 @@ as backup target. Arguments: +- "job-id": identifier for the newly-created block job (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)
Signature · 1 line
2.8.1
PATCH v2 8/15 stream: Add 'job-id' parameter to 'block-stream'
In reply to Alberto GarciaThis 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.
--- block/stream.c | 12 ++++++------ blockdev.c | 6 +++--- hmp.c | 2 +- include/block/block_int.h | 10 ++++++---- qapi/block-core.json | 7 +++++-- qmp-commands.hx | 3 ++- 6 files changed, 23 insertions(+), 17 deletions(-)
block/stream.c +6 -6
@@ -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 ; }
blockdev.c +3 -3
@@ -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;
hmp.c +1 -1
@@ -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);
include/block/block_int.h +6 -4
@@ -622,6 +622,8 @@ int is_windows_drive(const char *filename);/** * stream_start: +* @job_id: The id of the newly-created job, or %NULL to have one +* generated automatically. * @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.
@@ -640,10 +642,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:
qapi/block-core.json +5 -2
@@ -1421,6 +1421,8 @@ # 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 (Since 2.7) +# # @device: the device name # # @base: #optional the common backing file name
@@ -1452,8 +1454,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:
qmp-commands.hx +2 -1
@@ -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,7 @@ Copy data from a backing file into a block device. Arguments: +- "job-id": identifier for the newly-created block job (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)
Signature · 1 line
2.8.1
PATCH v2 9/15 commit: Add 'job-id' parameter to 'block-commit'
In reply to Alberto GarciaThis 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.
--- block/commit.c | 7 ++++--- block/mirror.c | 6 +++--- blockdev.c | 9 +++++---- include/block/block_int.h | 16 ++++++++++------ qapi/block-core.json | 4 +++- qemu-img.c | 2 +- qmp-commands.hx | 3 ++- 7 files changed, 28 insertions(+), 19 deletions(-)
block/commit.c +4 -3
@@ -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 ; }
block/mirror.c +3 -3
@@ -925,8 +925,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)
@@ -967,7 +967,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);
blockdev.c +5 -4
@@ -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 ) {
include/block/block_int.h +10 -6
@@ -649,6 +649,8 @@ void stream_start(const char *job_id, BlockDriverState *bs,/** * commit_start: +* @job_id: The id of the newly-created job, or %NULL to have one +* generated automatically. * @bs: Active block device. * @top: Top block device to be committed. * @base: Block device that will be written into, and become the new top.
@@ -660,12 +662,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 have one +* generated automatically. * @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.
@@ -675,8 +679,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);
qapi/block-core.json +3 -1
@@ -1008,6 +1008,8 @@ # 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 (Since 2.7) +# # @device: the name of the device # # @base: #optional The file name of the backing image to write data into.
@@ -1059,7 +1061,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' } } ##
qemu-img.c +1 -1
@@ -914,7 +914,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;
qmp-commands.hx +2 -1
@@ -1150,7 +1150,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, },
@@ -1163,6 +1163,7 @@ data between 'top' and 'base' into 'base'. Arguments: +- "job-id": identifier for the newly-created block job (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
Signature · 1 line
2.8.1
PATCH v2 10/15 blockjob: Add 'id' parameter to 'block-job-set-speed'
In reply to Alberto GarciaThis patch allows the 'block-job-set-speed' command to identify the job by either its ID or its device name. The latter becomes now optional since the ID alone is enough to identify the job.
The HMP 'block_job_set_speed' command remains unchanged.
--- blockdev.c | 7 +++++-- hmp.c | 2 +- qapi/block-core.json | 9 +++++++-- qmp-commands.hx | 2 +- 4 files changed, 14 insertions(+), 6 deletions(-)
blockdev.c +5 -2
@@ -3760,10 +3760,13 @@ static BlockJob *find_block_job(const char *id, const char *device,return job; } -void qmp_block_job_set_speed (const char *device,int64_t speed,Error **errp) +void qmp_block_job_set_speed (bool has_id,const char *id,bool has_device, +const char *device,int64_t speed,Error **errp) {AioContext *aio_context; -BlockJob *job =find_block_job (NULL , device, &aio_context, errp); +BlockJob *job =find_block_job (has_id ? id :NULL , + has_device ? device :NULL , + &aio_context, errp);if (!job) {return ;
hmp.c +1 -1
@@ -1498,7 +1498,7 @@ void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict)const char *device =qdict_get_str (qdict,"device" );int64_t value =qdict_get_int (qdict,"speed" ); -qmp_block_job_set_speed (device, value, &error); +qmp_block_job_set_speed (false,NULL , true, device, value, &error);hmp_handle_error (mon, &error); }
qapi/block-core.json +7 -2
@@ -1469,7 +1469,12 @@ # # Throttling can be disabled by setting the speed to 0. # -# @device: the device name +# The job must be identified with the @id or @device parameters, but +# only one of them must be set. +# +# @id: #optional the job identifier. (Since 2.7) +# +# @device: #optional the device name. # # @speed: the maximum speed, in bytes per second, or 0 for unlimited. # Defaults to 0.
@@ -1480,7 +1485,7 @@ # Since: 1.1 ## { 'command': 'block-job-set-speed', - 'data': { 'device': 'str', 'speed': 'int' } } + 'data': { '*id': 'str', '*device': 'str', 'speed': 'int' } } ## # @block-job-cancel:
qmp-commands.hx +1 -1
@@ -1311,7 +1311,7 @@ EQMP { .name = "block-job-set-speed", - .args_type = "device:B,speed:o", + .args_type = "id:s?,device:B?,speed:o", .mhandler.cmd_new = qmp_marshal_block_job_set_speed, },
Signature · 1 line
2.8.1
PATCH v2 11/15 blockjob: Add 'id' parameter to 'block-job-cancel'
In reply to Alberto GarciaThis patch allows the 'block-job-cancel' command to identify the job by either its ID or its device name. The latter becomes now optional since the ID alone is enough to identify the job.
The HMP 'block_job_cancel' command remains unchanged.
--- blockdev.c | 7 +++++-- hmp.c | 2 +- qapi/block-core.json | 10 ++++++++-- qmp-commands.hx | 2 +- 4 files changed, 15 insertions(+), 6 deletions(-)
blockdev.c +5 -2
@@ -3776,11 +3776,14 @@ void qmp_block_job_set_speed(bool has_id, const char *id, bool has_device,aio_context_release (aio_context); } -void qmp_block_job_cancel (const char *device, +void qmp_block_job_cancel (bool has_id,const char *id, +bool has_device,const char *device,bool has_force,bool force,Error **errp) {AioContext *aio_context; -BlockJob *job =find_block_job (NULL , device, &aio_context, errp); +BlockJob *job =find_block_job (has_id ? id :NULL , + has_device ? device :NULL , + &aio_context, errp);if (!job) {return ;
hmp.c +1 -1
@@ -1509,7 +1509,7 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)const char *device =qdict_get_str (qdict,"device" );bool force =qdict_get_try_bool (qdict,"force" , false); -qmp_block_job_cancel (device, true, force, &error); +qmp_block_job_cancel (false,NULL , true, device, true, force, &error);hmp_handle_error (mon, &error); }
qapi/block-core.json +8 -2
@@ -1505,7 +1505,12 @@ # operation can be started at a later time to finish copying all data from the # backing file. # -# @device: the device name +# The job must be identified with the @id or @device parameters, but +# only one of them must be set. +# +# @id: #optional the job identifier (Since 2.7) +# +# @device: #optional the device name # # @force: #optional whether to allow cancellation of a paused job (default # false). Since 1.3.
@@ -1515,7 +1520,8 @@ # # Since: 1.1 ## -{ 'command': 'block-job-cancel', 'data': { 'device': 'str', '*force': 'bool' } } +{ 'command': 'block-job-cancel', + 'data': { '*id': 'str', '*device': 'str', '*force': 'bool' } } ## # @block-job-pause:
qmp-commands.hx +1 -1
@@ -1317,7 +1317,7 @@ EQMP { .name = "block-job-cancel", - .args_type = "device:B,force:b?", + .args_type = "id:s?,device:B?,force:b?", .mhandler.cmd_new = qmp_marshal_block_job_cancel, }, {
Signature · 1 line
2.8.1
PATCH v2 12/15 blockjob: Add 'id' parameter to 'block-job-pause'
In reply to Alberto GarciaThis patch allows the 'block-job-pause' command to identify the job by either its ID or its device name. The latter becomes now optional since the ID alone is enough to identify the job.
The HMP 'block_job_pause' command remains unchanged.
--- blockdev.c | 7 +++++-- hmp.c | 2 +- qapi/block-core.json | 9 +++++++-- qmp-commands.hx | 2 +- 4 files changed, 14 insertions(+), 6 deletions(-)
blockdev.c +5 -2
@@ -3805,10 +3805,13 @@ out:aio_context_release (aio_context); } -void qmp_block_job_pause (const char *device,Error **errp) +void qmp_block_job_pause (bool has_id,const char *id, +bool has_device,const char *device,Error **errp) {AioContext *aio_context; -BlockJob *job =find_block_job (NULL , device, &aio_context, errp); +BlockJob *job =find_block_job (has_id ? id :NULL , + has_device ? device :NULL , + &aio_context, errp);if (!job || job->user_paused) {return ;
hmp.c +1 -1
@@ -1519,7 +1519,7 @@ void hmp_block_job_pause(Monitor *mon, const QDict *qdict)Error *error =NULL ;const char *device =qdict_get_str (qdict,"device" ); -qmp_block_job_pause (device, &error); +qmp_block_job_pause (false,NULL , true, device, &error);hmp_handle_error (mon, &error); }
qapi/block-core.json +7 -2
@@ -1537,14 +1537,19 @@ # the operation is actually paused. Cancelling a paused job automatically # resumes it. # -# @device: the device name +# The job must be identified with the @id or @device parameters, but +# only one of them must be set. +# +# @id: #optional the job identifier. (Since 2.7) +# +# @device: #optional the device name # # Returns: Nothing on success # If no background operation is active on this device, DeviceNotActive # # Since: 1.3 ## -{ 'command': 'block-job-pause', 'data': { 'device': 'str' } } +{ 'command': 'block-job-pause', 'data': { '*id': 'str', '*device': 'str' } } ## # @block-job-resume:
qmp-commands.hx +1 -1
@@ -1322,7 +1322,7 @@ EQMP }, { .name = "block-job-pause", - .args_type = "device:B", + .args_type = "id:s?,device:B?", .mhandler.cmd_new = qmp_marshal_block_job_pause, }, {
Signature · 1 line
2.8.1
PATCH v2 13/15 blockjob: Add 'id' parameter to 'block-job-resume'
In reply to Alberto GarciaThis patch allows the 'block-job-resume' command to identify the job by either its ID or its device name. The latter becomes now optional since the ID alone is enough to identify the job.
The HMP 'block_job_resume' command remains unchanged.
--- blockdev.c | 7 +++++-- hmp.c | 2 +- qapi/block-core.json | 7 ++++++- qmp-commands.hx | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-)
blockdev.c +5 -2
@@ -3823,10 +3823,13 @@ void qmp_block_job_pause(bool has_id, const char *id,aio_context_release (aio_context); } -void qmp_block_job_resume (const char *device,Error **errp) +void qmp_block_job_resume (bool has_id,const char *id, +bool has_device,const char *device,Error **errp) {AioContext *aio_context; -BlockJob *job =find_block_job (NULL , device, &aio_context, errp); +BlockJob *job =find_block_job (has_id ? id :NULL , + has_device ? device :NULL , + &aio_context, errp);if (!job || !job->user_paused) {return ;
hmp.c +1 -1
@@ -1529,7 +1529,7 @@ void hmp_block_job_resume(Monitor *mon, const QDict *qdict)Error *error =NULL ;const char *device =qdict_get_str (qdict,"device" ); -qmp_block_job_resume (device, &error); +qmp_block_job_resume (false,NULL , true, device, &error);hmp_handle_error (mon, &error); }
qapi/block-core.json +6 -1
@@ -1562,6 +1562,11 @@ # # This command also clears the error status of the job. # +# The job must be identified with the @id or @device parameters, but +# only one of them must be set. +# +# @id: #optional the job identifier. (Since 2.7) +# # @device: the device name # # Returns: Nothing on success
@@ -1569,7 +1574,7 @@ # # Since: 1.3 ## -{ 'command': 'block-job-resume', 'data': { 'device': 'str' } } +{ 'command': 'block-job-resume', 'data': { '*id': 'str', '*device': 'str' } } ## # @block-job-complete:
qmp-commands.hx +1 -1
@@ -1327,7 +1327,7 @@ EQMP }, { .name = "block-job-resume", - .args_type = "device:B", + .args_type = "id:s?,device:B?", .mhandler.cmd_new = qmp_marshal_block_job_resume, }, {
Signature · 1 line
2.8.1
PATCH v2 14/15 blockjob: Add 'id' parameter to 'block-job-complete'
In reply to Alberto GarciaThis patch allows the 'block-job-complete' command to identify the job by either its ID or its device name. The latter becomes now optional since the ID alone is enough to identify the job.
The HMP 'block_job_complete' command remains unchanged.
--- blockdev.c | 7 +++++-- hmp.c | 2 +- qapi/block-core.json | 7 ++++++- qmp-commands.hx | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-)
blockdev.c +5 -2
@@ -3842,10 +3842,13 @@ void qmp_block_job_resume(bool has_id, const char *id,aio_context_release (aio_context); } -void qmp_block_job_complete (const char *device,Error **errp) +void qmp_block_job_complete (bool has_id,const char *id, +bool has_device,const char *device,Error **errp) {AioContext *aio_context; -BlockJob *job =find_block_job (NULL , device, &aio_context, errp); +BlockJob *job =find_block_job (has_id ? id :NULL , + has_device ? device :NULL , + &aio_context, errp);if (!job) {return ;
hmp.c +1 -1
@@ -1539,7 +1539,7 @@ void hmp_block_job_complete(Monitor *mon, const QDict *qdict)Error *error =NULL ;const char *device =qdict_get_str (qdict,"device" ); -qmp_block_job_complete (device, &error); +qmp_block_job_complete (false,NULL , true, device, &error);hmp_handle_error (mon, &error); }
qapi/block-core.json +6 -1
@@ -1593,6 +1593,11 @@ # # A cancelled or paused job cannot be completed. # +# The job must be identified with the @id or @device parameters, but +# only one of them must be set. +# +# @id: #optional the job identifier. (Since 2.7) +# # @device: the device name # # Returns: Nothing on success
@@ -1600,7 +1605,7 @@ # # Since: 1.3 ## -{ 'command': 'block-job-complete', 'data': { 'device': 'str' } } +{ 'command': 'block-job-complete', 'data': { '*id': 'str', '*device': 'str' } } ## # @BlockdevDiscardOptions
qmp-commands.hx +1 -1
@@ -1332,7 +1332,7 @@ EQMP }, { .name = "block-job-complete", - .args_type = "device:B", + .args_type = "id:s?,device:B?", .mhandler.cmd_new = qmp_marshal_block_job_complete, }, {
Signature · 1 line
2.8.1
PATCH v2 15/15 blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events
In reply to Alberto GarciaNow that all jobs have an ID and all QMP commands that create and operate on block jobs can specify one we can finally expose the ID in all BLOCK_JOB_* events and the BlockJobInfo structure.
This modifies the output of several iotests, but now we have full control of the job IDs so this patch updates the affected ones.
--- blockjob.c | 6 +++- docs/qmp-events.txt | 4 +++ qapi/block-core.json | 18 ++++++++-- tests/qemu-iotests/095 | 2 +- tests/qemu-iotests/095.out | 2 +- tests/qemu-iotests/109 | 2 +- tests/qemu-iotests/109.out | 88 +++++++++++++++++++++++----------------------- tests/qemu-iotests/124 | 3 +- tests/qemu-iotests/141 | 6 +++- tests/qemu-iotests/141.out | 14 ++++---- tests/qemu-iotests/144 | 1 + tests/qemu-iotests/144.out | 4 +-- tests/qemu-iotests/156 | 1 + tests/qemu-iotests/156.out | 4 +-- 14 files changed, 92 insertions(+), 63 deletions(-)
blockjob.c +5 -1
@@ -492,6 +492,7 @@ BlockJobInfo *block_job_query(BlockJob *job) {BlockJobInfo *info =g_new0 (BlockJobInfo,1 ); info->type =g_strdup (BlockJobType_lookup[job->driver->job_type]); + info->id =g_strdup (job->id); info->device =g_strdup (job->device); info->len = job->len; info->busy = job->busy;
@@ -514,6 +515,7 @@ static void block_job_iostatus_set_err(BlockJob *job, int error)void block_job_event_cancelled (BlockJob *job) { qapi_event_send_block_job_cancelled(job->driver->job_type, + job->id, job->device, job->len, job->offset,
@@ -524,6 +526,7 @@ void block_job_event_cancelled(BlockJob *job)void block_job_event_completed (BlockJob *job,const char *msg) { qapi_event_send_block_job_completed(job->driver->job_type, + job->id, job->device, job->len, job->offset,
@@ -538,6 +541,7 @@ void block_job_event_ready(BlockJob *job) job->ready = true; qapi_event_send_block_job_ready(job->driver->job_type, + job->id, job->device, job->len, job->offset,
@@ -566,7 +570,7 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,default :abort (); } -qapi_event_send_block_job_error (job->device, +qapi_event_send_block_job_error (job->id, job->device, is_read ?IO_OPERATION_TYPE_READ :IO_OPERATION_TYPE_WRITE , action, &error_abort);
docs/qmp-events.txt +4 -0
@@ -92,6 +92,7 @@ Data: - "type": Job type (json-string; "stream" for image streaming "commit" for block commit) +- "id": Job identifier (json-string) - "device": Device name (json-string) - "len": Maximum progress value (json-int) - "offset": Current progress value (json-int)
@@ -116,6 +117,7 @@ Data: - "type": Job type (json-string; "stream" for image streaming "commit" for block commit) +- "id": Job identifier (json-string) - "device": Device name (json-string) - "len": Maximum progress value (json-int) - "offset": Current progress value (json-int)
@@ -143,6 +145,7 @@ Emitted when a block job encounters an error. Data: +- "id": job identifier (json-string) - "device": device name (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):
@@ -167,6 +170,7 @@ Data: - "type": Job type (json-string; "stream" for image streaming "commit" for block commit) +- "id": Job identifier (json-string) - "device": Device name (json-string) - "len": Maximum progress value (json-int) - "offset": Current progress value (json-int)
qapi/block-core.json +16 -2
@@ -713,6 +713,8 @@ # # @type: the job type ('stream' for image streaming) # +# @id: the job identifier +# # @device: the block device name # # @len: the maximum progress value
@@ -734,7 +736,7 @@ # Since: 1.1 ## { 'struct': 'BlockJobInfo', - 'data': {'type': 'str', 'device': 'str', 'len': 'int', + 'data': {'type': 'str', 'id': 'str', 'device': 'str', 'len': 'int', 'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int', 'io-status': 'BlockDeviceIoStatus', 'ready': 'bool'} }
@@ -2443,6 +2445,8 @@ # # @type: job type # +# @id: the job identifier +# # @device: device name # # @len: maximum progress value
@@ -2461,6 +2465,7 @@ ## { 'event': 'BLOCK_JOB_COMPLETED', 'data': { 'type' : 'BlockJobType', + 'id' : 'str', 'device': 'str', 'len' : 'int', 'offset': 'int',
@@ -2474,6 +2479,8 @@ # # @type: job type # +# @id: the job identifier +# # @device: device name # # @len: maximum progress value
@@ -2487,6 +2494,7 @@ ## { 'event': 'BLOCK_JOB_CANCELLED', 'data': { 'type' : 'BlockJobType', + 'id' : 'str', 'device': 'str', 'len' : 'int', 'offset': 'int',
@@ -2497,6 +2505,8 @@ # # Emitted when a block job encounters an error # +# @id: the job identifier +# # @device: device name # # @operation: I/O operation
@@ -2506,7 +2516,8 @@ # Since: 1.3 ## { 'event': 'BLOCK_JOB_ERROR', - 'data': { 'device' : 'str', + 'data': { 'id' : 'str', + 'device' : 'str', 'operation': 'IoOperationType', 'action' : 'BlockErrorAction' } }
@@ -2517,6 +2528,8 @@ # # @type: job type # +# @id: the job identifier +# # @device: device name # # @len: maximum progress value
@@ -2533,6 +2546,7 @@ ## { 'event': 'BLOCK_JOB_READY', 'data': { 'type' : 'BlockJobType', + 'id' : 'str', 'device': 'str', 'len' : 'int', 'offset': 'int',
tests/qemu-iotests/095 +1 -1
@@ -71,7 +71,7 @@ h=$QEMU_HANDLE _send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return" _send_qemu_cmd $h "{ 'execute': 'block-commit', - 'arguments': { 'device': 'test', + 'arguments': { 'job-id': 'testjob', 'device': 'test', 'top': '"${TEST_IMG}.snp1"' } }" "BLOCK_JOB_COMPLETED" _cleanup_qemu
tests/qemu-iotests/095.out +1 -1
@@ -12,7 +12,7 @@ virtual size: 5.0M (5242880 bytes) {"return": {}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "test", "len": 104857600, "offset": 104857600, "speed": 0, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "test", "len": 104857600, "offset": 104857600, "speed": 0, "type": "commit", "id": "testjob"}} === Base image info after commit and resize === image: TEST_DIR/t.IMGFMT.base
tests/qemu-iotests/109 +1 -1
@@ -58,7 +58,7 @@ function run_qemu() _send_qemu_cmd $QEMU_HANDLE \ "{'execute':'drive-mirror', 'arguments':{ 'device': 'src', 'target': '$raw_img', $qmp_format - 'mode': 'existing', 'sync': 'full'}}" \ + 'mode': 'existing', 'sync': 'full', 'job-id': 'mirror1'}}" \ "return" _send_qemu_cmd $QEMU_HANDLE '' "$qmp_event"
tests/qemu-iotests/109.out +44 -44
208 lines not on this page · raw message
tests/qemu-iotests/124 +2 -1
@@ -50,7 +50,7 @@ def transaction_bitmap_clear(node, name, **kwargs): def transaction_drive_backup(device, target, **kwargs): return transaction_action('drive-backup', device=device, target=target, - **kwargs) + job_id=device, **kwargs) class Bitmap:
@@ -479,6 +479,7 @@ class TestIncrementalBackup(TestIncrementalBackupBase): self.assertFalse(self.wait_qmp_backup(drive1['id'])) error = self.vm.event_wait('BLOCK_JOB_ERROR') self.assert_qmp(error, 'data', {'device': drive1['id'], + 'id': drive1['id'], 'action': 'report', 'operation': 'read'}) self.assertFalse(self.vm.get_qmp_events(wait=False))
tests/qemu-iotests/141 +5 -1
@@ -102,6 +102,7 @@ echo test_blockjob \ "{'execute': 'drive-backup', 'arguments': {'device': 'drv0', + 'job-id': 'backup1', 'target': '$TEST_DIR/o.$IMGFMT', 'format': '$IMGFMT', 'sync': 'none'}}" \
@@ -118,6 +119,7 @@ echo test_blockjob \ "{'execute': 'drive-mirror', 'arguments': {'device': 'drv0', + 'job-id': 'mirror1', 'target': '$TEST_DIR/o.$IMGFMT', 'format': '$IMGFMT', 'sync': 'none'}}" \
@@ -134,7 +136,7 @@ echo test_blockjob \ "{'execute': 'block-commit', - 'arguments': {'device': 'drv0'}}" \ + 'arguments': {'device': 'drv0', 'job-id': 'commit1'}}" \ 'BLOCK_JOB_READY' \ 'BLOCK_JOB_COMPLETED'
@@ -151,6 +153,7 @@ $QEMU_IO -c 'write 0 1M' "$TEST_DIR/m.$IMGFMT" | _filter_qemu_io test_blockjob \ "{'execute': 'block-commit', 'arguments': {'device': 'drv0', + 'job-id': 'commit2', 'top': '$TEST_DIR/m.$IMGFMT', 'speed': 1}}" \ 'return' \
@@ -173,6 +176,7 @@ $QEMU_IO -c 'write 0 1M' "$TEST_DIR/b.$IMGFMT" | _filter_qemu_io test_blockjob \ "{'execute': 'block-stream', 'arguments': {'device': 'drv0', + 'job-id': 'stream1', 'speed': 1}}" \ 'return' \ 'BLOCK_JOB_CANCELLED'
tests/qemu-iotests/141.out +7 -7
@@ -11,7 +11,7 @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t. {"return": {}} {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: backup"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 0, "speed": 0, "type": "backup"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 0, "speed": 0, "type": "backup", "id": "backup1"}} {"return": {}} === Testing drive-mirror ===
@@ -19,20 +19,20 @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t. {"return": {}} Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1"}} {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: mirror"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1"}} {"return": {}} === Testing active block-commit === {"return": {}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit", "id": "commit1"}} {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit", "id": "commit1"}} {"return": {}} === Testing non-active block-commit ===
@@ -43,7 +43,7 @@ wrote 1048576/1048576 bytes at offset 0 {"return": {}} {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "commit", "id": "commit2"}} {"return": {}} === Testing block-stream ===
@@ -54,6 +54,6 @@ wrote 1048576/1048576 bytes at offset 0 {"return": {}} {"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: stream"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "stream"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "stream", "id": "stream1"}} {"return": {}} *** done
tests/qemu-iotests/144 +1 -0
@@ -85,6 +85,7 @@ echo # Block commit on active layer, push the new overlay into base _send_qemu_cmd $h "{ 'execute': 'block-commit', 'arguments': { + 'job-id': 'commit1', 'device': 'virtio0' } }" "READY"
tests/qemu-iotests/144.out +2 -2
@@ -13,9 +13,9 @@ Formatting 'TEST_DIR/tmp.qcow2', fmt=qcow2 size=536870912 backing_file=TEST_DIR/ === Performing block-commit on active layer === {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit", "id": "commit1"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit", "id": "commit1"}} === Performing Live Snapshot 2 ===
tests/qemu-iotests/156 +1 -0
@@ -87,6 +87,7 @@ TEST_IMG="$TEST_IMG.target.overlay" _make_test_img -b "$TEST_IMG.target" 1M _send_qemu_cmd $QEMU_HANDLE \ "{ 'execute': 'drive-mirror', 'arguments': { 'device': 'source', + 'job-id': 'mirror1', 'target': '$TEST_IMG.target.overlay', 'mode': 'existing', 'sync': 'top' } }" \
tests/qemu-iotests/156.out +2 -2
@@ -13,12 +13,12 @@ wrote 131072/131072 bytes at offset 131072 {"return": ""} Formatting 'TEST_DIR/t.IMGFMT.target.overlay', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT.target {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "source", "len": 131072, "offset": 131072, "speed": 0, "type": "mirror"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "source", "len": 131072, "offset": 131072, "speed": 0, "type": "mirror", "id": "mirror1"}} wrote 65536/65536 bytes at offset 196608 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) {"return": ""} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "source", "len": 196608, "offset": 196608, "speed": 0, "type": "mirror"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "source", "len": 196608, "offset": 196608, "speed": 0, "type": "mirror", "id": "mirror1"}} read 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Signature · 1 line
2.8.1