Re: [PATCH RFC 0/3] mirror: rework soft-cancelling READY mirror

[Max Reitz <mreitz@redhat.com>]

On 27.07.21 18:47, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> That's an alternative to (part of) Max's
> "[PATCH for-6.1? v2 0/7] mirror: Handle errors after READY cancel"
> and shows' my idea of handling soft-cancelling READY mirror case
> directly in qmp_block_job_cancel. And cleanup all other job cancelling
> functions.
>
> That's untested draft, don't take it to heart :)

Well, I would have preferred it if you’d rebased this on top of that 
series, precisely because it’s an alternative to only part of it. And if 
it’s just an untested draft, that would have been even better, because 
it would’ve given a better idea on what the cleanup looks like.

There are also things like this series making cancel internally always a 
force-cancel, where I’m not sure whether we want that in the replication 
driver or not[1].  With my series, we add an explicit parameter, so 
we’re forced to think about it, and then in this series on top we can 
just drop the parameter for all force-cancel invocations again, and for 
all non-force-cancel invocations we would have to think a bit more.

Specifically as for this series, I don’t like job_complete_ex() very 
much, I think the parameter should be part of job_complete() itself.  If 
we think that’s too specific of a mirror parameter to include in normal 
job_complete(), well, then there shouldn’t be a job_complete_ex() 
either, and do_graph_change should be a property of the mirror job 
(perhaps as pivot_on_completion) that’s cleared by 
qmp_block_job_cancel() before invoking job_complete().

Max

[1] Although looking at it again now, it probably wants force-cancel.