Re: [Qemu-block] [PATCH v9 05/11] block: allow block jobs in any arbitrary node

[Alberto Garcia]

On Fri 29 Apr 2016 05:00:57 PM CEST, Kevin Wolf wrote:

>> - Block jobs can now be identified by the node name of their
>> BlockDriverState in addition to the device name. Since both device
>> and node names live in the same namespace there's no ambiguity.
>
> Careful, we know this is a part of our API that we have already messed
> up and we don't want to make things worse while adding new things
> before we've cleaned it up.
>
> Let's keep in mind where we are planning to go with block jobs: They
> should become background jobs, not tied to any block device. The close
> connection to a single BDS already doesn't make a lot of sense today
> because most block jobs involve at least two BDSes.
>
> In the final state, we will have a job ID that uniquely identifies the
> job, and each command that starts a job will take an ID from the
> client.  For compatibility, we'll use the block device name as the job
> ID when using old commands that don't get an explicit ID yet.
>
> In the existing qemu version, you can't start two block jobs on the
> same device, and in future versions, you're supposed to specify an ID
> each time. This is why the default can always be supposed to work
> without conflicts. If in newer versions, the client mixes both ways
> (which it shouldn't do), starting a new block job may fail because the
> device name is already in use as an ID for another job.
>
> Now we can probably make the same argument for node names, so we can
> extend the interface and still keep it compatible.
>
> Where we need to be careful is that with device names and node names,
> we have potentially two names describing the same BDS and therefore
> the same job. This must not happen, because we won't be able to
> represent that in the generic background job API. Any job has just a
> single ID there.

Ok, what can be done in this case is to keep the name that the client
used when the job was created.

block-stream virti0   <-- job id is 'virtio0'
block-stream node5    <-- job id is 'node5'

In this case it wouldn't matter if 'node5' is the one attached to
'virtio0'.

>> @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, 
>> AioContext **aio_context,
>>  
>>      *aio_context = NULL;
>>  
>> -    blk = blk_by_name(device);
>> -    if (!blk) {
>> +    bs = bdrv_lookup_bs(device_or_node, device_or_node, errp);
>
> Specifically, this one is bad. It allows two different ways to specify
> the same job.

I think we can simply iterate over all block jobs (now that we have a
function to do that) and return the one with the matching ID.

Berto