From: Eric Blake <eblake@redhat.com>
To: Max Reitz <mreitz@redhat.com>, Alberto Garcia <berto@igalia.com>,
qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
Jeff Cody <jcody@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 03/15] blockjob: Add block_job_get()
Date: Mon, 20 Jun 2016 12:48:26 -0600 [thread overview]
Message-ID: <57683A7A.7020103@redhat.com> (raw)
In-Reply-To: <6977c422-0899-baee-0282-41e32fa7782c@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1900 bytes --]
On 06/20/2016 10:24 AM, Max Reitz wrote:
> On 09.06.2016 10:20, Alberto Garcia wrote:
>> 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>
>> ---
>> blockjob.c | 13 +++++++++++++
>> include/block/blockjob.h | 10 ++++++++++
>> 2 files changed, 23 insertions(+)
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
> Just to throw out a suggestion, I'm not sure how useful it will be after
> the rest of the series:
>
> Would it make sense to prevent name clashes between block job IDs and
> device IDs (i.e. BlockBackend names)? If we did that, this function
> could be used to identify a block job both based on its name and its device.
Adding the restriction now, and then relaxing it later, is easier than
keeping it relaxed and wishing we could tighten it later.
>
> Just a suggestion, though, I don't think it will be necessary. Legacy
> applications will always create jobs with auto-generated IDs that cannot
> clash with BB names anyway. If we require applications that do name
> their block jobs manually to always use this ID when specifying a block
> job, everything will be fine even if block job and BB names do clash.
But I think you're right here - legacy users will be just fine (at most
one job per device, never using the id but also never clashing an id
with device names), and new users should just always assign a job id and
use that rather than relying on device names. So I don't think we need
the extra restriction of merging device and job id namespaces into one.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2016-06-20 18:48 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-09 8:20 [Qemu-devel] [PATCH 00/15] Add an 'id' field to block jobs Alberto Garcia
2016-06-09 8:20 ` [Qemu-devel] [PATCH 01/15] stream: Fix prototype of stream_start() Alberto Garcia
2016-06-20 16:07 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 02/15] blockjob: Decouple the ID from the device name in the BlockJob struct Alberto Garcia
2016-06-20 16:17 ` Max Reitz
2016-06-21 11:42 ` Alberto Garcia
2016-06-09 8:20 ` [Qemu-devel] [PATCH 03/15] blockjob: Add block_job_get() Alberto Garcia
2016-06-20 16:24 ` Max Reitz
2016-06-20 18:48 ` Eric Blake [this message]
2016-06-20 19:06 ` Max Reitz
2016-06-21 12:09 ` Alberto Garcia
2016-06-09 8:20 ` [Qemu-devel] [PATCH 04/15] block: Simplify find_block_job() and make it accept a job ID Alberto Garcia
2016-06-20 16:40 ` Max Reitz
2016-06-20 18:29 ` Max Reitz
2016-06-21 12:59 ` Alberto Garcia
2016-06-20 18:53 ` Eric Blake
2016-06-21 12:27 ` Alberto Garcia
2016-06-21 21:36 ` Eric Blake
2016-06-22 7:32 ` Alberto Garcia
2016-06-09 8:20 ` [Qemu-devel] [PATCH 05/15] blockjob: Add 'job_id' parameter to block_job_create() Alberto Garcia
2016-06-20 17:11 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 06/15] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror' Alberto Garcia
2016-06-20 17:33 ` Max Reitz
2016-06-20 17:34 ` Max Reitz
2016-06-20 18:56 ` Eric Blake
2016-06-09 8:20 ` [Qemu-devel] [PATCH 07/15] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup' Alberto Garcia
2016-06-20 18:14 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 08/15] stream: Add 'job-id' parameter to 'block-stream' Alberto Garcia
2016-06-20 18:16 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 09/15] stream: Add 'job-id' parameter to 'block-commit' Alberto Garcia
2016-06-20 18:22 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 10/15] blockjob: Add 'id' parameter to 'block-job-set-speed' Alberto Garcia
2016-06-20 18:31 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 11/15] blockjob: Add 'id' parameter to 'block-job-cancel' Alberto Garcia
2016-06-20 18:32 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 12/15] blockjob: Add 'id' parameter to 'block-job-pause' Alberto Garcia
2016-06-20 18:34 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 13/15] blockjob: Add 'id' parameter to 'block-job-resume' Alberto Garcia
2016-06-20 18:36 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 14/15] blockjob: Add 'id' parameter to 'block-job-complete' Alberto Garcia
2016-06-20 18:37 ` Max Reitz
2016-06-09 8:20 ` [Qemu-devel] [PATCH 15/15] blockjob: Add 'id' field to 'BlockJobInfo' and all BLOCK_JOB_* events Alberto Garcia
2016-06-20 19:14 ` Max Reitz
2016-06-21 14:23 ` Alberto Garcia
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57683A7A.7020103@redhat.com \
--to=eblake@redhat.com \
--cc=berto@igalia.com \
--cc=jcody@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).