| Message ID | e536acb56aada2f3b701af14b3b78f5b894ec1fb.1459776815.git.berto@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04.04.2016 15:43, Alberto Garcia wrote: > bdrv_drain_all() pauses all block jobs by using bdrv_next() to iterate > over all top-level BlockDriverStates. Therefore the code is unable to > find block jobs in other nodes. > > This patch uses block_job_next() to iterate over all block jobs. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block/io.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/block/io.c b/block/io.c > index c4869b9..884ed1e 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -288,15 +288,21 @@ void bdrv_drain_all(void) > /* Always run first iteration so any pending completion BHs run */ > bool busy = true; > BlockDriverState *bs = NULL; > + BlockJob *job = NULL; > GSList *aio_ctxs = NULL, *ctx; > > + while ((job = block_job_next(job))) { > + AioContext *aio_context = bdrv_get_aio_context(job->bs); > + > + aio_context_acquire(aio_context); > + block_job_pause(job); > + aio_context_release(aio_context); > + } > + > while ((bs = bdrv_next(bs))) { > AioContext *aio_context = bdrv_get_aio_context(bs); > > aio_context_acquire(aio_context); > - if (bs->job) { > - block_job_pause(bs->job); > - } > bdrv_drain_recurse(bs); > aio_context_release(aio_context); > > @@ -333,14 +339,11 @@ void bdrv_drain_all(void) > } > } > > - bs = NULL; > - while ((bs = bdrv_next(bs))) { > - AioContext *aio_context = bdrv_get_aio_context(bs); > + while ((job = block_job_next(job))) { > + AioContext *aio_context = bdrv_get_aio_context(job->bs); Technically, the "bs = NULL;" before didn't do anything either. But in my opinion, it made the code more readable, therefore I'd really like a "job = NULL;" before this loop, too. But it's correct as it is, so: Reviewed-by: Max Reitz <mreitz@redhat.com> > > aio_context_acquire(aio_context); > - if (bs->job) { > - block_job_resume(bs->job); > - } > + block_job_resume(job); > aio_context_release(aio_context); > } > g_slist_free(aio_ctxs); >
On Wed 27 Apr 2016 02:04:33 PM CEST, Max Reitz wrote: >> - bs = NULL; >> - while ((bs = bdrv_next(bs))) { >> - AioContext *aio_context = bdrv_get_aio_context(bs); >> + while ((job = block_job_next(job))) { >> + AioContext *aio_context = bdrv_get_aio_context(job->bs); > > Technically, the "bs = NULL;" before didn't do anything either. But in > my opinion, it made the code more readable, therefore I'd really like > a "job = NULL;" before this loop, too. Looks reasonable, I'll update it in the next revision. Berto
Am 04.04.2016 um 15:43 hat Alberto Garcia geschrieben: > bdrv_drain_all() pauses all block jobs by using bdrv_next() to iterate > over all top-level BlockDriverStates. Therefore the code is unable to > find block jobs in other nodes. > > This patch uses block_job_next() to iterate over all block jobs. > > Signed-off-by: Alberto Garcia <berto@igalia.com> This conflicts with Paolo's patches in block-next. Please rebase. (Apart from that, the change looks sane.) Kevin
diff --git a/block/io.c b/block/io.c index c4869b9..884ed1e 100644 --- a/block/io.c +++ b/block/io.c @@ -288,15 +288,21 @@ void bdrv_drain_all(void) /* Always run first iteration so any pending completion BHs run */ bool busy = true; BlockDriverState *bs = NULL; + BlockJob *job = NULL; GSList *aio_ctxs = NULL, *ctx; + while ((job = block_job_next(job))) { + AioContext *aio_context = bdrv_get_aio_context(job->bs); + + aio_context_acquire(aio_context); + block_job_pause(job); + aio_context_release(aio_context); + } + while ((bs = bdrv_next(bs))) { AioContext *aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); - if (bs->job) { - block_job_pause(bs->job); - } bdrv_drain_recurse(bs); aio_context_release(aio_context); @@ -333,14 +339,11 @@ void bdrv_drain_all(void) } } - bs = NULL; - while ((bs = bdrv_next(bs))) { - AioContext *aio_context = bdrv_get_aio_context(bs); + while ((job = block_job_next(job))) { + AioContext *aio_context = bdrv_get_aio_context(job->bs); aio_context_acquire(aio_context); - if (bs->job) { - block_job_resume(bs->job); - } + block_job_resume(job); aio_context_release(aio_context); } g_slist_free(aio_ctxs);
bdrv_drain_all() pauses all block jobs by using bdrv_next() to iterate over all top-level BlockDriverStates. Therefore the code is unable to find block jobs in other nodes. This patch uses block_job_next() to iterate over all block jobs. Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/io.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)