On 04.04.2016 15:43, Alberto Garcia wrote: > 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. > > - The "device" parameter used by all commands that operate on block > jobs can also be a node name now. > > - 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); > + if (!bs) {
If we get here, *errp is already set... [1] > goto notfound; > } > > - *aio_context = blk_get_aio_context(blk); > + *aio_context = bdrv_get_aio_context(bs); > aio_context_acquire(*aio_context); > > - if (!blk_is_available(blk)) { > + blk = blk_by_name(device_or_node); > + if (blk && !blk_is_available(blk)) { I'd just drop this. The reason blk_is_available() was added here because it became possible for BBs not to have a BDS. Now that you get the BDS directly through bdrv_lookup_bs(), it's no longer necessary. > goto notfound; > } > - bs = blk_bs(blk); > > if (!bs->job) { > goto notfound; > @@ -3715,7 +3717,7 @@ static BlockJob *find_block_job(const char *device, > AioContext **aio_context, > > notfound: > error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE, > - "No active block job on device '%s'", device); > + "No active block job on node '%s'", device_or_node); [1] ...and then we'll get a failed assertion here (through error_setv()). > if (*aio_context) { > aio_context_release(*aio_context); > *aio_context = NULL; > diff --git a/blockjob.c b/blockjob.c > index 3557048..2ab4794 100644 > --- a/blockjob.c > +++ b/blockjob.c > @@ -67,7 +67,8 @@ void *block_job_create(const BlockJobDriver *driver, > BlockDriverState *bs, > BlockJob *job; > > if (bs->job) { > - error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); > + error_setg(errp, "Node '%s' is in use", > + bdrv_get_device_or_node_name(bs)); > return NULL; > } > bdrv_ref(bs); > @@ -78,7 +79,7 @@ void *block_job_create(const BlockJobDriver *driver, > BlockDriverState *bs, > bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); > > job->driver = driver; > - job->id = g_strdup(bdrv_get_device_name(bs)); > + job->id = g_strdup(bdrv_get_device_or_node_name(bs)); Hm, since this is only used for events, it's not too bad that a block job will have a different name if it was created on a root BDS by specifying its node name. But it still is kind of strange. But as I said, it should be fine as long as one can still use the node name for controlling it (which is the case). Max
signature.asc
Description: OpenPGP digital signature