diff mbox series

[v2,02/40] blockjob: Improve BlockJobInfo.offset/len documentation

Message ID 20180518132114.4070-3-kwolf@redhat.com
State New
Headers show
Series Generic background jobs | expand

Commit Message

Kevin Wolf May 18, 2018, 1:20 p.m. UTC
Clarify that len is just an estimation of the end value of offset, and
that offset increases monotonically while len can change arbitrarily.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/block-core.json | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Eric Blake May 18, 2018, 2:25 p.m. UTC | #1
On 05/18/2018 08:20 AM, Kevin Wolf wrote:
> Clarify that len is just an estimation of the end value of offset, and
> that offset increases monotonically while len can change arbitrarily.

That's tighter than what libvirt promises (and in fact, there are cases 
where libvirt synthesizes an offset/len of 1/1 to indicate completion 
when the information is no longer available from qemu, which means the 
offset has gone backwards to libvirt clients).

But it matches the qemu implementation, and I'm okay if we want to stick 
to those semantics of monotonically increasing offset (the more 
important semantics of being an estimate of time remaining is possible 
whether or not you have a guarantee of a monotonic increase on one of 
the two numbers).

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qapi/block-core.json | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index d32ec95666..0e29abf099 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1148,7 +1148,12 @@
>   # @device: The job identifier. Originally the device name but other
>   #          values are allowed since QEMU 2.7
>   #
> -# @len: the maximum progress value
> +# @len: Estimated @offset value at the completion of the job. This value can
> +#       arbitrarily change while the job is running, in both directions.
> +#
> +# @offset: Progress made until now. The unit is arbitrary and the value can
> +#          only meaningfully be used for the ratio of @offset to @len. The
> +#          value is monotonically increasing.

So I'm okay whether or not you drop that last sentence.

You're also rearranging the docs to occur in the order that the struct 
declares things (good change, and trivial; not sure if you want to call 
it out in the commit message).

Reviewed-by: Eric Blake <eblake@redhat.com>
John Snow May 18, 2018, 5:47 p.m. UTC | #2
On 05/18/2018 09:20 AM, Kevin Wolf wrote:
> Clarify that len is just an estimation of the end value of offset, and
> that offset increases monotonically while len can change arbitrarily.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-core.json | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index d32ec95666..0e29abf099 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1148,7 +1148,12 @@
>  # @device: The job identifier. Originally the device name but other
>  #          values are allowed since QEMU 2.7
>  #
> -# @len: the maximum progress value
> +# @len: Estimated @offset value at the completion of the job. This value can
> +#       arbitrarily change while the job is running, in both directions.
> +#
> +# @offset: Progress made until now. The unit is arbitrary and the value can
> +#          only meaningfully be used for the ratio of @offset to @len. The
> +#          value is monotonically increasing.
>  #
>  # @busy: false if the job is known to be in a quiescent state, with
>  #        no pending I/O.  Since 1.3.
> @@ -1156,8 +1161,6 @@
>  # @paused: whether the job is paused or, if @busy is true, will
>  #          pause itself as soon as possible.  Since 1.3.
>  #
> -# @offset: the current progress value
> -#
>  # @speed: the rate limit, bytes per second
>  #
>  # @io-status: the status of the job (since 1.3)
> 

It matches current actual behavior, so it's probably a good update. It
feels like a change in behavior, but it's rather just codifying the
existing reality.

OK.

Reviewed-by: John Snow <jsnow@redhat.com>
diff mbox series

Patch

diff --git a/qapi/block-core.json b/qapi/block-core.json
index d32ec95666..0e29abf099 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1148,7 +1148,12 @@ 
 # @device: The job identifier. Originally the device name but other
 #          values are allowed since QEMU 2.7
 #
-# @len: the maximum progress value
+# @len: Estimated @offset value at the completion of the job. This value can
+#       arbitrarily change while the job is running, in both directions.
+#
+# @offset: Progress made until now. The unit is arbitrary and the value can
+#          only meaningfully be used for the ratio of @offset to @len. The
+#          value is monotonically increasing.
 #
 # @busy: false if the job is known to be in a quiescent state, with
 #        no pending I/O.  Since 1.3.
@@ -1156,8 +1161,6 @@ 
 # @paused: whether the job is paused or, if @busy is true, will
 #          pause itself as soon as possible.  Since 1.3.
 #
-# @offset: the current progress value
-#
 # @speed: the rate limit, bytes per second
 #
 # @io-status: the status of the job (since 1.3)