[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/6] block: require job-id when device is a n
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/6] block: require job-id when device is a node name |
Date: |
Mon, 21 Aug 2017 17:05:30 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Wed 09 Aug 2017 04:02:53 PM CEST, Manos Pitsidianakis wrote:
> @@ -622,20 +622,14 @@ void *block_job_create(const char *job_id, const
> BlockJobDriver *driver,
> return NULL;
> }
>
> - if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) {
> - job_id = bdrv_get_device_name(bs);
> - if (!*job_id) {
> - error_setg(errp, "An explicit job ID is required for this node");
> - return NULL;
> - }
> - }
> -
> - if (job_id) {
> - if (flags & BLOCK_JOB_INTERNAL) {
> + if (flags & BLOCK_JOB_INTERNAL) {
> + if (job_id) {
> error_setg(errp, "Cannot specify job ID for internal block job");
> return NULL;
> }
When BLOCK_JOB_INTERNAL is set, then job_id must be NULL...
> -
> + } else {
> + /* Require job-id. */
> + assert(job_id);
> if (!id_wellformed(job_id)) {
> error_setg(errp, "Invalid job ID '%s'", job_id);
> return NULL;
> diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
> index f13ad05c0d..ff906808a6 100644
> --- a/include/block/blockjob_int.h
> +++ b/include/block/blockjob_int.h
> @@ -112,8 +112,7 @@ struct BlockJobDriver {
>
> /**
> * block_job_create:
> - * @job_id: The id of the newly-created job, or %NULL to have one
> - * generated automatically.
> + * @job_id: The id of the newly-created job, must be non %NULL.
...but here you say that it must not be NULL.
And since job_id can be NULL in some cases I think I'd replace the
assert(job_id) that you added to block_job_create() with a normal
pointer check + error_setg().
> @@ -93,9 +93,6 @@ static void test_job_ids(void)
> blk[1] = create_blk("drive1");
> blk[2] = create_blk("drive2");
>
> - /* No job ID provided and the block backend has no name */
> - job[0] = do_test_id(blk[0], NULL, false);
> -
If you decide to handle NULL ids by returning and error instead of
asserting, we should keep a couple of tests for that scenario.
Berto
Re: [Qemu-devel] [PATCH v2 0/6] block: remove legacy I/O throttling, Eric Blake, 2017/08/09