Re: [PATCH] block-jobs: add final flush

[Hanna Czenczek]

On 04.10.23 15:56, Vladimir Sementsov-Ogievskiy wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> Actually block job is not completed without the final flush. It's
> rather unexpected to have broken target when job was successfully
> completed long ago and now we fail to flush or process just
> crashed/killed.
>
> Mirror job already has mirror_flush() for this. So, it's OK.
>
> Add similar things for other jobs: backup, stream, commit.
>
> Note, that stream has (documented) different treatment of IGNORE
> action: it don't retry the operation, continue execution and report
> error at last. We keep it for final flush too.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>
> Was: [PATCH v4] block-jobs: flush target at the end of .run()
>    But now rewritten.
> Supersedes: <20230725174008.1147467-1-vsementsov@yandex-team.ru>
>
>   block/backup.c             |  2 +-
>   block/block-copy.c         |  7 +++++++
>   block/commit.c             | 16 ++++++++++++----
>   block/stream.c             | 21 +++++++++++++++++----
>   include/block/block-copy.h |  1 +
>   5 files changed, 38 insertions(+), 9 deletions(-)

[...]

> diff --git a/block/commit.c b/block/commit.c
> index aa45beb0f0..5205c77ec9 100644
> --- a/block/commit.c
> +++ b/block/commit.c

[...]

> @@ -187,7 +187,15 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
>           }
>       }
>   
> -    return 0;
> +    do {
> +        ret = blk_co_flush(s->base);
> +        if (ret < 0) {
> +            action = block_job_error_action(&s->common, s->on_error,
> +                                            false, -ret);
> +        }
> +    } while (ret < 0 && action != BLOCK_ERROR_ACTION_REPORT);

Do we need to yield in this loop somewhere so that 
BLOCK_ERROR_ACTION_STOP can pause the job?

Hanna