[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 04/11] block: Use block_job_get() in find_blo
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v3 04/11] block: Use block_job_get() in find_block_job() |
Date: |
Sat, 2 Jul 2016 16:02:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
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 <address@hidden>
> ---
> 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 <address@hidden>
> + 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)
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v3 10/11] qemu-img: Set the ID of the block job in img_commit(), (continued)
- [Qemu-block] [PATCH v3 10/11] qemu-img: Set the ID of the block job in img_commit(), Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 02/11] blockjob: Update description of the 'id' field, Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 01/11] stream: Fix prototype of stream_start(), Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 05/11] blockjob: Add 'job_id' parameter to block_job_create(), Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 09/11] commit: Add 'job-id' parameter to 'block-commit', Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 04/11] block: Use block_job_get() in find_block_job(), Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 11/11] blockjob: Update description of the 'device' field in the QMP API, Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 03/11] blockjob: Add block_job_get(), Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 08/11] stream: Add 'job-id' parameter to 'block-stream', Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 07/11] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup', Alberto Garcia, 2016/07/01
- [Qemu-block] [PATCH v3 06/11] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror', Alberto Garcia, 2016/07/01