Re: [Qemu-block] [PATCH 03/15] blockjob: Add block_job_get()

[Max Reitz]

On 20.06.2016 20:48, Eric Blake wrote:
> 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 <address@hidden>
>>> ---
>>>  blockjob.c               | 13 +++++++++++++
>>>  include/block/blockjob.h | 10 ++++++++++
>>>  2 files changed, 23 insertions(+)
>>
>> Reviewed-by: Max Reitz <address@hidden>
>>
>> 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.

Yes, also, after having reviewed (most of) the rest of the series, I've
seen that Berto has added the ID as a separate parameter of all block
job commands. Therefore, there really is no need to restrict the job ID
namespace.

Max

signature.asc
Description: OpenPGP digital signature