Re: [PATCH v5 00/13] mirror: Handle errors after READY cancel

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

10/6/21 18:19, Hanna Reitz wrote:
> Hi,
> 
> v1 cover letter:
> https://lists.nongnu.org/archive/html/qemu-block/2021-07/msg00705.html
> 
> v2 cover letter:
> https://lists.nongnu.org/archive/html/qemu-block/2021-07/msg00747.html
> 
> v3 cover letter:
> https://lists.nongnu.org/archive/html/qemu-block/2021-08/msg00127.html
> 
> v4 cover letter:
> https://lists.nongnu.org/archive/html/qemu-block/2021-09/msg00314.html
> 
> 
> Changes in v5:
> - Added patch 7: When it was soft-cancelled, the mirror job clears
>    .cancelled before leaving its main loop.  The clear intention is to
>    have the job be treated as having completed successfully (and this is
>    also supported by the QMP documentation of block-job-cancel).  It is
>    however possible to cancel the job after it has left its main loop,
>    before it can be unwound, and then the job would again be treated as
>    cancelled.  We don’t want that, so make a soft job-cancel a no-op when
>    .deferred_to_main_loop is true.
>    (This fixes the test-replication failure.)
> 
> - Patch 8: Add a comment in job_cancel() that job_cancel_requested() and
>    job_is_cancelled() are equivalent here, but why we want to inquire
>    job_is_cancelled() instead of job_cancel_requested().
> 
> 
> git-backport-diff against v4:
> 
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
> 001/13:[----] [--] 'job: Context changes in job_completed_txn_abort()'
> 002/13:[----] [--] 'mirror: Keep s->synced on error'
> 003/13:[----] [--] 'mirror: Drop s->synced'
> 004/13:[----] [--] 'job: Force-cancel jobs in a failed transaction'
> 005/13:[----] [--] 'job: @force parameter for job_cancel_sync()'
> 006/13:[----] [--] 'jobs: Give Job.force_cancel more meaning'
> 007/13:[down] 'job: Do not soft-cancel after a job is done'
> 008/13:[0005] [FC] 'job: Add job_cancel_requested()'
> 009/13:[----] [--] 'mirror: Use job_is_cancelled()'
> 010/13:[----] [--] 'mirror: Check job_is_cancelled() earlier'
> 011/13:[----] [--] 'mirror: Stop active mirroring after force-cancel'
> 012/13:[----] [--] 'mirror: Do not clear .cancelled'
> 013/13:[----] [--] 'iotests: Add mirror-ready-cancel-error test'
> 
> 
> Hanna Reitz (13):
>    job: Context changes in job_completed_txn_abort()
>    mirror: Keep s->synced on error
>    mirror: Drop s->synced
>    job: Force-cancel jobs in a failed transaction
>    job: @force parameter for job_cancel_sync()
>    jobs: Give Job.force_cancel more meaning
>    job: Do not soft-cancel after a job is done
>    job: Add job_cancel_requested()
>    mirror: Use job_is_cancelled()
>    mirror: Check job_is_cancelled() earlier
>    mirror: Stop active mirroring after force-cancel
>    mirror: Do not clear .cancelled
>    iotests: Add mirror-ready-cancel-error test
> 
>   include/qemu/job.h                            |  29 +++-
>   block/backup.c                                |   3 +-
>   block/mirror.c                                |  56 ++++---
>   block/replication.c                           |   4 +-
>   blockdev.c                                    |   4 +-
>   job.c                                         |  94 ++++++++++--
>   tests/unit/test-blockjob.c                    |   2 +-
>   tests/qemu-iotests/109.out                    |  60 +++-----
>   .../tests/mirror-ready-cancel-error           | 143 ++++++++++++++++++
>   .../tests/mirror-ready-cancel-error.out       |   5 +
>   tests/qemu-iotests/tests/qsd-jobs.out         |   2 +-
>   11 files changed, 312 insertions(+), 90 deletions(-)
>   create mode 100755 tests/qemu-iotests/tests/mirror-ready-cancel-error
>   create mode 100644 tests/qemu-iotests/tests/mirror-ready-cancel-error.out
> 

Thanks, applied to my jobs branch.

-- 
Best regards,
Vladimir