Re: [Qemu-devel] [PATCH v3 for-2.11] QAPI & interop: Clarify events emitted by 'block-job-cancel'

[Kashyap Chamarthy <kchamart@redhat.com>]

On Fri, Nov 17, 2017 at 11:07:41AM -0600, Eric Blake wrote:
> On 11/17/2017 10:25 AM, Kashyap Chamarthy wrote:

[...]

> Sorry for not wordsmithing on earlier versions:

That's fine.  Always appreciate your wordsmithing :-)

> > +    'mirror' job *after* it has indicated (by emitting the event
> > +    ``BLOCK_JOB_READY``) that the source and target now remain
> > +    synchronized, then ``block-job-cancel`` will emit the event
> 
> Might read slightly nicer as:
> 
> the source and target have reached synchronization,

Ah, that sounds nicer indeed.  Will fix.

[...]

> > +# Note that the 'block-job-cancel' command will emit the event
> > +# BLOCK_JOB_COMPLETED if you issue it ('block-job-cancel') after 'drive-mirror'
> > +# has indicated (by emitting the event BLOCK_JOB_READY) that the source and
> > +# destination remain synchronized.  In this case, the BLOCK_JOB_COMPLETED
> > +# event indicates that synchronization (from 'drive-mirror') has successfully
> > +# ended and the destination now has a point-in-time copy, which is at the time
> > +# of cancel.
> 
> 
> Accurate, but a bit hard to follow the flow of the sentence.

Yeah, I wasn't satisfied with my phrasing too.  I sat on it for nearly
half an hour, and still it came out super clunky.

> Might read nicer as:
> 
> Note that if you issue 'block-job-cancel' after 'drive-mirror' has
> indicated (via the event BLOCK_JOB_READY) that the source and
> destination are synchronized, then the event triggered by this command
> changes to BLOCK_JOB_COMPLETED, to indicate that the mirroring has
> ended and the destination now has a point-in-time copy tied to the
> time of the cancellation.

Thanks, yours reads much nicer.

> Documentation is worth of inclusion in 2.11.  Whether you keep your
> wording, or incorporate mine in a v4, you can add:
> Reviewed-by: Eric Blake <eblake@redhat.com>

I'll definitely incorporate in v4.  

I'll also make similar wording adjustment in live-block-operations.rst.

Thanks for the quick review.

-- 
/kashyap