Am 02.07.2016 um 16:02 hat Max Reitz geschrieben: > On 01.07.2016 17:52, Alberto Garcia wrote: > > find_block_job() looks for a block backend with a specified name, > > checks whether it has a block job and acquires its AioContext. > > > > We want to identify jobs by their ID and not by the block backend > > they're attached to, so this patch ignores the backends altogether and > > gets the job directly. Apart from making the code simpler, this will > > allow us to find block jobs once they start having user-specified IDs. > > > > To ensure backward compatibility we keep ERROR_CLASS_DEVICE_NOT_ACTIVE > > as the error class if the job doesn't exist. In subsequent patches > > we'll also need to keep the device name as the default job ID if the > > user doesn't specify a different one. > > > > Signed-off-by: Alberto Garcia <be...@igalia.com> > > --- > > blockdev.c | 43 ++++++++++++++++--------------------------- > > 1 file changed, 16 insertions(+), 27 deletions(-) > > > > diff --git a/blockdev.c b/blockdev.c > > index 3a104a0..8cedb60 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -3704,42 +3704,31 @@ 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 a block job using its ID and acquire its AioContext */ > > +static BlockJob *find_block_job(const char *id, AioContext **aio_context, > > Error **errp) > > { > > - BlockBackend *blk; > > - BlockDriverState *bs; > > + BlockJob *job; > > > > *aio_context = NULL; > > > > - blk = blk_by_name(device); > > - if (!blk) { > > - goto notfound; > > + if (!id) { > > + error_setg(errp, "Unspecified job ID when looking for a block > > job"); > > + return NULL; > > } > > Why no plain assertion? Do you expect callers who may pass a NULL ID? > > > > > - *aio_context = blk_get_aio_context(blk); > > + job = block_job_get(id); > > + > > + if (!job) { > > + error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE, > > + "Block job '%s' not found", id); > > This error class seems a bit weird now... I know I advocated for it in > v2, but that was because you could actually specifically pass a device > name to find block jobs on that device, but now you're just looking for > block jobs with a certain ID (that happens to default to the device name).
libvirt uses the error class, so I don't think we can drop it. Kevin
pgpiyHCwO08id.pgp
Description: PGP signature