[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: fix memory leak in early exit
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH] block: fix memory leak in early exit |
Date: |
Fri, 16 Oct 2015 10:31:25 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Thu, 10/15 17:54, Stefan Hajnoczi wrote:
> The stream block job has two early exit code paths. They do not free
> s->backing_file_str.
>
> Also, the early exits rely on the fact that the coroutine hasn't yielded
> yet and was launched from the main thread. Therefore the coroutine is
> guaranteed to be running in the main thread where block_job_completed()
> may be called safely. This is very subtle so it's nice to eliminate the
> assumption by unifying the early exit with the normal exit code path.
>
> Cc: Fam Zheng <address@hidden>
> Cc: Jeff Cody <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
> block/stream.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/block/stream.c b/block/stream.c
> index ab0bd05..1986e9a 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -120,16 +120,16 @@ static void coroutine_fn stream_run(void *opaque)
> int ret = 0;
> int n = 0;
> void *buf;
> + bool reached_end = false;
>
> if (!bs->backing_hd) {
> - block_job_completed(&s->common, 0);
> - return;
> + goto out;
> }
>
> s->common.len = bdrv_getlength(bs);
> if (s->common.len < 0) {
> - block_job_completed(&s->common, s->common.len);
> - return;
> + ret = s->common.len;
> + goto out;
> }
>
> end = s->common.len >> BDRV_SECTOR_BITS;
> @@ -207,6 +207,10 @@ wait:
> s->common.offset += n * BDRV_SECTOR_SIZE;
> }
>
> + if (sector_num == end) {
> + reached_end = true;
> + }
> +
> if (!base) {
> bdrv_disable_copy_on_read(bs);
> }
> @@ -216,10 +220,11 @@ wait:
>
> qemu_vfree(buf);
>
> +out:
> /* Modify backing chain and close BDSes in main loop */
> data = g_malloc(sizeof(*data));
> data->ret = ret;
> - data->reached_end = sector_num == end;
> + data->reached_end = reached_end;
> block_job_defer_to_main_loop(&s->common, stream_complete, data);
> }
>
> --
> 2.4.3
>
Reviewed-by: Fam Zheng <address@hidden>