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