Remove use of block_job_pause/resume from outside blockjob.c, thus
making them static. Again, one reason to do this is that
block_job_pause/resume will have different locking rules compared
to everything else that block.c and block/io.c use.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/io.c | 18 +-------
blockjob.c | 114 ++++++++++++++++++++++++++++-------------------
include/block/blockjob.h | 14 +++---
3 files changed, 77 insertions(+), 69 deletions(-)
diff --git a/block/io.c b/block/io.c
index 2709a70..1cc9318 100644
--- a/block/io.c
+++ b/block/io.c
@@ -284,16 +284,9 @@ void bdrv_drain_all_begin(void)
bool waited = true;
BlockDriverState *bs;
BdrvNextIterator it;
- BlockJob *job = NULL;
GSList *aio_ctxs = NULL, *ctx;
- while ((job = block_job_next(job))) {
- AioContext *aio_context = blk_get_aio_context(job->blk);
-
- aio_context_acquire(aio_context);
- block_job_pause(job);
- aio_context_release(aio_context);
- }
+ block_job_pause_all();
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *aio_context = bdrv_get_aio_context(bs);
@@ -337,7 +330,6 @@ void bdrv_drain_all_end(void)
{
BlockDriverState *bs;
BdrvNextIterator it;
- BlockJob *job = NULL;
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
AioContext *aio_context = bdrv_get_aio_context(bs);
@@ -348,13 +340,7 @@ void bdrv_drain_all_end(void)
aio_context_release(aio_context);
}
- while ((job = block_job_next(job))) {
- AioContext *aio_context = blk_get_aio_context(job->blk);
-
- aio_context_acquire(aio_context);
- block_job_resume(job);
- aio_context_release(aio_context);
- }
+ block_job_resume_all();
}
void bdrv_drain_all(void)
diff --git a/blockjob.c b/blockjob.c
index 1c63d15..caca718 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -55,36 +55,6 @@ struct BlockJobTxn {
static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs);
-static char *child_job_get_parent_desc(BdrvChild *c)
-{
- BlockJob *job = c->opaque;
- return g_strdup_printf("%s job '%s'",
- BlockJobType_lookup[job->driver->job_type],
- job->id);
-}
-
-static const BdrvChildRole child_job = {
- .get_parent_desc = child_job_get_parent_desc,
- .stay_at_node = true,
-};
-
-static void block_job_drained_begin(void *opaque)
-{
- BlockJob *job = opaque;
- block_job_pause(job);
-}
-
-static void block_job_drained_end(void *opaque)
-{
- BlockJob *job = opaque;
- block_job_resume(job);
-}
-
-static const BlockDevOps block_job_dev_ops = {
- .drained_begin = block_job_drained_begin,
- .drained_end = block_job_drained_end,
-};
-
BlockJob *block_job_next(BlockJob *job)
{
if (!job) {
@@ -106,6 +76,21 @@ BlockJob *block_job_get(const char *id)
return NULL;
}
+static void block_job_pause(BlockJob *job)
+{
+ job->pause_count++;
+}
+
+static void block_job_resume(BlockJob *job)
+{
+ assert(job->pause_count > 0);
+ job->pause_count--;
+ if (job->pause_count) {
+ return;
+ }
+ block_job_enter(job);
+}
+
static void block_job_ref(BlockJob *job)
{
++job->refcnt;
@@ -171,6 +156,36 @@ static void block_job_detach_aio_context(void *opaque)
block_job_unref(job);
}
+static char *child_job_get_parent_desc(BdrvChild *c)
+{
+ BlockJob *job = c->opaque;
+ return g_strdup_printf("%s job '%s'",
+ BlockJobType_lookup[job->driver->job_type],
+ job->id);
+}
+
+static const BdrvChildRole child_job = {
+ .get_parent_desc = child_job_get_parent_desc,
+ .stay_at_node = true,
+};
+
+static void block_job_drained_begin(void *opaque)
+{
+ BlockJob *job = opaque;
+ block_job_pause(job);
+}
+
+static void block_job_drained_end(void *opaque)
+{
+ BlockJob *job = opaque;
+ block_job_resume(job);
+}
+
+static const BlockDevOps block_job_dev_ops = {
+ .drained_begin = block_job_drained_begin,
+ .drained_end = block_job_drained_end,
+};
+
void block_job_remove_all_bdrv(BlockJob *job)
{
GSList *l;
@@ -471,11 +486,6 @@ void block_job_complete(BlockJob *job, Error **errp)
job->driver->complete(job, errp);
}
-void block_job_pause(BlockJob *job)
-{
- job->pause_count++;
-}
-
void block_job_user_pause(BlockJob *job)
{
job->user_paused = true;
@@ -520,16 +530,6 @@ void coroutine_fn block_job_pause_point(BlockJob *job)
}
}
-void block_job_resume(BlockJob *job)
-{
- assert(job->pause_count > 0);
- job->pause_count--;
- if (job->pause_count) {
- return;
- }
- block_job_enter(job);
-}
-
void block_job_user_resume(BlockJob *job)
{
if (job && job->user_paused && job->pause_count > 0) {
@@ -723,6 +723,30 @@ static void block_job_event_completed(BlockJob *job, const char *msg)
&error_abort);
}
+void block_job_pause_all(void)
+{
+ BlockJob *job = NULL;
+ while ((job = block_job_next(job))) {
+ AioContext *aio_context = blk_get_aio_context(job->blk);
+
+ aio_context_acquire(aio_context);
+ block_job_pause(job);
+ aio_context_release(aio_context);
+ }
+}
+
+void block_job_resume_all(void)
+{
+ BlockJob *job = NULL;
+ while ((job = block_job_next(job))) {
+ AioContext *aio_context = blk_get_aio_context(job->blk);
+
+ aio_context_acquire(aio_context);
+ block_job_resume(job);
+ aio_context_release(aio_context);
+ }
+}
+
void block_job_event_ready(BlockJob *job)
{
job->ready = true;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 9e906f7..aac87cd 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -235,12 +235,11 @@ void block_job_complete(BlockJob *job, Error **errp);
BlockJobInfo *block_job_query(BlockJob *job, Error **errp);
/**
- * block_job_pause:
- * @job: The job to be paused.
+ * block_job_pause_all:
*
- * Asynchronously pause the specified job.
+ * Asynchronously pause all jobs.
*/
-void block_job_pause(BlockJob *job);
+void block_job_pause_all(void);
/**
* block_job_user_pause:
@@ -260,12 +259,11 @@ void block_job_user_pause(BlockJob *job);
bool block_job_user_paused(BlockJob *job);
/**
- * block_job_resume:
- * @job: The job to be resumed.
+ * block_job_resume_all:
*
- * Resume the specified job. Must be paired with a preceding block_job_pause.
+ * Resume all block jobs. Must be paired with a preceding block_job_pause_all.
*/
-void block_job_resume(BlockJob *job);
+void block_job_resume_all(void);
/**
* block_job_user_resume:
--
2.9.3
On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Remove use of block_job_pause/resume from outside blockjob.c, thus
> making them static. Again, one reason to do this is that
> block_job_pause/resume will have different locking rules compared
> to everything else that block.c and block/io.c use.
>
Ominous
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> block/io.c | 18 +-------
> blockjob.c | 114 ++++++++++++++++++++++++++++-------------------
> include/block/blockjob.h | 14 +++---
> 3 files changed, 77 insertions(+), 69 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 2709a70..1cc9318 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -284,16 +284,9 @@ void bdrv_drain_all_begin(void)
> bool waited = true;
> BlockDriverState *bs;
> BdrvNextIterator it;
> - BlockJob *job = NULL;
> GSList *aio_ctxs = NULL, *ctx;
>
> - while ((job = block_job_next(job))) {
> - AioContext *aio_context = blk_get_aio_context(job->blk);
> -
> - aio_context_acquire(aio_context);
> - block_job_pause(job);
> - aio_context_release(aio_context);
> - }
> + block_job_pause_all();
>
Cool. Well, maybe cool. I think it would be cool if we didn't have to
explicitly pause jobs here at all. This is better at least, though.
> for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
> AioContext *aio_context = bdrv_get_aio_context(bs);
> @@ -337,7 +330,6 @@ void bdrv_drain_all_end(void)
> {
> BlockDriverState *bs;
> BdrvNextIterator it;
> - BlockJob *job = NULL;
>
> for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
> AioContext *aio_context = bdrv_get_aio_context(bs);
> @@ -348,13 +340,7 @@ void bdrv_drain_all_end(void)
> aio_context_release(aio_context);
> }
>
> - while ((job = block_job_next(job))) {
> - AioContext *aio_context = blk_get_aio_context(job->blk);
> -
> - aio_context_acquire(aio_context);
> - block_job_resume(job);
> - aio_context_release(aio_context);
> - }
> + block_job_resume_all();
> }
>
> void bdrv_drain_all(void)
> diff --git a/blockjob.c b/blockjob.c
> index 1c63d15..caca718 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -55,36 +55,6 @@ struct BlockJobTxn {
>
> static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs);
>
> -static char *child_job_get_parent_desc(BdrvChild *c)
> -{
> - BlockJob *job = c->opaque;
> - return g_strdup_printf("%s job '%s'",
> - BlockJobType_lookup[job->driver->job_type],
> - job->id);
> -}
> -
> -static const BdrvChildRole child_job = {
> - .get_parent_desc = child_job_get_parent_desc,
> - .stay_at_node = true,
> -};
> -
> -static void block_job_drained_begin(void *opaque)
> -{
> - BlockJob *job = opaque;
> - block_job_pause(job);
> -}
> -
> -static void block_job_drained_end(void *opaque)
> -{
> - BlockJob *job = opaque;
> - block_job_resume(job);
> -}
> -
> -static const BlockDevOps block_job_dev_ops = {
> - .drained_begin = block_job_drained_begin,
> - .drained_end = block_job_drained_end,
> -};
> -
> BlockJob *block_job_next(BlockJob *job)
> {
> if (!job) {
> @@ -106,6 +76,21 @@ BlockJob *block_job_get(const char *id)
> return NULL;
> }
>
> +static void block_job_pause(BlockJob *job)
> +{
> + job->pause_count++;
> +}
> +
> +static void block_job_resume(BlockJob *job)
> +{
> + assert(job->pause_count > 0);
> + job->pause_count--;
> + if (job->pause_count) {
> + return;
> + }
> + block_job_enter(job);
> +}
> +
> static void block_job_ref(BlockJob *job)
> {
> ++job->refcnt;
> @@ -171,6 +156,36 @@ static void block_job_detach_aio_context(void *opaque)
> block_job_unref(job);
> }
>
> +static char *child_job_get_parent_desc(BdrvChild *c)
> +{
> + BlockJob *job = c->opaque;
> + return g_strdup_printf("%s job '%s'",
> + BlockJobType_lookup[job->driver->job_type],
> + job->id);
> +}
> +
> +static const BdrvChildRole child_job = {
> + .get_parent_desc = child_job_get_parent_desc,
> + .stay_at_node = true,
> +};
> +
> +static void block_job_drained_begin(void *opaque)
> +{
> + BlockJob *job = opaque;
> + block_job_pause(job);
> +}
> +
> +static void block_job_drained_end(void *opaque)
> +{
> + BlockJob *job = opaque;
> + block_job_resume(job);
> +}
> +
> +static const BlockDevOps block_job_dev_ops = {
> + .drained_begin = block_job_drained_begin,
> + .drained_end = block_job_drained_end,
> +};
> +
Movement should probably be its own patch, but I'm not a cop or nothin'
> void block_job_remove_all_bdrv(BlockJob *job)
> {
> GSList *l;
> @@ -471,11 +486,6 @@ void block_job_complete(BlockJob *job, Error **errp)
> job->driver->complete(job, errp);
> }
>
> -void block_job_pause(BlockJob *job)
> -{
> - job->pause_count++;
> -}
> -
> void block_job_user_pause(BlockJob *job)
> {
> job->user_paused = true;
> @@ -520,16 +530,6 @@ void coroutine_fn block_job_pause_point(BlockJob *job)
> }
> }
>
> -void block_job_resume(BlockJob *job)
> -{
> - assert(job->pause_count > 0);
> - job->pause_count--;
> - if (job->pause_count) {
> - return;
> - }
> - block_job_enter(job);
> -}
> -
> void block_job_user_resume(BlockJob *job)
> {
> if (job && job->user_paused && job->pause_count > 0) {
> @@ -723,6 +723,30 @@ static void block_job_event_completed(BlockJob *job, const char *msg)
> &error_abort);
> }
>
> +void block_job_pause_all(void)
> +{
> + BlockJob *job = NULL;
> + while ((job = block_job_next(job))) {
> + AioContext *aio_context = blk_get_aio_context(job->blk);
> +
> + aio_context_acquire(aio_context);
> + block_job_pause(job);
> + aio_context_release(aio_context);
> + }
> +}
> +
> +void block_job_resume_all(void)
> +{
> + BlockJob *job = NULL;
> + while ((job = block_job_next(job))) {
> + AioContext *aio_context = blk_get_aio_context(job->blk);
> +
> + aio_context_acquire(aio_context);
> + block_job_resume(job);
> + aio_context_release(aio_context);
> + }
> +}
> +
> void block_job_event_ready(BlockJob *job)
> {
> job->ready = true;
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index 9e906f7..aac87cd 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -235,12 +235,11 @@ void block_job_complete(BlockJob *job, Error **errp);
> BlockJobInfo *block_job_query(BlockJob *job, Error **errp);
>
> /**
> - * block_job_pause:
> - * @job: The job to be paused.
> + * block_job_pause_all:
> *
> - * Asynchronously pause the specified job.
> + * Asynchronously pause all jobs.
> */
> -void block_job_pause(BlockJob *job);
> +void block_job_pause_all(void);
>
> /**
> * block_job_user_pause:
> @@ -260,12 +259,11 @@ void block_job_user_pause(BlockJob *job);
> bool block_job_user_paused(BlockJob *job);
>
> /**
> - * block_job_resume:
> - * @job: The job to be resumed.
> + * block_job_resume_all:
> *
> - * Resume the specified job. Must be paired with a preceding block_job_pause.
> + * Resume all block jobs. Must be paired with a preceding block_job_pause_all.
> */
> -void block_job_resume(BlockJob *job);
> +void block_job_resume_all(void);
>
> /**
> * block_job_user_resume:
>
Reviewed-by: John Snow <jsnow@redhat.com>
On Thu, Mar 23, 2017 at 06:39:22PM +0100, Paolo Bonzini wrote: > Remove use of block_job_pause/resume from outside blockjob.c, thus > making them static. Again, one reason to do this is that > block_job_pause/resume will have different locking rules compared > to everything else that block.c and block/io.c use. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block/io.c | 18 +------- > blockjob.c | 114 ++++++++++++++++++++++++++++------------------- > include/block/blockjob.h | 14 +++--- > 3 files changed, 77 insertions(+), 69 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
© 2016 - 2026 Red Hat, Inc.