Am 04.04.2016 um 15:43 hat Alberto Garcia geschrieben: > Currently, block jobs can only be owned by root nodes. This patch > allows block jobs to be in any arbitrary node, by making the following > changes: > > - 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. > - The "device" parameter used by all commands that operate on block > jobs can also be a node name now. So I think the least we need to do is change this one to resolve the block job by its ID (job->id) rather than device or node names. > - The node name is used as a fallback to fill in the BlockJobInfo > structure and all BLOCK_JOB_* events if there is no device name for > that job. > > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > blockdev.c | 18 ++++++++++-------- > blockjob.c | 5 +++-- > docs/qmp-events.txt | 8 ++++---- > qapi/block-core.json | 20 ++++++++++---------- > 4 files changed, 27 insertions(+), 24 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index edbcc19..d1f5dfb 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3685,8 +3685,10 @@ void qmp_blockdev_mirror(const char *device, const > char *target, > aio_context_release(aio_context); > } > > -/* Get the block job for a given device name and acquire its AioContext */ > -static BlockJob *find_block_job(const char *device, AioContext **aio_context, > +/* Get the block job for a given device or node name > + * and acquire its AioContext */ > +static BlockJob *find_block_job(const char *device_or_node, > + AioContext **aio_context, > Error **errp) > { > BlockBackend *blk; > @@ -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. The other thing about this patch is that I'm not sure how badly it conflicts with my series to convert block jobs to BlockBackend. It seems that you switch everything from blk to bs here, and I'll have to switch back to blk (once job->blk exists). Kevin