* [Qemu-devel] [PATCH v2 01/15] stream: Fix prototype of stream_start()
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
@ 2016-06-22 12:24 ` Alberto Garcia
2016-06-22 12:45 ` Kevin Wolf
2016-06-22 12:24 ` [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct Alberto Garcia
` (13 subsequent siblings)
14 siblings, 1 reply; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
'stream-start' has a parameter called 'backing-file', which is the
string to be written to bs->backing when the job finishes.
In the stream_start() implementation it is called 'backing_file_str',
but it the prototype in the header file it is called 'base_id'.
This patch fixes it so the name is the same in both cases and is
consistent with other cases (like commit_start()).
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/block_int.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2057156..549b1ed 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -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);
/**
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 01/15] stream: Fix prototype of stream_start()
2016-06-22 12:24 ` [Qemu-devel] [PATCH v2 01/15] stream: Fix prototype of stream_start() Alberto Garcia
@ 2016-06-22 12:45 ` Kevin Wolf
0 siblings, 0 replies; 27+ messages in thread
From: Kevin Wolf @ 2016-06-22 12:45 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody
Am 22.06.2016 um 14:24 hat Alberto Garcia geschrieben:
> 'stream-start' has a parameter called 'backing-file', which is the
> string to be written to bs->backing when the job finishes.
>
> In the stream_start() implementation it is called 'backing_file_str',
> but it the prototype in the header file it is called 'base_id'.
>
> This patch fixes it so the name is the same in both cases and is
> consistent with other cases (like commit_start()).
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
2016-06-22 12:24 ` [Qemu-devel] [PATCH v2 01/15] stream: Fix prototype of stream_start() Alberto Garcia
@ 2016-06-22 12:24 ` Alberto Garcia
2016-06-22 12:42 ` Kevin Wolf
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 03/15] blockjob: Add block_job_get() Alberto Garcia
` (12 subsequent siblings)
14 siblings, 1 reply; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
Block 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.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
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(-)
diff --git a/block/mirror.c b/block/mirror.c
index a04ed9c..bcb1999 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -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;
}
diff --git a/blockjob.c b/blockjob.c
index 90c4e26..8bcb5ea 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -33,6 +33,7 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qjson.h"
#include "qemu/coroutine.h"
+#include "qemu/id.h"
#include "qmp-commands.h"
#include "qemu/timer.h"
#include "qapi-event.h"
@@ -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);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 7dc720c..856486a 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -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.
*/
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index d08652a..6586c9f 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -19,9 +19,6 @@
#define QERR_BASE_NOT_FOUND \
"Base '%s' not found"
-#define QERR_BLOCK_JOB_NOT_READY \
- "The active block job for device '%s' cannot be completed"
-
#define QERR_BUS_NO_HOTPLUG \
"Bus '%s' does not support hotplugging"
diff --git a/include/qemu/id.h b/include/qemu/id.h
index 7d90335..c6c73ca 100644
--- a/include/qemu/id.h
+++ b/include/qemu/id.h
@@ -4,6 +4,7 @@
typedef enum IdSubSystems {
ID_QDEV,
ID_BLOCK,
+ ID_JOB,
ID_MAX /* last element, used as array size */
} IdSubSystems;
diff --git a/util/id.c b/util/id.c
index 6141352..eb5478b 100644
--- a/util/id.c
+++ b/util/id.c
@@ -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",
};
/*
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
2016-06-22 12:24 ` [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct Alberto Garcia
@ 2016-06-22 12:42 ` Kevin Wolf
2016-06-22 14:42 ` Alberto Garcia
0 siblings, 1 reply; 27+ messages in thread
From: Kevin Wolf @ 2016-06-22 12:42 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody
Am 22.06.2016 um 14:24 hat Alberto Garcia geschrieben:
> 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.
> 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?
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> /**
> + * 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.
> @@ -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);
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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
2016-06-22 12:42 ` Kevin Wolf
@ 2016-06-22 14:42 ` Alberto Garcia
2016-06-22 15:49 ` Kevin Wolf
0 siblings, 1 reply; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 14:42 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody
On Wed 22 Jun 2016 02:42:24 PM CEST, Kevin Wolf wrote:
>> 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?
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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
2016-06-22 14:42 ` Alberto Garcia
@ 2016-06-22 15:49 ` Kevin Wolf
2016-06-23 12:47 ` Alberto Garcia
0 siblings, 1 reply; 27+ messages in thread
From: Kevin Wolf @ 2016-06-22 15:49 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody
Am 22.06.2016 um 16:42 hat Alberto Garcia geschrieben:
> On Wed 22 Jun 2016 02:42:24 PM CEST, Kevin Wolf wrote:
> >> 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?
>
> 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.
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.
> - 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.
> - 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?
> 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.
> 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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
2016-06-22 15:49 ` Kevin Wolf
@ 2016-06-23 12:47 ` Alberto Garcia
2016-06-29 17:20 ` Max Reitz
0 siblings, 1 reply; 27+ messages in thread
From: Alberto Garcia @ 2016-06-23 12:47 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody
On Wed 22 Jun 2016 05:49:28 PM CEST, Kevin Wolf wrote:
>> 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.
I think this part is negligible, but ok.
> 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.
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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
2016-06-23 12:47 ` Alberto Garcia
@ 2016-06-29 17:20 ` Max Reitz
2016-06-30 13:03 ` Alberto Garcia
0 siblings, 1 reply; 27+ messages in thread
From: Max Reitz @ 2016-06-29 17:20 UTC (permalink / raw)
To: Alberto Garcia, Kevin Wolf; +Cc: qemu-devel, qemu-block, Eric Blake, Jeff Cody
[-- Attachment #1: Type: text/plain, Size: 3337 bytes --]
On 23.06.2016 14:47, Alberto Garcia wrote:
> On Wed 22 Jun 2016 05:49:28 PM CEST, Kevin Wolf wrote:
>>> 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.
>
> I think this part is negligible, but ok.
>
>> 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.
>
> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
2016-06-29 17:20 ` Max Reitz
@ 2016-06-30 13:03 ` Alberto Garcia
2016-07-01 18:48 ` John Snow
0 siblings, 1 reply; 27+ messages in thread
From: Alberto Garcia @ 2016-06-30 13:03 UTC (permalink / raw)
To: Max Reitz, Kevin Wolf
Cc: qemu-devel, qemu-block, Eric Blake, Jeff Cody, John Snow
On Wed 29 Jun 2016 07:20:55 PM CEST, Max Reitz wrote:
>>>> 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.
>>
>> I think this part is negligible, but ok.
>>
>>> 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.
>>
>> 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.
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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct
2016-06-30 13:03 ` Alberto Garcia
@ 2016-07-01 18:48 ` John Snow
0 siblings, 0 replies; 27+ messages in thread
From: John Snow @ 2016-07-01 18:48 UTC (permalink / raw)
To: Alberto Garcia, Max Reitz, Kevin Wolf; +Cc: Jeff Cody, qemu-devel, qemu-block
On 06/30/2016 09:03 AM, Alberto Garcia wrote:
> On Wed 29 Jun 2016 07:20:55 PM CEST, Max Reitz wrote:
>>>>> 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.
>>>
>>> I think this part is negligible, but ok.
>>>
>>>> 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.
>>>
>>> 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.
>
> 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
>
Reviewing everything now, sorry for being MIA, and thank you for keeping
me in the loop.
--js
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 03/15] blockjob: Add block_job_get()
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
2016-06-22 12:24 ` [Qemu-devel] [PATCH v2 01/15] stream: Fix prototype of stream_start() Alberto Garcia
2016-06-22 12:24 ` [Qemu-devel] [PATCH v2 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:45 ` Kevin Wolf
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 04/15] block: Simplify find_block_job() and make it accept a job ID Alberto Garcia
` (11 subsequent siblings)
14 siblings, 1 reply; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
Currently the way to look for a specific block job is to iterate the
list manually using block_job_next().
Since we want to be able to identify a job primarily by its ID it
makes sense to have a function that does just that.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockjob.c | 13 +++++++++++++
include/block/blockjob.h | 10 ++++++++++
2 files changed, 23 insertions(+)
diff --git a/blockjob.c b/blockjob.c
index 8bcb5ea..51c6402 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -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.
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 856486a..9f28230 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -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
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 03/15] blockjob: Add block_job_get()
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 03/15] blockjob: Add block_job_get() Alberto Garcia
@ 2016-06-22 12:45 ` Kevin Wolf
0 siblings, 0 replies; 27+ messages in thread
From: Kevin Wolf @ 2016-06-22 12:45 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody
Am 22.06.2016 um 14:25 hat Alberto Garcia geschrieben:
> Currently the way to look for a specific block job is to iterate the
> list manually using block_job_next().
>
> Since we want to be able to identify a job primarily by its ID it
> makes sense to have a function that does just that.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 04/15] block: Simplify find_block_job() and make it accept a job ID
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (2 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 03/15] blockjob: Add block_job_get() Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:50 ` Kevin Wolf
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 05/15] blockjob: Add 'job_id' parameter to block_job_create() Alberto Garcia
` (10 subsequent siblings)
14 siblings, 1 reply; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
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 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.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
blockdev.c | 77 +++++++++++++++++++++++++++++++++++---------------------------
1 file changed, 44 insertions(+), 33 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 3a104a0..ffb4383 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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;
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 04/15] block: Simplify find_block_job() and make it accept a job ID
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 04/15] block: Simplify find_block_job() and make it accept a job ID Alberto Garcia
@ 2016-06-22 12:50 ` Kevin Wolf
0 siblings, 0 replies; 27+ messages in thread
From: Kevin Wolf @ 2016-06-22 12:50 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody
Am 22.06.2016 um 14:25 hat Alberto Garcia geschrieben:
> 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 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.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 05/15] blockjob: Add 'job_id' parameter to block_job_create()
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (3 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 04/15] block: Simplify find_block_job() and make it accept a job ID Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 13:10 ` Kevin Wolf
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 06/15] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' Alberto Garcia
` (9 subsequent siblings)
14 siblings, 1 reply; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
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 <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
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(-)
diff --git a/block/backup.c b/block/backup.c
index 581269b..33bb2ce 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -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;
}
diff --git a/block/commit.c b/block/commit.c
index 444333b..3535ba7 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -236,7 +236,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
return;
}
- s = block_job_create(&commit_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(NULL, &commit_job_driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/block/mirror.c b/block/mirror.c
index bcb1999..48253fb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -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;
}
diff --git a/block/stream.c b/block/stream.c
index c0efbda..e4319d3 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -226,7 +226,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base,
{
StreamBlockJob *s;
- s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(NULL, &stream_job_driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/blockjob.c b/blockjob.c
index 51c6402..c7f6992 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -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;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 9f28230..0fd6eaa 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -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:
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index d3030f1..84e2af5 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -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;
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Qemu-devel] [PATCH v2 05/15] blockjob: Add 'job_id' parameter to block_job_create()
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 05/15] blockjob: Add 'job_id' parameter to block_job_create() Alberto Garcia
@ 2016-06-22 13:10 ` Kevin Wolf
0 siblings, 0 replies; 27+ messages in thread
From: Kevin Wolf @ 2016-06-22 13:10 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody
Am 22.06.2016 um 14:25 hat Alberto Garcia geschrieben:
> 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 <berto@igalia.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> @@ -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.
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH v2 06/15] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (4 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 05/15] blockjob: Add 'job_id' parameter to block_job_create() Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 07/15] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup' Alberto Garcia
` (8 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This patch adds a new optional 'job-id' parameter to 'blockdev-mirror'
and 'drive-mirror', allowing the user to specify the ID of the block
job to be created.
The HMP 'drive_mirror' command remains unchanged.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
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(-)
diff --git a/block/mirror.c b/block/mirror.c
index 48253fb..960934b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -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) {
diff --git a/blockdev.c b/blockdev.c
index ffb4383..f4ebd4f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3422,7 +3422,7 @@ void qmp_blockdev_backup(const char *device, const char *target,
/* Parameter check and block job starting for drive mirroring.
* Caller should hold @device and @target's aio context (must be the same).
**/
-static void blockdev_mirror_common(BlockDriverState *bs,
+static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
BlockDriverState *target,
bool has_replaces, const char *replaces,
enum MirrorSyncMode sync,
@@ -3482,15 +3482,15 @@ static void blockdev_mirror_common(BlockDriverState *bs,
/* pass the node name to replace to mirror start since it's loose coupling
* and will allow to check whether the node still exist at mirror completion
*/
- mirror_start(bs, target,
+ mirror_start(job_id, bs, target,
has_replaces ? replaces : NULL,
speed, granularity, buf_size, sync, backing_mode,
on_source_error, on_target_error, unmap,
block_job_cb, bs, errp);
}
-void qmp_drive_mirror(const char *device, const char *target,
- bool has_format, const char *format,
+void qmp_drive_mirror(bool has_job_id, const char *job_id, const char *device,
+ const char *target, bool has_format, const char *format,
bool has_node_name, const char *node_name,
bool has_replaces, const char *replaces,
enum MirrorSyncMode sync,
@@ -3634,7 +3634,7 @@ void qmp_drive_mirror(const char *device, const char *target,
bdrv_set_aio_context(target_bs, aio_context);
- blockdev_mirror_common(bs, target_bs,
+ blockdev_mirror_common(has_job_id ? job_id : NULL, bs, target_bs,
has_replaces, replaces, sync, backing_mode,
has_speed, speed,
has_granularity, granularity,
@@ -3649,7 +3649,8 @@ out:
aio_context_release(aio_context);
}
-void qmp_blockdev_mirror(const char *device, const char *target,
+void qmp_blockdev_mirror(bool has_job_id, const char *job_id,
+ const char *device, const char *target,
bool has_replaces, const char *replaces,
MirrorSyncMode sync,
bool has_speed, int64_t speed,
@@ -3690,7 +3691,7 @@ void qmp_blockdev_mirror(const char *device, const char *target,
bdrv_set_aio_context(target_bs, aio_context);
- blockdev_mirror_common(bs, target_bs,
+ blockdev_mirror_common(has_job_id ? job_id : NULL, bs, target_bs,
has_replaces, replaces, sync, backing_mode,
has_speed, speed,
has_granularity, granularity,
diff --git a/hmp.c b/hmp.c
index 997a768..087ae53 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1097,7 +1097,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
}
- qmp_drive_mirror(device, filename, !!format, format,
+ qmp_drive_mirror(false, NULL, device, filename, !!format, format,
false, NULL, false, NULL,
full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
true, mode, false, 0, false, 0, false, 0,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 549b1ed..dd45fec 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -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,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 98a20d2..6f015c8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b444c20..9eee869 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1656,8 +1656,8 @@ EQMP
{
.name = "drive-mirror",
- .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
- "node-name:s?,replaces:s?,"
+ .args_type = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?,"
+ "format:s?,node-name:s?,replaces:s?,"
"on-source-error:s?,on-target-error:s?,"
"unmap:b?,"
"granularity:i?,buf-size:i?",
@@ -1677,6 +1677,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
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 07/15] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (5 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 06/15] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 08/15] stream: Add 'job-id' parameter to 'block-stream' Alberto Garcia
` (7 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This patch adds a new optional 'job-id' parameter to 'blockdev-backup'
and 'drive-backup', allowing the user to specify the ID of the block
job to be created.
The HMP 'drive_backup' command remains unchanged.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
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(-)
diff --git a/block/backup.c b/block/backup.c
index 33bb2ce..050936a 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -474,9 +474,9 @@ static void coroutine_fn backup_run(void *opaque)
block_job_defer_to_main_loop(&job->common, backup_complete, data);
}
-void backup_start(BlockDriverState *bs, BlockDriverState *target,
- int64_t speed, MirrorSyncMode sync_mode,
- BdrvDirtyBitmap *sync_bitmap,
+void backup_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, int64_t speed,
+ MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
@@ -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;
diff --git a/blockdev.c b/blockdev.c
index f4ebd4f..d23ef42 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1836,9 +1836,9 @@ typedef struct DriveBackupState {
BlockJob *job;
} DriveBackupState;
-static void do_drive_backup(const char *device, const char *target,
- bool has_format, const char *format,
- enum MirrorSyncMode sync,
+static void do_drive_backup(const char *job_id, const char *device,
+ const char *target, bool has_format,
+ const char *format, enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
bool has_speed, int64_t speed,
bool has_bitmap, const char *bitmap,
@@ -1876,7 +1876,8 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
bdrv_drained_begin(blk_bs(blk));
state->bs = blk_bs(blk);
- do_drive_backup(backup->device, backup->target,
+ do_drive_backup(backup->has_job_id ? backup->job_id : NULL,
+ backup->device, backup->target,
backup->has_format, backup->format,
backup->sync,
backup->has_mode, backup->mode,
@@ -1921,8 +1922,8 @@ typedef struct BlockdevBackupState {
AioContext *aio_context;
} BlockdevBackupState;
-static void do_blockdev_backup(const char *device, const char *target,
- enum MirrorSyncMode sync,
+static void do_blockdev_backup(const char *job_id, const char *device,
+ const char *target, enum MirrorSyncMode sync,
bool has_speed, int64_t speed,
bool has_on_source_error,
BlockdevOnError on_source_error,
@@ -1968,8 +1969,8 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
state->bs = blk_bs(blk);
bdrv_drained_begin(state->bs);
- do_blockdev_backup(backup->device, backup->target,
- backup->sync,
+ do_blockdev_backup(backup->has_job_id ? backup->job_id : NULL,
+ backup->device, backup->target, backup->sync,
backup->has_speed, backup->speed,
backup->has_on_source_error, backup->on_source_error,
backup->has_on_target_error, backup->on_target_error,
@@ -3182,9 +3183,9 @@ out:
aio_context_release(aio_context);
}
-static void do_drive_backup(const char *device, const char *target,
- bool has_format, const char *format,
- enum MirrorSyncMode sync,
+static void do_drive_backup(const char *job_id, const char *device,
+ const char *target, bool has_format,
+ const char *format, enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
bool has_speed, int64_t speed,
bool has_bitmap, const char *bitmap,
@@ -3303,7 +3304,7 @@ static void do_drive_backup(const char *device, const char *target,
}
}
- backup_start(bs, target_bs, speed, sync, bmap,
+ backup_start(job_id, bs, target_bs, speed, sync, bmap,
on_source_error, on_target_error,
block_job_cb, bs, txn, &local_err);
bdrv_unref(target_bs);
@@ -3316,7 +3317,8 @@ out:
aio_context_release(aio_context);
}
-void qmp_drive_backup(const char *device, const char *target,
+void qmp_drive_backup(bool has_job_id, const char *job_id,
+ const char *device, const char *target,
bool has_format, const char *format,
enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
@@ -3326,7 +3328,8 @@ void qmp_drive_backup(const char *device, const char *target,
bool has_on_target_error, BlockdevOnError on_target_error,
Error **errp)
{
- return do_drive_backup(device, target, has_format, format, sync,
+ return do_drive_backup(has_job_id ? job_id : NULL, device, target,
+ has_format, format, sync,
has_mode, mode, has_speed, speed,
has_bitmap, bitmap,
has_on_source_error, on_source_error,
@@ -3339,8 +3342,8 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
return bdrv_named_nodes_list(errp);
}
-void do_blockdev_backup(const char *device, const char *target,
- enum MirrorSyncMode sync,
+void do_blockdev_backup(const char *job_id, const char *device,
+ const char *target, enum MirrorSyncMode sync,
bool has_speed, int64_t speed,
bool has_on_source_error,
BlockdevOnError on_source_error,
@@ -3395,7 +3398,7 @@ void do_blockdev_backup(const char *device, const char *target,
goto out;
}
}
- backup_start(bs, target_bs, speed, sync, NULL, on_source_error,
+ backup_start(job_id, bs, target_bs, speed, sync, NULL, on_source_error,
on_target_error, block_job_cb, bs, txn, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
@@ -3404,7 +3407,8 @@ out:
aio_context_release(aio_context);
}
-void qmp_blockdev_backup(const char *device, const char *target,
+void qmp_blockdev_backup(bool has_job_id, const char *job_id,
+ const char *device, const char *target,
enum MirrorSyncMode sync,
bool has_speed, int64_t speed,
bool has_on_source_error,
@@ -3413,7 +3417,8 @@ void qmp_blockdev_backup(const char *device, const char *target,
BlockdevOnError on_target_error,
Error **errp)
{
- do_blockdev_backup(device, target, sync, has_speed, speed,
+ do_blockdev_backup(has_job_id ? job_id : NULL, device, target,
+ sync, has_speed, speed,
has_on_source_error, on_source_error,
has_on_target_error, on_target_error,
NULL, errp);
diff --git a/hmp.c b/hmp.c
index 087ae53..35eceb5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1127,7 +1127,7 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
}
- qmp_drive_backup(device, filename, !!format, format,
+ qmp_drive_backup(false, NULL, device, filename, !!format, format,
full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
true, mode, false, 0, false, NULL,
false, 0, false, 0, &err);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index dd45fec..b433b79 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -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,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6f015c8..e30e38d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9eee869..720d6f2 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1212,8 +1212,8 @@ EQMP
{
.name = "drive-backup",
- .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
- "bitmap:s?,on-source-error:s?,on-target-error:s?",
+ .args_type = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?,"
+ "format:s?,bitmap:s?,on-source-error:s?,on-target-error:s?",
.mhandler.cmd_new = qmp_marshal_drive_backup,
},
@@ -1229,6 +1229,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)
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 08/15] stream: Add 'job-id' parameter to 'block-stream'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (6 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 07/15] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 09/15] commit: Add 'job-id' parameter to 'block-commit' Alberto Garcia
` (6 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This patch adds a new optional 'job-id' parameter to 'block-stream',
allowing the user to specify the ID of the block job to be created.
The HMP 'block_stream' command remains unchanged.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
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(-)
diff --git a/block/stream.c b/block/stream.c
index e4319d3..54c8cd8 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -218,15 +218,15 @@ static const BlockJobDriver stream_job_driver = {
.set_speed = stream_set_speed,
};
-void stream_start(BlockDriverState *bs, BlockDriverState *base,
- const char *backing_file_str, int64_t speed,
- BlockdevOnError on_error,
- BlockCompletionFunc *cb,
- void *opaque, Error **errp)
+void stream_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, const char *backing_file_str,
+ int64_t speed, BlockdevOnError on_error,
+ BlockCompletionFunc *cb, void *opaque, Error **errp)
{
StreamBlockJob *s;
- s = block_job_create(NULL, &stream_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(job_id, &stream_job_driver, bs, speed,
+ cb, opaque, errp);
if (!s) {
return;
}
diff --git a/blockdev.c b/blockdev.c
index d23ef42..ac3b079 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3005,7 +3005,7 @@ static void block_job_cb(void *opaque, int ret)
}
}
-void qmp_block_stream(const char *device,
+void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
bool has_base, const char *base,
bool has_backing_file, const char *backing_file,
bool has_speed, int64_t speed,
@@ -3064,8 +3064,8 @@ void qmp_block_stream(const char *device,
/* backing_file string overrides base bs filename */
base_name = has_backing_file ? backing_file : base_name;
- stream_start(bs, base_bs, base_name, has_speed ? speed : 0,
- on_error, block_job_cb, bs, &local_err);
+ stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name,
+ has_speed ? speed : 0, on_error, block_job_cb, bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
diff --git a/hmp.c b/hmp.c
index 35eceb5..6b7786e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1485,7 +1485,7 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
const char *base = qdict_get_try_str(qdict, "base");
int64_t speed = qdict_get_try_int(qdict, "speed", 0);
- qmp_block_stream(device, base != NULL, base, false, NULL,
+ qmp_block_stream(false, NULL, device, base != NULL, base, false, NULL,
qdict_haskey(qdict, "speed"), speed,
true, BLOCKDEV_ON_ERROR_REPORT, &error);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index b433b79..9c0ecf5 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -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:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e30e38d..14073f7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 720d6f2..8018dc3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1106,7 +1106,7 @@ EQMP
{
.name = "block-stream",
- .args_type = "device:B,base:s?,speed:o?,backing-file:s?,on-error:s?",
+ .args_type = "job-id:s?,device:B,base:s?,speed:o?,backing-file:s?,on-error:s?",
.mhandler.cmd_new = qmp_marshal_block_stream,
},
@@ -1118,6 +1118,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)
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 09/15] commit: Add 'job-id' parameter to 'block-commit'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (7 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 08/15] stream: Add 'job-id' parameter to 'block-stream' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 10/15] blockjob: Add 'id' parameter to 'block-job-set-speed' Alberto Garcia
` (5 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This patch adds a new optional 'job-id' parameter to 'block-commit',
allowing the user to specify the ID of the block job to be created.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
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(-)
diff --git a/block/commit.c b/block/commit.c
index 3535ba7..6e576d9 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -211,8 +211,8 @@ static const BlockJobDriver commit_job_driver = {
.set_speed = commit_set_speed,
};
-void commit_start(BlockDriverState *bs, BlockDriverState *base,
- BlockDriverState *top, int64_t speed,
+void commit_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, BlockDriverState *top, int64_t speed,
BlockdevOnError on_error, BlockCompletionFunc *cb,
void *opaque, const char *backing_file_str, Error **errp)
{
@@ -236,7 +236,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
return;
}
- s = block_job_create(NULL, &commit_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(job_id, &commit_job_driver, bs, speed,
+ cb, opaque, errp);
if (!s) {
return;
}
diff --git a/block/mirror.c b/block/mirror.c
index 960934b..a453e8b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -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);
diff --git a/blockdev.c b/blockdev.c
index ac3b079..4c2f659 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3077,7 +3077,7 @@ out:
aio_context_release(aio_context);
}
-void qmp_block_commit(const char *device,
+void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,
bool has_base, const char *base,
bool has_top, const char *top,
bool has_backing_file, const char *backing_file,
@@ -3168,10 +3168,11 @@ void qmp_block_commit(const char *device,
" but 'top' is the active layer");
goto out;
}
- commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
- bs, &local_err);
+ commit_active_start(has_job_id ? job_id : NULL, bs, base_bs, speed,
+ on_error, block_job_cb, bs, &local_err);
} else {
- commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
+ commit_start(has_job_id ? job_id : NULL, bs, base_bs, top_bs, speed,
+ on_error, block_job_cb, bs,
has_backing_file ? backing_file : NULL, &local_err);
}
if (local_err != NULL) {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 9c0ecf5..e77f8fc 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -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);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 14073f7..f754c29 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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' } }
##
diff --git a/qemu-img.c b/qemu-img.c
index 14e2661..0083a74 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -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;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8018dc3..9ee14a8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -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
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 10/15] blockjob: Add 'id' parameter to 'block-job-set-speed'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (8 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 09/15] commit: Add 'job-id' parameter to 'block-commit' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 11/15] blockjob: Add 'id' parameter to 'block-job-cancel' Alberto Garcia
` (4 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This 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.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 7 +++++--
hmp.c | 2 +-
qapi/block-core.json | 9 +++++++--
qmp-commands.hx | 2 +-
4 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 4c2f659..bd276ee 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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;
diff --git a/hmp.c b/hmp.c
index 6b7786e..f498ccd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -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);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f754c29..64038cc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9ee14a8..cc70007 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -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,
},
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 11/15] blockjob: Add 'id' parameter to 'block-job-cancel'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (9 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 10/15] blockjob: Add 'id' parameter to 'block-job-set-speed' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 12/15] blockjob: Add 'id' parameter to 'block-job-pause' Alberto Garcia
` (3 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This 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.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 7 +++++--
hmp.c | 2 +-
qapi/block-core.json | 10 ++++++++--
qmp-commands.hx | 2 +-
4 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index bd276ee..3a0b22a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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;
diff --git a/hmp.c b/hmp.c
index f498ccd..ed5a125 100644
--- a/hmp.c
+++ b/hmp.c
@@ -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);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 64038cc..38dd3f2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cc70007..c7d207a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -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,
},
{
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 12/15] blockjob: Add 'id' parameter to 'block-job-pause'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (10 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 11/15] blockjob: Add 'id' parameter to 'block-job-cancel' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 13/15] blockjob: Add 'id' parameter to 'block-job-resume' Alberto Garcia
` (2 subsequent siblings)
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This 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.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 7 +++++--
hmp.c | 2 +-
qapi/block-core.json | 9 +++++++--
qmp-commands.hx | 2 +-
4 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 3a0b22a..348eaef 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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;
diff --git a/hmp.c b/hmp.c
index ed5a125..1f04129 100644
--- a/hmp.c
+++ b/hmp.c
@@ -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);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 38dd3f2..0eaf531 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c7d207a..bd7f749 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -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,
},
{
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 13/15] blockjob: Add 'id' parameter to 'block-job-resume'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (11 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 12/15] blockjob: Add 'id' parameter to 'block-job-pause' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 14/15] blockjob: Add 'id' parameter to 'block-job-complete' Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 15/15] blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events Alberto Garcia
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This 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.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 7 +++++--
hmp.c | 2 +-
qapi/block-core.json | 7 ++++++-
qmp-commands.hx | 2 +-
4 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 348eaef..5d8d405 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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;
diff --git a/hmp.c b/hmp.c
index 1f04129..8846673 100644
--- a/hmp.c
+++ b/hmp.c
@@ -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);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0eaf531..0efa73d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index bd7f749..b2020f3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -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,
},
{
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 14/15] blockjob: Add 'id' parameter to 'block-job-complete'
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (12 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 13/15] blockjob: Add 'id' parameter to 'block-job-resume' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 15/15] blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events Alberto Garcia
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
This 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.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
blockdev.c | 7 +++++--
hmp.c | 2 +-
qapi/block-core.json | 7 ++++++-
qmp-commands.hx | 2 +-
4 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 5d8d405..9bb1033 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -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;
diff --git a/hmp.c b/hmp.c
index 8846673..c100ad8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -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);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0efa73d..f15e62f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b2020f3..df43d18 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -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,
},
{
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Qemu-devel] [PATCH v2 15/15] blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events
2016-06-22 12:24 [Qemu-devel] [PATCH v2 00/15] Add an 'id' field to block jobs Alberto Garcia
` (13 preceding siblings ...)
2016-06-22 12:25 ` [Qemu-devel] [PATCH v2 14/15] blockjob: Add 'id' parameter to 'block-job-complete' Alberto Garcia
@ 2016-06-22 12:25 ` Alberto Garcia
14 siblings, 0 replies; 27+ messages in thread
From: Alberto Garcia @ 2016-06-22 12:25 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Eric Blake, Kevin Wolf, Jeff Cody, Alberto Garcia
Now 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.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
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(-)
diff --git a/blockjob.c b/blockjob.c
index c7f6992..5f2b8de 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -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);
diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index fa7574d..2b2af0f 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -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)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f15e62f..b9544d1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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',
diff --git a/tests/qemu-iotests/095 b/tests/qemu-iotests/095
index 030adb2..34617a7 100755
--- a/tests/qemu-iotests/095
+++ b/tests/qemu-iotests/095
@@ -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
diff --git a/tests/qemu-iotests/095.out b/tests/qemu-iotests/095.out
index 73875ca..c146c72 100644
--- a/tests/qemu-iotests/095.out
+++ b/tests/qemu-iotests/095.out
@@ -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
diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
index adf9889..d23ede0 100755
--- a/tests/qemu-iotests/109
+++ b/tests/qemu-iotests/109
@@ -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"
diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
index 7c797ed..a2fc346 100644
--- a/tests/qemu-iotests/109.out
+++ b/tests/qemu-iotests/109.out
@@ -9,15 +9,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 1024, "offset": 1024, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -30,15 +30,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 197120, "offset": 512, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 197120, "offset": 512, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 197120, "offset": 197120, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 197120, "offset": 197120, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 197120, "offset": 197120, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 197120, "offset": 197120, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -51,15 +51,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 327680, "offset": 262144, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 327680, "offset": 262144, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 327680, "offset": 327680, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 327680, "offset": 327680, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 327680, "offset": 327680, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 327680, "offset": 327680, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -72,15 +72,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 1024, "offset": 1024, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -93,15 +93,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 65536, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 65536, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -114,15 +114,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -134,15 +134,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -154,15 +154,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 31457280, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 31457280, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 31457280, "offset": 31457280, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 31457280, "offset": 31457280, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 31457280, "offset": 31457280, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 31457280, "offset": 31457280, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -174,15 +174,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 327680, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 327680, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 327680, "offset": 327680, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 327680, "offset": 327680, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 327680, "offset": 327680, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 327680, "offset": 327680, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -194,15 +194,15 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2048, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report", "id": "mirror1"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2048, "offset": 0, "speed": 0, "type": "mirror", "id": "mirror1", "error": "Operation not permitted"}}
{"return": []}
read 65536/65536 bytes at offset 0
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_READY", "data": {"device": "src", "len": 2048, "offset": 2048, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2048, "offset": 2048, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2048, "offset": 2048, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2048, "offset": 2048, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
@@ -214,14 +214,14 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 512, "offset": 512, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 512, "offset": 512, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 512, "offset": 512, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 512, "offset": 512, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
{"return": {}}
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 512, "offset": 512, "speed": 0, "type": "mirror"}}
-{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 512, "offset": 512, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 512, "offset": 512, "speed": 0, "type": "mirror", "id": "mirror1"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 512, "offset": 512, "paused": false, "speed": 0, "ready": true, "type": "mirror", "id": "mirror1"}]}
Warning: Image size mismatch!
Images are identical.
*** done
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index de7cdbe..89d74cb 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -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))
diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141
index b2617e5..85d9d9d 100755
--- a/tests/qemu-iotests/141
+++ b/tests/qemu-iotests/141
@@ -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'
diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out
index adceac1..36c72bc 100644
--- a/tests/qemu-iotests/141.out
+++ b/tests/qemu-iotests/141.out
@@ -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
diff --git a/tests/qemu-iotests/144 b/tests/qemu-iotests/144
index 00de3c3..09cebcc 100755
--- a/tests/qemu-iotests/144
+++ b/tests/qemu-iotests/144
@@ -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"
diff --git a/tests/qemu-iotests/144.out b/tests/qemu-iotests/144.out
index 410d741..3208c1f 100644
--- a/tests/qemu-iotests/144.out
+++ b/tests/qemu-iotests/144.out
@@ -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 ===
diff --git a/tests/qemu-iotests/156 b/tests/qemu-iotests/156
index cc95ff1..f3d9842 100755
--- a/tests/qemu-iotests/156
+++ b/tests/qemu-iotests/156
@@ -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' } }" \
diff --git a/tests/qemu-iotests/156.out b/tests/qemu-iotests/156.out
index 3af82ae..ced3be3 100644
--- a/tests/qemu-iotests/156.out
+++ b/tests/qemu-iotests/156.out
@@ -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)
--
2.8.1
^ permalink raw reply related [flat|nested] 27+ messages in thread