When you cancel an in-progress live block operation with QMP
`block-job-cancel`, it emits the event: BLOCK_JOB_CANCELLED. However,
when `block-job-cancel` is issued after `drive-mirror` has indicated (by
emitting the event BLOCK_JOB_READY) that the source and destination
remain synchronized:
[...] # Snip `drive-mirror` invocation & outputs
{
"execute":"block-job-cancel",
"arguments":{
"device":"virtio0"
}
}
{"return": {}}
It (`block-job-cancel`) will counterintuitively emit the event
'BLOCK_JOB_COMPLETED':
{
"timestamp":{
"seconds":1510678024,
"microseconds":526240
},
"event":"BLOCK_JOB_COMPLETED",
"data":{
"device":"virtio0",
"len":41126400,
"offset":41126400,
"speed":0,
"type":"mirror"
}
}
But this is expected behaviour, where the _COMPLETED event indicates
that synchronization has successfully ended (and the destination has a
point-in-time copy, which is at the time of cancel).
So add a small note to this effect. (Thanks: Max Reitz for reminding
me of this on IRC.)
Signed-off-by: Kashyap Chamarthy <kchamart@redhat.com>
---
Changes in v2:
- "Note:" seems to be a special construct in Patchew, my usage caused a
build failure. So do: s/Note:/Note that/
- Add the missing 'Signed-off-by'
---
qapi/block-core.json | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ab96e348e6317bb42769ae20f4a4519bac02e93a..4ecfd1fbc5bc231c69da15d489c44e3e8b0706a5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2065,6 +2065,14 @@
# BLOCK_JOB_CANCELLED event. Before that happens the job is still visible when
# enumerated using query-block-jobs.
#
+# 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.
+#
# For streaming, the image file retains its backing file unless the streaming
# operation happens to complete just as it is being cancelled. A new streaming
# operation can be started at a later time to finish copying all data from the
--
2.9.5
On 11/15/2017 04:09 AM, Kashyap Chamarthy wrote:
> When you cancel an in-progress live block operation with QMP
> `block-job-cancel`, it emits the event: BLOCK_JOB_CANCELLED. However,
> when `block-job-cancel` is issued after `drive-mirror` has indicated (by
> emitting the event BLOCK_JOB_READY) that the source and destination
> remain synchronized:
>
> [...] # Snip `drive-mirror` invocation & outputs
> {
> "execute":"block-job-cancel",
> "arguments":{
> "device":"virtio0"
> }
> }
>
> {"return": {}}
>
> It (`block-job-cancel`) will counterintuitively emit the event
> 'BLOCK_JOB_COMPLETED':
>
> {
> "timestamp":{
> "seconds":1510678024,
> "microseconds":526240
> },
> "event":"BLOCK_JOB_COMPLETED",
> "data":{
> "device":"virtio0",
> "len":41126400,
> "offset":41126400,
> "speed":0,
> "type":"mirror"
> }
> }
>
> But this is expected behaviour, where the _COMPLETED event indicates
> that synchronization has successfully ended (and the destination has a
> point-in-time copy, which is at the time of cancel).
>
> So add a small note to this effect. (Thanks: Max Reitz for reminding
> me of this on IRC.)
>
I suppose this difference probably isn't covered in what was the
bitmaps.md doc file (we don't bother covering mirror there, only
backup); is it covered sufficiently in live-block-operations.rst ?
--js
On Wed, Nov 15, 2017 at 02:15:57PM -0500, John Snow wrote:
>
>
> On 11/15/2017 04:09 AM, Kashyap Chamarthy wrote:
> > When you cancel an in-progress live block operation with QMP
> > `block-job-cancel`, it emits the event: BLOCK_JOB_CANCELLED. However,
> > when `block-job-cancel` is issued after `drive-mirror` has indicated (by
> > emitting the event BLOCK_JOB_READY) that the source and destination
> > remain synchronized:
[...]
> > But this is expected behaviour, where the _COMPLETED event indicates
> > that synchronization has successfully ended (and the destination has a
> > point-in-time copy, which is at the time of cancel).
> >
> > So add a small note to this effect. (Thanks: Max Reitz for reminding
> > me of this on IRC.)
> >
>
> I suppose this difference probably isn't covered in what was the
> bitmaps.md doc file (we don't bother covering mirror there, only
> backup);
Your supposition is correct: Looking at the now-renamed
'bitmaps.rst'[1], it isn't covered there.
> is it covered sufficiently in live-block-operations.rst ?
I looked in there[2] too. Short answer: no. Long: In the "Live disk
synchronization — drive-mirror and blockdev-mirror" section, I simply
seemed to declare:
"Issuing the command ``block-job-cancel`` after it emits the event
``BLOCK_JOB_CANCELLED``"
As if that's the *only* event it emits, which is clearly not the case.
So while at it, wonder if should I also update it
('live-block-operations.rst') too.
[1] https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/bitmaps.rst
[2] https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/live-block-operations.rst#l513
--
/kashyap
On 11/15/2017 04:54 PM, Kashyap Chamarthy wrote:
> On Wed, Nov 15, 2017 at 02:15:57PM -0500, John Snow wrote:
>>
>>
>> On 11/15/2017 04:09 AM, Kashyap Chamarthy wrote:
>>> When you cancel an in-progress live block operation with QMP
>>> `block-job-cancel`, it emits the event: BLOCK_JOB_CANCELLED. However,
>>> when `block-job-cancel` is issued after `drive-mirror` has indicated (by
>>> emitting the event BLOCK_JOB_READY) that the source and destination
>>> remain synchronized:
>
> [...]
>
>>> But this is expected behaviour, where the _COMPLETED event indicates
>>> that synchronization has successfully ended (and the destination has a
>>> point-in-time copy, which is at the time of cancel).
>>>
>>> So add a small note to this effect. (Thanks: Max Reitz for reminding
>>> me of this on IRC.)
>>>
>>
>> I suppose this difference probably isn't covered in what was the
>> bitmaps.md doc file (we don't bother covering mirror there, only
>> backup);
>
> Your supposition is correct: Looking at the now-renamed
> 'bitmaps.rst'[1], it isn't covered there.
>
Makes sense, I don't think we need to correct this, and
>> is it covered sufficiently in live-block-operations.rst ?
>
> I looked in there[2] too. Short answer: no. Long: In the "Live disk
> synchronization — drive-mirror and blockdev-mirror" section, I simply
> seemed to declare:
>
> "Issuing the command ``block-job-cancel`` after it emits the event
> ``BLOCK_JOB_CANCELLED``"
>
> As if that's the *only* event it emits, which is clearly not the case.
> So while at it, wonder if should I also update it
> ('live-block-operations.rst') too.
>
It's an interesting gotcha that I wasn't really acutely aware of myself,
so having it in the doc format for API programmers who aren't
necessarily digging through our source sounds like a pleasant courtesy.
> [1] https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/bitmaps.rst
> [2] https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/live-block-operations.rst#l513
>
Thanks Kashyap :)
On Wed, Nov 15, 2017 at 04:56:13PM -0500, John Snow wrote:
>
>
> On 11/15/2017 04:54 PM, Kashyap Chamarthy wrote:
> > On Wed, Nov 15, 2017 at 02:15:57PM -0500, John Snow wrote:
[...]
> >> is it covered sufficiently in live-block-operations.rst ?
> >
> > I looked in there[2] too. Short answer: no. Long: In the "Live disk
> > synchronization — drive-mirror and blockdev-mirror" section, I simply
> > seemed to declare:
> >
> > "Issuing the command ``block-job-cancel`` after it emits the event
> > ``BLOCK_JOB_CANCELLED``"
> >
> > As if that's the *only* event it emits, which is clearly not the case.
> > So while at it, wonder if should I also update it
> > ('live-block-operations.rst') too.
> >
>
> It's an interesting gotcha that I wasn't really acutely aware of myself,
> so having it in the doc format for API programmers who aren't
> necessarily digging through our source sounds like a pleasant courtesy.
Indeed, will do. (Just for my own clarity, did you imply: don't update
it in block-core.json? FWIW, my first instinct is to check the QAPI
documentation for such things, that's why I wrote there first :-))
Thanks for looking.
[...]
--
/kashyap
On 11/16/2017 04:14 AM, Kashyap Chamarthy wrote:
> On Wed, Nov 15, 2017 at 04:56:13PM -0500, John Snow wrote:
>>
>>
>> On 11/15/2017 04:54 PM, Kashyap Chamarthy wrote:
>>> On Wed, Nov 15, 2017 at 02:15:57PM -0500, John Snow wrote:
>
> [...]
>
>>>> is it covered sufficiently in live-block-operations.rst ?
>>>
>>> I looked in there[2] too. Short answer: no. Long: In the "Live disk
>>> synchronization — drive-mirror and blockdev-mirror" section, I simply
>>> seemed to declare:
>>>
>>> "Issuing the command ``block-job-cancel`` after it emits the event
>>> ``BLOCK_JOB_CANCELLED``"
>>>
>>> As if that's the *only* event it emits, which is clearly not the case.
>>> So while at it, wonder if should I also update it
>>> ('live-block-operations.rst') too.
>>>
>>
>> It's an interesting gotcha that I wasn't really acutely aware of myself,
>> so having it in the doc format for API programmers who aren't
>> necessarily digging through our source sounds like a pleasant courtesy.
>
> Indeed, will do. (Just for my own clarity, did you imply: don't update
> it in block-core.json? FWIW, my first instinct is to check the QAPI
> documentation for such things, that's why I wrote there first :-))
>
No, definitely update both.
> Thanks for looking.
>
> [...]
>
Am 16.11.2017 um 10:14 hat Kashyap Chamarthy geschrieben:
> On Wed, Nov 15, 2017 at 04:56:13PM -0500, John Snow wrote:
> >
> >
> > On 11/15/2017 04:54 PM, Kashyap Chamarthy wrote:
> > > On Wed, Nov 15, 2017 at 02:15:57PM -0500, John Snow wrote:
>
> [...]
>
> > >> is it covered sufficiently in live-block-operations.rst ?
> > >
> > > I looked in there[2] too. Short answer: no. Long: In the "Live disk
> > > synchronization — drive-mirror and blockdev-mirror" section, I simply
> > > seemed to declare:
> > >
> > > "Issuing the command ``block-job-cancel`` after it emits the event
> > > ``BLOCK_JOB_CANCELLED``"
> > >
> > > As if that's the *only* event it emits, which is clearly not the case.
> > > So while at it, wonder if should I also update it
> > > ('live-block-operations.rst') too.
> > >
> >
> > It's an interesting gotcha that I wasn't really acutely aware of myself,
> > so having it in the doc format for API programmers who aren't
> > necessarily digging through our source sounds like a pleasant courtesy.
>
> Indeed, will do. (Just for my own clarity, did you imply: don't update
> it in block-core.json? FWIW, my first instinct is to check the QAPI
> documentation for such things, that's why I wrote there first :-))
Will that be a separate patch or do you intend to send a v3?
Kevin
On Fri, Nov 17, 2017 at 11:49:05AM +0100, Kevin Wolf wrote: > Am 16.11.2017 um 10:14 hat Kashyap Chamarthy geschrieben: > > On Wed, Nov 15, 2017 at 04:56:13PM -0500, John Snow wrote: [...] > > > It's an interesting gotcha that I wasn't really acutely aware of myself, > > > so having it in the doc format for API programmers who aren't > > > necessarily digging through our source sounds like a pleasant courtesy. > > > > Indeed, will do. (Just for my own clarity, did you imply: don't update > > it in block-core.json? FWIW, my first instinct is to check the QAPI > > documentation for such things, that's why I wrote there first :-)) > > Will that be a separate patch or do you intend to send a v3? I can send a separate one if you prefer, but I was intending to send a v3 with both files fixed as it's just a related trivial change. Is that okay? -- /kashyap
© 2016 - 2024 Red Hat, Inc.