* [Qemu-devel] [PATCH v3 01/11] stream: Fix prototype of stream_start()
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
@ 2016-07-01 15:51 ` Alberto Garcia
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 02/11] blockjob: Update description of the 'id' field Alberto Garcia
` (10 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:51 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block_int.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 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] 23+ messages in thread* [Qemu-devel] [PATCH v3 02/11] blockjob: Update description of the 'id' field
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
2016-07-01 15:51 ` [Qemu-devel] [PATCH v3 01/11] stream: Fix prototype of stream_start() Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-02 13:47 ` Max Reitz
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 03/11] blockjob: Add block_job_get() Alberto Garcia
` (9 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, Alberto Garcia
The 'id' field of the BlockJob structure will be able to hold any ID,
not only a device name. This patch updates the description of that
field and the error messages where it is being used.
Soon we'll add the ability to set an arbitrary ID when creating a
block job.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
block/mirror.c | 3 ++-
blockjob.c | 3 ++-
include/block/blockjob.h | 5 +----
include/qapi/qmp/qerror.h | 3 ---
4 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 8d96049..6e3dbd2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -761,7 +761,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
target = blk_bs(s->target);
if (!s->synced) {
- error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
+ error_setg(errp, "The active block job '%s' cannot be completed",
+ job->id);
return;
}
diff --git a/blockjob.c b/blockjob.c
index 205da9d..ce0e27c 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -290,7 +290,8 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
void block_job_complete(BlockJob *job, Error **errp)
{
if (job->pause_count || job->cancelled || !job->driver->complete) {
- error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
+ error_setg(errp, "The active block job '%s' cannot be completed",
+ job->id);
return;
}
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 7dc720c..5181514 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -106,10 +106,7 @@ struct BlockJob {
BlockBackend *blk;
/**
- * The ID of the block job. Currently the BlockBackend name of the BDS
- * owning the job at the time when the job is started.
- *
- * TODO Decouple block job IDs from BlockBackend names
+ * The ID of the block job.
*/
char *id;
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index d08652a..6586c9f 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -19,9 +19,6 @@
#define QERR_BASE_NOT_FOUND \
"Base '%s' not found"
-#define QERR_BLOCK_JOB_NOT_READY \
- "The active block job for device '%s' cannot be completed"
-
#define QERR_BUS_NO_HOTPLUG \
"Bus '%s' does not support hotplugging"
--
2.8.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 02/11] blockjob: Update description of the 'id' field
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 02/11] blockjob: Update description of the 'id' field Alberto Garcia
@ 2016-07-02 13:47 ` Max Reitz
0 siblings, 0 replies; 23+ messages in thread
From: Max Reitz @ 2016-07-02 13:47 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Jeff Cody, John Snow
[-- Attachment #1: Type: text/plain, Size: 648 bytes --]
On 01.07.2016 17:52, Alberto Garcia wrote:
> The 'id' field of the BlockJob structure will be able to hold any ID,
> not only a device name. This patch updates the description of that
> field and the error messages where it is being used.
>
> Soon we'll add the ability to set an arbitrary ID when creating a
> block job.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> block/mirror.c | 3 ++-
> blockjob.c | 3 ++-
> include/block/blockjob.h | 5 +----
> include/qapi/qmp/qerror.h | 3 ---
> 4 files changed, 5 insertions(+), 9 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 03/11] blockjob: Add block_job_get()
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
2016-07-01 15:51 ` [Qemu-devel] [PATCH v3 01/11] stream: Fix prototype of stream_start() Alberto Garcia
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 02/11] blockjob: Update description of the 'id' field Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 04/11] block: Use block_job_get() in find_block_job() Alberto Garcia
` (8 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, 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>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
blockjob.c | 13 +++++++++++++
include/block/blockjob.h | 10 ++++++++++
2 files changed, 23 insertions(+)
diff --git a/blockjob.c b/blockjob.c
index ce0e27c..ca2291b 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -60,6 +60,19 @@ BlockJob *block_job_next(BlockJob *job)
return QLIST_NEXT(job, job_list);
}
+BlockJob *block_job_get(const char *id)
+{
+ BlockJob *job;
+
+ QLIST_FOREACH(job, &block_jobs, job_list) {
+ if (!strcmp(id, job->id)) {
+ return job;
+ }
+ }
+
+ return NULL;
+}
+
/* Normally the job runs in its BlockBackend's AioContext. The exception is
* block_job_defer_to_main_loop() where it runs in the QEMU main loop. Code
* that supports both cases uses this helper function.
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 5181514..0fe1540 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -211,6 +211,16 @@ struct BlockJob {
BlockJob *block_job_next(BlockJob *job);
/**
+ * block_job_get:
+ * @id: The id of the block job.
+ *
+ * Get the block job identified by @id (which must not be %NULL).
+ *
+ * Returns the requested job, or %NULL if it doesn't exist.
+ */
+BlockJob *block_job_get(const char *id);
+
+/**
* block_job_create:
* @job_type: The class object for the newly-created job.
* @bs: The block
--
2.8.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PATCH v3 04/11] block: Use block_job_get() in find_block_job()
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (2 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 03/11] blockjob: Add block_job_get() Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-02 14:02 ` Max Reitz
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 05/11] blockjob: Add 'job_id' parameter to block_job_create() Alberto Garcia
` (7 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, 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.
We want to identify jobs by their ID and not by the block backend
they're attached to, so this patch ignores the backends altogether and
gets the job directly. Apart from making the code simpler, this will
allow us to find block jobs once they start having user-specified IDs.
To ensure backward compatibility we keep ERROR_CLASS_DEVICE_NOT_ACTIVE
as the error class if the job doesn't exist. In subsequent patches
we'll also need to keep the device name as the default job ID if the
user doesn't specify a different one.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
blockdev.c | 43 ++++++++++++++++---------------------------
1 file changed, 16 insertions(+), 27 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 3a104a0..8cedb60 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3704,42 +3704,31 @@ void qmp_blockdev_mirror(const char *device, const char *target,
aio_context_release(aio_context);
}
-/* Get the block job for a given device name and acquire its AioContext */
-static BlockJob *find_block_job(const char *device, AioContext **aio_context,
+/* Get a block job using its ID and acquire its AioContext */
+static BlockJob *find_block_job(const char *id, AioContext **aio_context,
Error **errp)
{
- BlockBackend *blk;
- BlockDriverState *bs;
+ BlockJob *job;
*aio_context = NULL;
- blk = blk_by_name(device);
- if (!blk) {
- goto notfound;
+ if (!id) {
+ error_setg(errp, "Unspecified job ID when looking for a block job");
+ return NULL;
}
- *aio_context = blk_get_aio_context(blk);
+ job = block_job_get(id);
+
+ if (!job) {
+ error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
+ "Block job '%s' not found", id);
+ return NULL;
+ }
+
+ *aio_context = blk_get_aio_context(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)
--
2.8.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/11] block: Use block_job_get() in find_block_job()
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 04/11] block: Use block_job_get() in find_block_job() Alberto Garcia
@ 2016-07-02 14:02 ` Max Reitz
2016-07-04 13:23 ` Kevin Wolf
2016-07-04 13:35 ` Alberto Garcia
0 siblings, 2 replies; 23+ messages in thread
From: Max Reitz @ 2016-07-02 14:02 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Jeff Cody, John Snow
[-- Attachment #1: Type: text/plain, Size: 3293 bytes --]
On 01.07.2016 17:52, Alberto Garcia wrote:
> find_block_job() looks for a block backend with a specified name,
> checks whether it has a block job and acquires its AioContext.
>
> We want to identify jobs by their ID and not by the block backend
> they're attached to, so this patch ignores the backends altogether and
> gets the job directly. Apart from making the code simpler, this will
> allow us to find block jobs once they start having user-specified IDs.
>
> To ensure backward compatibility we keep ERROR_CLASS_DEVICE_NOT_ACTIVE
> as the error class if the job doesn't exist. In subsequent patches
> we'll also need to keep the device name as the default job ID if the
> user doesn't specify a different one.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> blockdev.c | 43 ++++++++++++++++---------------------------
> 1 file changed, 16 insertions(+), 27 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 3a104a0..8cedb60 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3704,42 +3704,31 @@ void qmp_blockdev_mirror(const char *device, const char *target,
> aio_context_release(aio_context);
> }
>
> -/* Get the block job for a given device name and acquire its AioContext */
> -static BlockJob *find_block_job(const char *device, AioContext **aio_context,
> +/* Get a block job using its ID and acquire its AioContext */
> +static BlockJob *find_block_job(const char *id, AioContext **aio_context,
> Error **errp)
> {
> - BlockBackend *blk;
> - BlockDriverState *bs;
> + BlockJob *job;
>
> *aio_context = NULL;
>
> - blk = blk_by_name(device);
> - if (!blk) {
> - goto notfound;
> + if (!id) {
> + error_setg(errp, "Unspecified job ID when looking for a block job");
> + return NULL;
> }
Why no plain assertion? Do you expect callers who may pass a NULL ID?
>
> - *aio_context = blk_get_aio_context(blk);
> + job = block_job_get(id);
> +
> + if (!job) {
> + error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
> + "Block job '%s' not found", id);
This error class seems a bit weird now... I know I advocated for it in
v2, but that was because you could actually specifically pass a device
name to find block jobs on that device, but now you're just looking for
block jobs with a certain ID (that happens to default to the device name).
Anyway, nothing's really wrong, so:
Reviewed-by: Max Reitz <mreitz@redhat.com>
> + 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)
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/11] block: Use block_job_get() in find_block_job()
2016-07-02 14:02 ` Max Reitz
@ 2016-07-04 13:23 ` Kevin Wolf
2016-07-04 14:05 ` Daniel P. Berrange
2016-07-04 13:35 ` Alberto Garcia
1 sibling, 1 reply; 23+ messages in thread
From: Kevin Wolf @ 2016-07-04 13:23 UTC (permalink / raw)
To: Max Reitz
Cc: Alberto Garcia, qemu-devel, qemu-block, Eric Blake, Jeff Cody,
John Snow
[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]
Am 02.07.2016 um 16:02 hat Max Reitz geschrieben:
> On 01.07.2016 17:52, Alberto Garcia wrote:
> > find_block_job() looks for a block backend with a specified name,
> > checks whether it has a block job and acquires its AioContext.
> >
> > We want to identify jobs by their ID and not by the block backend
> > they're attached to, so this patch ignores the backends altogether and
> > gets the job directly. Apart from making the code simpler, this will
> > allow us to find block jobs once they start having user-specified IDs.
> >
> > To ensure backward compatibility we keep ERROR_CLASS_DEVICE_NOT_ACTIVE
> > as the error class if the job doesn't exist. In subsequent patches
> > we'll also need to keep the device name as the default job ID if the
> > user doesn't specify a different one.
> >
> > Signed-off-by: Alberto Garcia <berto@igalia.com>
> > ---
> > blockdev.c | 43 ++++++++++++++++---------------------------
> > 1 file changed, 16 insertions(+), 27 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 3a104a0..8cedb60 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -3704,42 +3704,31 @@ void qmp_blockdev_mirror(const char *device, const char *target,
> > aio_context_release(aio_context);
> > }
> >
> > -/* Get the block job for a given device name and acquire its AioContext */
> > -static BlockJob *find_block_job(const char *device, AioContext **aio_context,
> > +/* Get a block job using its ID and acquire its AioContext */
> > +static BlockJob *find_block_job(const char *id, AioContext **aio_context,
> > Error **errp)
> > {
> > - BlockBackend *blk;
> > - BlockDriverState *bs;
> > + BlockJob *job;
> >
> > *aio_context = NULL;
> >
> > - blk = blk_by_name(device);
> > - if (!blk) {
> > - goto notfound;
> > + if (!id) {
> > + error_setg(errp, "Unspecified job ID when looking for a block job");
> > + return NULL;
> > }
>
> Why no plain assertion? Do you expect callers who may pass a NULL ID?
>
> >
> > - *aio_context = blk_get_aio_context(blk);
> > + job = block_job_get(id);
> > +
> > + if (!job) {
> > + error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
> > + "Block job '%s' not found", id);
>
> This error class seems a bit weird now... I know I advocated for it in
> v2, but that was because you could actually specifically pass a device
> name to find block jobs on that device, but now you're just looking for
> block jobs with a certain ID (that happens to default to the device name).
libvirt uses the error class, so I don't think we can drop it.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/11] block: Use block_job_get() in find_block_job()
2016-07-04 13:23 ` Kevin Wolf
@ 2016-07-04 14:05 ` Daniel P. Berrange
0 siblings, 0 replies; 23+ messages in thread
From: Daniel P. Berrange @ 2016-07-04 14:05 UTC (permalink / raw)
To: Kevin Wolf
Cc: Max Reitz, Alberto Garcia, qemu-block, Jeff Cody, qemu-devel,
John Snow
On Mon, Jul 04, 2016 at 03:23:14PM +0200, Kevin Wolf wrote:
> Am 02.07.2016 um 16:02 hat Max Reitz geschrieben:
> > On 01.07.2016 17:52, Alberto Garcia wrote:
> > > find_block_job() looks for a block backend with a specified name,
> > > checks whether it has a block job and acquires its AioContext.
> > >
> > > We want to identify jobs by their ID and not by the block backend
> > > they're attached to, so this patch ignores the backends altogether and
> > > gets the job directly. Apart from making the code simpler, this will
> > > allow us to find block jobs once they start having user-specified IDs.
> > >
> > > To ensure backward compatibility we keep ERROR_CLASS_DEVICE_NOT_ACTIVE
> > > as the error class if the job doesn't exist. In subsequent patches
> > > we'll also need to keep the device name as the default job ID if the
> > > user doesn't specify a different one.
> > >
> > > Signed-off-by: Alberto Garcia <berto@igalia.com>
> > > ---
> > > blockdev.c | 43 ++++++++++++++++---------------------------
> > > 1 file changed, 16 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/blockdev.c b/blockdev.c
> > > index 3a104a0..8cedb60 100644
> > > --- a/blockdev.c
> > > +++ b/blockdev.c
> > > @@ -3704,42 +3704,31 @@ void qmp_blockdev_mirror(const char *device, const char *target,
> > > aio_context_release(aio_context);
> > > }
> > >
> > > -/* Get the block job for a given device name and acquire its AioContext */
> > > -static BlockJob *find_block_job(const char *device, AioContext **aio_context,
> > > +/* Get a block job using its ID and acquire its AioContext */
> > > +static BlockJob *find_block_job(const char *id, AioContext **aio_context,
> > > Error **errp)
> > > {
> > > - BlockBackend *blk;
> > > - BlockDriverState *bs;
> > > + BlockJob *job;
> > >
> > > *aio_context = NULL;
> > >
> > > - blk = blk_by_name(device);
> > > - if (!blk) {
> > > - goto notfound;
> > > + if (!id) {
> > > + error_setg(errp, "Unspecified job ID when looking for a block job");
> > > + return NULL;
> > > }
> >
> > Why no plain assertion? Do you expect callers who may pass a NULL ID?
> >
> > >
> > > - *aio_context = blk_get_aio_context(blk);
> > > + job = block_job_get(id);
> > > +
> > > + if (!job) {
> > > + error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
> > > + "Block job '%s' not found", id);
> >
> > This error class seems a bit weird now... I know I advocated for it in
> > v2, but that was because you could actually specifically pass a device
> > name to find block jobs on that device, but now you're just looking for
> > block jobs with a certain ID (that happens to default to the device name).
>
> libvirt uses the error class, so I don't think we can drop it.
libvirt checks for the following when seeing block job errors.
if (qemuMonitorJSONErrorIsClass(error, "DeviceNotActive")) {
virReportError(VIR_ERR_OPERATION_INVALID,
_("No active operation on device: %s"), device);
} else if (qemuMonitorJSONErrorIsClass(error, "DeviceInUse")) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("Device %s in use"), device);
} else if (qemuMonitorJSONErrorIsClass(error, "NotSupported")) {
virReportError(VIR_ERR_OPERATION_INVALID,
_("Operation is not supported for device: %s"), device);
} else if (qemuMonitorJSONErrorIsClass(error, "CommandNotFound")) {
virReportError(VIR_ERR_OPERATION_INVALID,
_("Command '%s' is not found"), cmd_name);
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected error: (%s) '%s'"),
NULLSTR(virJSONValueObjectGetString(error, "class")),
NULLSTR(virJSONValueObjectGetString(error, "desc")));
}
So yes we use it, but if you changed it to a different error class it
won't neccessarily break libvirt - we'd just end up in the final else
case, and report a different error code + message. It is possible that
this might upset an app using libvirt that checked the error code but
its fairly slim chance.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 04/11] block: Use block_job_get() in find_block_job()
2016-07-02 14:02 ` Max Reitz
2016-07-04 13:23 ` Kevin Wolf
@ 2016-07-04 13:35 ` Alberto Garcia
1 sibling, 0 replies; 23+ messages in thread
From: Alberto Garcia @ 2016-07-04 13:35 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Jeff Cody, John Snow
On Sat 02 Jul 2016 04:02:11 PM CEST, Max Reitz wrote:
>> +/* Get a block job using its ID and acquire its AioContext */
>> +static BlockJob *find_block_job(const char *id, AioContext **aio_context,
>> Error **errp)
>> {
>> - BlockBackend *blk;
>> - BlockDriverState *bs;
>> + BlockJob *job;
>>
>> *aio_context = NULL;
>>
>> - blk = blk_by_name(device);
>> - if (!blk) {
>> - goto notfound;
>> + if (!id) {
>> + error_setg(errp, "Unspecified job ID when looking for a block job");
>> + return NULL;
>> }
>
> Why no plain assertion? Do you expect callers who may pass a NULL ID?
I think you're right, I'll use an assertion instead. I assume I can keep
your R-b if I just change that ?
Berto
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 05/11] blockjob: Add 'job_id' parameter to block_job_create()
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (3 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 04/11] block: Use block_job_get() in find_block_job() Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-02 14:09 ` Max Reitz
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 06/11] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' Alberto Garcia
` (6 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, Alberto Garcia
When a new job is created, the job ID is taken from the device name of
the BDS. This patch adds a new 'job_id' parameter to let the caller
provide one instead.
This patch also verifies that the ID is always unique and well-formed.
This causes problems in a couple of places where no ID is being set,
because the BDS does not have a device name.
In the case of test_block_job_start() (from test-blockjob-txn.c) we
can simply use this new 'job_id' parameter to set the missing ID.
In the case of img_commit() (from qemu-img.c) we still don't have the
API to make commit_active_start() set the job ID, so we solve it by
setting a default value. We'll get rid of this as soon as we extend
the API.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
block/backup.c | 3 ++-
block/commit.c | 2 +-
block/mirror.c | 2 +-
block/stream.c | 2 +-
blockjob.c | 29 +++++++++++++++++++++++++----
include/block/blockjob.h | 8 +++++---
tests/test-blockjob-txn.c | 7 +++++--
7 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index f87f8d5..19c587c 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -541,7 +541,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
goto error;
}
- job = block_job_create(&backup_job_driver, bs, speed, cb, opaque, errp);
+ job = block_job_create(NULL, &backup_job_driver, bs, speed,
+ cb, opaque, errp);
if (!job) {
goto error;
}
diff --git a/block/commit.c b/block/commit.c
index 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 6e3dbd2..08d88ca 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -873,7 +873,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
buf_size = DEFAULT_MIRROR_BUF_SIZE;
}
- s = block_job_create(driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(NULL, driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/block/stream.c b/block/stream.c
index c0efbda..e4319d3 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -226,7 +226,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base,
{
StreamBlockJob *s;
- s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(NULL, &stream_job_driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
diff --git a/blockjob.c b/blockjob.c
index ca2291b..511c0db 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -33,6 +33,7 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qjson.h"
#include "qemu/coroutine.h"
+#include "qemu/id.h"
#include "qmp-commands.h"
#include "qemu/timer.h"
#include "qapi-event.h"
@@ -116,9 +117,9 @@ static void block_job_detach_aio_context(void *opaque)
block_job_unref(job);
}
-void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
- int64_t speed, BlockCompletionFunc *cb,
- void *opaque, Error **errp)
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+ BlockDriverState *bs, int64_t speed,
+ BlockCompletionFunc *cb, void *opaque, Error **errp)
{
BlockBackend *blk;
BlockJob *job;
@@ -129,6 +130,26 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
return NULL;
}
+ if (job_id == NULL) {
+ job_id = bdrv_get_device_name(bs);
+ /* Assign a default ID if the BDS does not have a device
+ * name. We'll get rid of this soon when we finish extending
+ * the API of all commands that create block jobs. */
+ if (job_id[0] == '\0') {
+ job_id = "default_job";
+ }
+ }
+
+ if (!id_wellformed(job_id)) {
+ error_setg(errp, "Invalid job ID '%s'", job_id);
+ return NULL;
+ }
+
+ if (block_job_get(job_id)) {
+ error_setg(errp, "Job ID '%s' already in use", job_id);
+ return NULL;
+ }
+
blk = blk_new();
blk_insert_bs(blk, bs);
@@ -139,7 +160,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
job->driver = driver;
- job->id = g_strdup(bdrv_get_device_name(bs));
+ job->id = g_strdup(job_id);
job->blk = blk;
job->cb = cb;
job->opaque = opaque;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 0fe1540..7f6d924 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -222,6 +222,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.
@@ -238,9 +240,9 @@ BlockJob *block_job_get(const char *id);
* This function is not part of the public job interface; it should be
* called from a wrapper that is specific to the job type.
*/
-void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
- int64_t speed, BlockCompletionFunc *cb,
- void *opaque, Error **errp);
+void *block_job_create(const char *job_id, const BlockJobDriver *driver,
+ BlockDriverState *bs, int64_t speed,
+ BlockCompletionFunc *cb, void *opaque, Error **errp);
/**
* block_job_sleep_ns:
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index d3030f1..50e232a 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -91,11 +91,14 @@ static BlockJob *test_block_job_start(unsigned int iterations,
BlockDriverState *bs;
TestBlockJob *s;
TestBlockJobCBData *data;
+ static unsigned counter;
+ char job_id[24];
data = g_new0(TestBlockJobCBData, 1);
bs = bdrv_new();
- s = block_job_create(&test_block_job_driver, bs, 0, test_block_job_cb,
- data, &error_abort);
+ snprintf(job_id, sizeof(job_id), "job%u", counter++);
+ s = block_job_create(job_id, &test_block_job_driver, bs, 0,
+ test_block_job_cb, data, &error_abort);
s->iterations = iterations;
s->use_timer = use_timer;
s->rc = rc;
--
2.8.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 05/11] blockjob: Add 'job_id' parameter to block_job_create()
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 05/11] blockjob: Add 'job_id' parameter to block_job_create() Alberto Garcia
@ 2016-07-02 14:09 ` Max Reitz
0 siblings, 0 replies; 23+ messages in thread
From: Max Reitz @ 2016-07-02 14:09 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Jeff Cody, John Snow
[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]
On 01.07.2016 17:52, Alberto Garcia wrote:
> When a new job is created, the job ID is taken from the device name of
> the BDS. This patch adds a new 'job_id' parameter to let the caller
> provide one instead.
>
> This patch also verifies that the ID is always unique and well-formed.
> This causes problems in a couple of places where no ID is being set,
> because the BDS does not have a device name.
>
> In the case of test_block_job_start() (from test-blockjob-txn.c) we
> can simply use this new 'job_id' parameter to set the missing ID.
>
> In the case of img_commit() (from qemu-img.c) we still don't have the
> API to make commit_active_start() set the job ID, so we solve it by
> setting a default value. We'll get rid of this as soon as we extend
> the API.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> block/backup.c | 3 ++-
> block/commit.c | 2 +-
> block/mirror.c | 2 +-
> block/stream.c | 2 +-
> blockjob.c | 29 +++++++++++++++++++++++++----
> include/block/blockjob.h | 8 +++++---
> tests/test-blockjob-txn.c | 7 +++++--
> 7 files changed, 40 insertions(+), 13 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 06/11] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (4 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 05/11] blockjob: Add 'job_id' parameter to block_job_create() Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-02 14:30 ` Max Reitz
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 07/11] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup' Alberto Garcia
` (5 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, 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 08d88ca..702a686 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -843,8 +843,8 @@ static const BlockJobDriver commit_active_job_driver = {
.attached_aio_context = mirror_attached_aio_context,
};
-static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
- const char *replaces,
+static void mirror_start_job(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, const char *replaces,
int64_t speed, uint32_t granularity,
int64_t buf_size,
BlockMirrorBackingMode backing_mode,
@@ -873,7 +873,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
buf_size = DEFAULT_MIRROR_BUF_SIZE;
}
- s = block_job_create(NULL, driver, bs, speed, cb, opaque, errp);
+ s = block_job_create(job_id, driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
@@ -906,8 +906,8 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
qemu_coroutine_enter(s->common.co, s);
}
-void mirror_start(BlockDriverState *bs, BlockDriverState *target,
- const char *replaces,
+void mirror_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, const char *replaces,
int64_t speed, uint32_t granularity, int64_t buf_size,
MirrorSyncMode mode, BlockMirrorBackingMode backing_mode,
BlockdevOnError on_source_error,
@@ -925,7 +925,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
}
is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
- mirror_start_job(bs, target, replaces,
+ mirror_start_job(job_id, bs, target, replaces,
speed, granularity, buf_size, backing_mode,
on_source_error, on_target_error, unmap, cb, opaque, errp,
&mirror_job_driver, is_none_mode, base);
@@ -973,7 +973,8 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
}
}
- mirror_start_job(bs, base, NULL, speed, 0, 0, MIRROR_LEAVE_BACKING_CHAIN,
+ mirror_start_job(NULL, bs, base, NULL, speed, 0, 0,
+ MIRROR_LEAVE_BACKING_CHAIN,
on_error, on_error, false, cb, opaque, &local_err,
&commit_active_job_driver, false, base);
if (local_err) {
diff --git a/blockdev.c b/blockdev.c
index 8cedb60..f0e98e0 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 925601a..3dd9943 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 6937e83..d9a9510 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] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/11] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror'
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 06/11] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' Alberto Garcia
@ 2016-07-02 14:30 ` Max Reitz
0 siblings, 0 replies; 23+ messages in thread
From: Max Reitz @ 2016-07-02 14:30 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Jeff Cody, John Snow
[-- Attachment #1: Type: text/plain, Size: 5773 bytes --]
On 01.07.2016 17:52, Alberto Garcia wrote:
> 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/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.
No, NULL to use the device name (same in the three following patches).
> * @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)
You should probably mention that this defaults to @device (as below and
in the three following patches).
Max
> +#
> # @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 6937e83..d9a9510 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
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 07/11] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup'
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (5 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 06/11] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 08/11] stream: Add 'job-id' parameter to 'block-stream' Alberto Garcia
` (4 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, 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 19c587c..dce6614 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -474,9 +474,9 @@ static void coroutine_fn backup_run(void *opaque)
block_job_defer_to_main_loop(&job->common, backup_complete, data);
}
-void backup_start(BlockDriverState *bs, BlockDriverState *target,
- int64_t speed, MirrorSyncMode sync_mode,
- BdrvDirtyBitmap *sync_bitmap,
+void backup_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *target, int64_t speed,
+ MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockCompletionFunc *cb, void *opaque,
@@ -541,7 +541,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
goto error;
}
- job = block_job_create(NULL, &backup_job_driver, bs, speed,
+ job = block_job_create(job_id, &backup_job_driver, bs, speed,
cb, opaque, errp);
if (!job) {
goto error;
diff --git a/blockdev.c b/blockdev.c
index f0e98e0..6144618 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 3dd9943..0ca480a 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 d9a9510..59583a6 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] 23+ messages in thread* [Qemu-devel] [PATCH v3 08/11] stream: Add 'job-id' parameter to 'block-stream'
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (6 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 07/11] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup' Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 09/11] commit: Add 'job-id' parameter to 'block-commit' Alberto Garcia
` (3 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, 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 6144618..d0f6a18 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 0ca480a..3733863 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 59583a6..2a550df 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] 23+ messages in thread* [Qemu-devel] [PATCH v3 09/11] commit: Add 'job-id' parameter to 'block-commit'
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (7 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 08/11] stream: Add 'job-id' parameter to 'block-stream' Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 10/11] qemu-img: Set the ID of the block job in img_commit() Alberto Garcia
` (2 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, 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 702a686..705fbc0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -931,8 +931,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
&mirror_job_driver, is_none_mode, base);
}
-void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
- int64_t speed,
+void commit_active_start(const char *job_id, BlockDriverState *bs,
+ BlockDriverState *base, int64_t speed,
BlockdevOnError on_error,
BlockCompletionFunc *cb,
void *opaque, Error **errp)
@@ -973,7 +973,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
}
}
- mirror_start_job(NULL, bs, base, NULL, speed, 0, 0,
+ mirror_start_job(job_id, bs, base, NULL, speed, 0, 0,
MIRROR_LEAVE_BACKING_CHAIN,
on_error, on_error, false, cb, opaque, &local_err,
&commit_active_job_driver, false, base);
diff --git a/blockdev.c b/blockdev.c
index d0f6a18..6b1f6ef 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 3322a1e..a78f598 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -921,7 +921,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 2a550df..d546955 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] 23+ messages in thread* [Qemu-devel] [PATCH v3 10/11] qemu-img: Set the ID of the block job in img_commit()
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (8 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 09/11] commit: Add 'job-id' parameter to 'block-commit' Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-02 14:21 ` Max Reitz
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 11/11] blockjob: Update description of the 'device' field in the QMP API Alberto Garcia
2016-07-04 15:49 ` [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Kevin Wolf
11 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, Alberto Garcia
img_commit() creates a block job without an ID. This is no longer
allowed now that we require it to be unique and well-formed. We were
solving this by having a fallback in block_job_create(), but now that
we extended the API of commit_active_start() we can finally set an
explicit ID and revert that change.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
blockjob.c | 6 ------
qemu-img.c | 2 +-
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index 511c0db..3b9cec7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -132,12 +132,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
if (job_id == NULL) {
job_id = bdrv_get_device_name(bs);
- /* Assign a default ID if the BDS does not have a device
- * name. We'll get rid of this soon when we finish extending
- * the API of all commands that create block jobs. */
- if (job_id[0] == '\0') {
- job_id = "default_job";
- }
}
if (!id_wellformed(job_id)) {
diff --git a/qemu-img.c b/qemu-img.c
index a78f598..521724c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -921,7 +921,7 @@ static int img_commit(int argc, char **argv)
.bs = bs,
};
- commit_active_start(NULL, bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
+ commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
common_block_job_cb, &cbi, &local_err);
if (local_err) {
goto done;
--
2.8.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 10/11] qemu-img: Set the ID of the block job in img_commit()
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 10/11] qemu-img: Set the ID of the block job in img_commit() Alberto Garcia
@ 2016-07-02 14:21 ` Max Reitz
2016-07-04 12:43 ` Alberto Garcia
0 siblings, 1 reply; 23+ messages in thread
From: Max Reitz @ 2016-07-02 14:21 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Jeff Cody, John Snow
[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]
On 01.07.2016 17:52, Alberto Garcia wrote:
> img_commit() creates a block job without an ID. This is no longer
> allowed now that we require it to be unique and well-formed. We were
> solving this by having a fallback in block_job_create(), but now that
> we extended the API of commit_active_start() we can finally set an
> explicit ID and revert that change.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> blockjob.c | 6 ------
> qemu-img.c | 2 +-
> 2 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index 511c0db..3b9cec7 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -132,12 +132,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
>
> if (job_id == NULL) {
> job_id = bdrv_get_device_name(bs);
> - /* Assign a default ID if the BDS does not have a device
> - * name. We'll get rid of this soon when we finish extending
> - * the API of all commands that create block jobs. */
> - if (job_id[0] == '\0') {
> - job_id = "default_job";
> - }
I think keeping an assertion here makes sense, though.
In any case:
Reviewed-by: Max Reitz <mreitz@redhat.com>
> }
>
> if (!id_wellformed(job_id)) {
> diff --git a/qemu-img.c b/qemu-img.c
> index a78f598..521724c 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -921,7 +921,7 @@ static int img_commit(int argc, char **argv)
> .bs = bs,
> };
>
> - commit_active_start(NULL, bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
> + commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
> common_block_job_cb, &cbi, &local_err);
> if (local_err) {
> goto done;
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 10/11] qemu-img: Set the ID of the block job in img_commit()
2016-07-02 14:21 ` Max Reitz
@ 2016-07-04 12:43 ` Alberto Garcia
0 siblings, 0 replies; 23+ messages in thread
From: Alberto Garcia @ 2016-07-04 12:43 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Jeff Cody, John Snow
On Sat 02 Jul 2016 04:21:33 PM CEST, Max Reitz wrote:
> On 01.07.2016 17:52, Alberto Garcia wrote:
>> img_commit() creates a block job without an ID. This is no longer
>> allowed now that we require it to be unique and well-formed. We were
>> solving this by having a fallback in block_job_create(), but now that
>> we extended the API of commit_active_start() we can finally set an
>> explicit ID and revert that change.
>>
>> Signed-off-by: Alberto Garcia <berto@igalia.com>
>> ---
>> blockjob.c | 6 ------
>> qemu-img.c | 2 +-
>> 2 files changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/blockjob.c b/blockjob.c
>> index 511c0db..3b9cec7 100644
>> --- a/blockjob.c
>> +++ b/blockjob.c
>> @@ -132,12 +132,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
>>
>> if (job_id == NULL) {
>> job_id = bdrv_get_device_name(bs);
>> - /* Assign a default ID if the BDS does not have a device
>> - * name. We'll get rid of this soon when we finish extending
>> - * the API of all commands that create block jobs. */
>> - if (job_id[0] == '\0') {
>> - job_id = "default_job";
>> - }
>
> I think keeping an assertion here makes sense, though.
I don't think so, this is an optional parameter. If the user fails to
provide an ID when making the QMP call and there's no device name, QEMU
should simply return an error.
Berto
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v3 11/11] blockjob: Update description of the 'device' field in the QMP API
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (9 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 10/11] qemu-img: Set the ID of the block job in img_commit() Alberto Garcia
@ 2016-07-01 15:52 ` Alberto Garcia
2016-07-02 14:37 ` Max Reitz
2016-07-04 15:49 ` [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Kevin Wolf
11 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2016-07-01 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Max Reitz, Kevin Wolf, Eric Blake, Jeff Cody,
John Snow, Alberto Garcia
The 'device' field in all BLOCK_JOB_* events and 'block-job-*' command
is no longer the device name, but the ID of the job. This patch
updates the documentation to clarify that.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
docs/qmp-events.txt | 12 ++++++++----
qapi/block-core.json | 35 +++++++++++++++++++++++++----------
2 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index fa7574d..a0ad3ac 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -92,7 +92,8 @@ Data:
- "type": Job type (json-string; "stream" for image streaming
"commit" for block commit)
-- "device": Device name (json-string)
+- "device": Job identifier. Originally the device name but other
+ values are allowed since QEMU 2.7 (json-string)
- "len": Maximum progress value (json-int)
- "offset": Current progress value (json-int)
On success this is equal to len.
@@ -116,7 +117,8 @@ Data:
- "type": Job type (json-string; "stream" for image streaming
"commit" for block commit)
-- "device": Device name (json-string)
+- "device": Job identifier. Originally the device name but other
+ values are allowed since QEMU 2.7 (json-string)
- "len": Maximum progress value (json-int)
- "offset": Current progress value (json-int)
On success this is equal to len.
@@ -143,7 +145,8 @@ Emitted when a block job encounters an error.
Data:
-- "device": device name (json-string)
+- "device": job identifier. Originally the device name but other
+ values are allowed since QEMU 2.7 (json-string)
- "operation": I/O operation (json-string, "read" or "write")
- "action": action that has been taken, it's one of the following (json-string):
"ignore": error has been ignored, the job may fail later
@@ -167,7 +170,8 @@ Data:
- "type": Job type (json-string; "stream" for image streaming
"commit" for block commit)
-- "device": Device name (json-string)
+- "device": Job identifier. Originally the device name but other
+ values are allowed since QEMU 2.7 (json-string)
- "len": Maximum progress value (json-int)
- "offset": Current progress value (json-int)
On success this is equal to len.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f754c29..5a91bc6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -713,7 +713,8 @@
#
# @type: the job type ('stream' for image streaming)
#
-# @device: the block device name
+# @device: the job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @len: the maximum progress value
#
@@ -1469,7 +1470,9 @@
#
# Throttling can be disabled by setting the speed to 0.
#
-# @device: the device name
+# @device: the job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# @speed: the maximum speed, in bytes per second, or 0 for unlimited.
# Defaults to 0.
@@ -1500,7 +1503,9 @@
# operation can be started at a later time to finish copying all data from the
# backing file.
#
-# @device: the device name
+# @device: the job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# @force: #optional whether to allow cancellation of a paused job (default
# false). Since 1.3.
@@ -1526,7 +1531,9 @@
# the operation is actually paused. Cancelling a paused job automatically
# resumes it.
#
-# @device: the device name
+# @device: the job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# Returns: Nothing on success
# If no background operation is active on this device, DeviceNotActive
@@ -1546,7 +1553,9 @@
#
# This command also clears the error status of the job.
#
-# @device: the device name
+# @device: the job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# Returns: Nothing on success
# If no background operation is active on this device, DeviceNotActive
@@ -1572,7 +1581,9 @@
#
# A cancelled or paused job cannot be completed.
#
-# @device: the device name
+# @device: the job identifier. This used to be a device name (hence
+# the name of the parameter), but since QEMU 2.7 it can have
+# other values.
#
# Returns: Nothing on success
# If no background operation is active on this device, DeviceNotActive
@@ -2417,7 +2428,8 @@
#
# @type: job type
#
-# @device: device name
+# @device: the job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @len: maximum progress value
#
@@ -2448,7 +2460,8 @@
#
# @type: job type
#
-# @device: device name
+# @device: the job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @len: maximum progress value
#
@@ -2471,7 +2484,8 @@
#
# Emitted when a block job encounters an error
#
-# @device: device name
+# @device: the job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @operation: I/O operation
#
@@ -2491,7 +2505,8 @@
#
# @type: job type
#
-# @device: device name
+# @device: the job identifier. Originally the device name but other
+# values are allowed since QEMU 2.7
#
# @len: maximum progress value
#
--
2.8.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH v3 11/11] blockjob: Update description of the 'device' field in the QMP API
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 11/11] blockjob: Update description of the 'device' field in the QMP API Alberto Garcia
@ 2016-07-02 14:37 ` Max Reitz
0 siblings, 0 replies; 23+ messages in thread
From: Max Reitz @ 2016-07-02 14:37 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Jeff Cody, John Snow
[-- Attachment #1: Type: text/plain, Size: 6479 bytes --]
On 01.07.2016 17:52, Alberto Garcia wrote:
> The 'device' field in all BLOCK_JOB_* events and 'block-job-*' command
> is no longer the device name, but the ID of the job. This patch
> updates the documentation to clarify that.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> docs/qmp-events.txt | 12 ++++++++----
> qapi/block-core.json | 35 +++++++++++++++++++++++++----------
> 2 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
> index fa7574d..a0ad3ac 100644
> --- a/docs/qmp-events.txt
> +++ b/docs/qmp-events.txt
> @@ -92,7 +92,8 @@ Data:
>
> - "type": Job type (json-string; "stream" for image streaming
> "commit" for block commit)
> -- "device": Device name (json-string)
> +- "device": Job identifier. Originally the device name but other
> + values are allowed since QEMU 2.7 (json-string)
> - "len": Maximum progress value (json-int)
> - "offset": Current progress value (json-int)
> On success this is equal to len.
> @@ -116,7 +117,8 @@ Data:
>
> - "type": Job type (json-string; "stream" for image streaming
> "commit" for block commit)
> -- "device": Device name (json-string)
> +- "device": Job identifier. Originally the device name but other
> + values are allowed since QEMU 2.7 (json-string)
> - "len": Maximum progress value (json-int)
> - "offset": Current progress value (json-int)
> On success this is equal to len.
> @@ -143,7 +145,8 @@ Emitted when a block job encounters an error.
>
> Data:
>
> -- "device": device name (json-string)
> +- "device": job identifier. Originally the device name but other
> + values are allowed since QEMU 2.7 (json-string)
> - "operation": I/O operation (json-string, "read" or "write")
> - "action": action that has been taken, it's one of the following (json-string):
> "ignore": error has been ignored, the job may fail later
> @@ -167,7 +170,8 @@ Data:
>
> - "type": Job type (json-string; "stream" for image streaming
> "commit" for block commit)
> -- "device": Device name (json-string)
> +- "device": Job identifier. Originally the device name but other
> + values are allowed since QEMU 2.7 (json-string)
> - "len": Maximum progress value (json-int)
> - "offset": Current progress value (json-int)
> On success this is equal to len.
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index f754c29..5a91bc6 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -713,7 +713,8 @@
> #
> # @type: the job type ('stream' for image streaming)
> #
> -# @device: the block device name
> +# @device: the job identifier. Originally the device name but other
> +# values are allowed since QEMU 2.7
> #
> # @len: the maximum progress value
> #
> @@ -1469,7 +1470,9 @@
> #
> # Throttling can be disabled by setting the speed to 0.
> #
> -# @device: the device name
> +# @device: the job identifier. This used to be a device name (hence
> +# the name of the parameter), but since QEMU 2.7 it can have
> +# other values.
Considering you put a full stop after "the job identifier", I'd
capitalize "the" (same in all following cases).
With that fixed:
Reviewed-by: Max Reitz <mreitz@redhat.com>
> #
> # @speed: the maximum speed, in bytes per second, or 0 for unlimited.
> # Defaults to 0.
> @@ -1500,7 +1503,9 @@
> # operation can be started at a later time to finish copying all data from the
> # backing file.
> #
> -# @device: the device name
> +# @device: the job identifier. This used to be a device name (hence
> +# the name of the parameter), but since QEMU 2.7 it can have
> +# other values.
> #
> # @force: #optional whether to allow cancellation of a paused job (default
> # false). Since 1.3.
> @@ -1526,7 +1531,9 @@
> # the operation is actually paused. Cancelling a paused job automatically
> # resumes it.
> #
> -# @device: the device name
> +# @device: the job identifier. This used to be a device name (hence
> +# the name of the parameter), but since QEMU 2.7 it can have
> +# other values.
> #
> # Returns: Nothing on success
> # If no background operation is active on this device, DeviceNotActive
> @@ -1546,7 +1553,9 @@
> #
> # This command also clears the error status of the job.
> #
> -# @device: the device name
> +# @device: the job identifier. This used to be a device name (hence
> +# the name of the parameter), but since QEMU 2.7 it can have
> +# other values.
> #
> # Returns: Nothing on success
> # If no background operation is active on this device, DeviceNotActive
> @@ -1572,7 +1581,9 @@
> #
> # A cancelled or paused job cannot be completed.
> #
> -# @device: the device name
> +# @device: the job identifier. This used to be a device name (hence
> +# the name of the parameter), but since QEMU 2.7 it can have
> +# other values.
> #
> # Returns: Nothing on success
> # If no background operation is active on this device, DeviceNotActive
> @@ -2417,7 +2428,8 @@
> #
> # @type: job type
> #
> -# @device: device name
> +# @device: the job identifier. Originally the device name but other
> +# values are allowed since QEMU 2.7
> #
> # @len: maximum progress value
> #
> @@ -2448,7 +2460,8 @@
> #
> # @type: job type
> #
> -# @device: device name
> +# @device: the job identifier. Originally the device name but other
> +# values are allowed since QEMU 2.7
> #
> # @len: maximum progress value
> #
> @@ -2471,7 +2484,8 @@
> #
> # Emitted when a block job encounters an error
> #
> -# @device: device name
> +# @device: the job identifier. Originally the device name but other
> +# values are allowed since QEMU 2.7
> #
> # @operation: I/O operation
> #
> @@ -2491,7 +2505,8 @@
> #
> # @type: job type
> #
> -# @device: device name
> +# @device: the job identifier. Originally the device name but other
> +# values are allowed since QEMU 2.7
> #
> # @len: maximum progress value
> #
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID
2016-07-01 15:51 [Qemu-devel] [PATCH v3 00/11] Allow creating block jobs with a user-defined ID Alberto Garcia
` (10 preceding siblings ...)
2016-07-01 15:52 ` [Qemu-devel] [PATCH v3 11/11] blockjob: Update description of the 'device' field in the QMP API Alberto Garcia
@ 2016-07-04 15:49 ` Kevin Wolf
11 siblings, 0 replies; 23+ messages in thread
From: Kevin Wolf @ 2016-07-04 15:49 UTC (permalink / raw)
To: Alberto Garcia
Cc: qemu-devel, qemu-block, Max Reitz, Eric Blake, Jeff Cody,
John Snow
Am 01.07.2016 um 17:51 hat Alberto Garcia geschrieben:
> Hi all,
>
> block jobs are currently identified by the name of the block backend
> of the BDS where the job was started.
>
> The problem with this is that you cannot have block jobs on nodes
> where there is no such name.
>
> This series turns the 'id' field of the BlockJob structure into an
> actual ID, it guarantees that is unique and always present and allows
> the user to set it.
>
> Berto
With Max' comments addressed:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 23+ messages in thread