block: drop bs->job

Drop remaining users of bs->job:
1. assertions actually duplicated by assert(!bs->refcnt)
2. trace-point seems not enough reason to change stream_start to return
   BlockJob pointer
3. Restricting creation of two jobs based on same bs is bad idea, as
   3.1 Some jobs creates filters to be their main node, so, this check
   don't actually prevent creating second job on same real node (which
   will create another filter node) (but I hope it is restricted by
   other mechanisms)
   3.2 Even without bs->job we have two systems of permissions:
   op-blockers and BLK_PERM
   3.3 We may want to run several jobs on one node one day

And finally, drop bs->job pointer itself. Hurrah!

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Vladimir Sementsov-Ogievskiy 2019-06-06 18:41:32 +03:00 committed by Kevin Wolf
parent 8164102ffe
commit b23c580c94
6 changed files with 5 additions and 17 deletions

View File

@ -3905,7 +3905,6 @@ static void bdrv_close(BlockDriverState *bs)
BdrvAioNotifier *ban, *ban_next; BdrvAioNotifier *ban, *ban_next;
BdrvChild *child, *next; BdrvChild *child, *next;
assert(!bs->job);
assert(!bs->refcnt); assert(!bs->refcnt);
bdrv_drained_begin(bs); /* complete I/O */ bdrv_drained_begin(bs); /* complete I/O */
@ -4146,7 +4145,6 @@ out:
static void bdrv_delete(BlockDriverState *bs) static void bdrv_delete(BlockDriverState *bs)
{ {
assert(!bs->job);
assert(bdrv_op_blocker_is_empty(bs)); assert(bdrv_op_blocker_is_empty(bs));
assert(!bs->refcnt); assert(!bs->refcnt);

View File

@ -53,7 +53,7 @@ qmp_block_job_resume(void *job) "job %p"
qmp_block_job_complete(void *job) "job %p" qmp_block_job_complete(void *job) "job %p"
qmp_block_job_finalize(void *job) "job %p" qmp_block_job_finalize(void *job) "job %p"
qmp_block_job_dismiss(void *job) "job %p" qmp_block_job_dismiss(void *job) "job %p"
qmp_block_stream(void *bs, void *job) "bs %p job %p" qmp_block_stream(void *bs) "bs %p"
# file-posix.c # file-posix.c
# file-win32.c # file-win32.c

View File

@ -3260,7 +3260,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
goto out; goto out;
} }
trace_qmp_block_stream(bs, bs->job); trace_qmp_block_stream(bs);
out: out:
aio_context_release(aio_context); aio_context_release(aio_context);

View File

@ -83,9 +83,7 @@ BlockJob *block_job_get(const char *id)
void block_job_free(Job *job) void block_job_free(Job *job)
{ {
BlockJob *bjob = container_of(job, BlockJob, job); BlockJob *bjob = container_of(job, BlockJob, job);
BlockDriverState *bs = blk_bs(bjob->blk);
bs->job = NULL;
block_job_remove_all_bdrv(bjob); block_job_remove_all_bdrv(bjob);
blk_unref(bjob->blk); blk_unref(bjob->blk);
error_free(bjob->blocker); error_free(bjob->blocker);
@ -402,11 +400,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
BlockJob *job; BlockJob *job;
int ret; int ret;
if (bs->job) {
error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
return NULL;
}
if (job_id == NULL && !(flags & JOB_INTERNAL)) { if (job_id == NULL && !(flags & JOB_INTERNAL)) {
job_id = bdrv_get_device_name(bs); job_id = bdrv_get_device_name(bs);
} }
@ -449,7 +442,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
error_setg(&job->blocker, "block device is in use by block job: %s", error_setg(&job->blocker, "block device is in use by block job: %s",
job_type_str(&job->job)); job_type_str(&job->job));
block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort); block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort);
bs->job = job;
bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);

View File

@ -812,9 +812,6 @@ struct BlockDriverState {
/* operation blockers */ /* operation blockers */
QLIST_HEAD(, BdrvOpBlocker) op_blockers[BLOCK_OP_TYPE_MAX]; QLIST_HEAD(, BdrvOpBlocker) op_blockers[BLOCK_OP_TYPE_MAX];
/* long-running background operation */
BlockJob *job;
/* The node that this node inherited default options from (and a reopen on /* The node that this node inherited default options from (and a reopen on
* which can affect this node by changing these defaults). This is always a * which can affect this node by changing these defaults). This is always a
* parent node of this node. */ * parent node of this node. */

View File

@ -122,8 +122,9 @@ static void test_job_ids(void)
/* This one is valid */ /* This one is valid */
job[0] = do_test_id(blk[0], "id0", true); job[0] = do_test_id(blk[0], "id0", true);
/* We cannot have two jobs in the same BDS */ /* We can have two jobs in the same BDS */
do_test_id(blk[0], "id1", false); job[1] = do_test_id(blk[0], "id1", true);
job_early_fail(&job[1]->job);
/* Duplicate job IDs are not allowed */ /* Duplicate job IDs are not allowed */
job[1] = do_test_id(blk[1], "id0", false); job[1] = do_test_id(blk[1], "id0", false);