[Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Kevin Wolf]

This removes duplicated definitions and documentation by reusing the
existing data type.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---

To be applied on top of '[PATCH v3 0/3] qapi: Top-level type reference for
command definitions'

 qapi-schema.json | 32 ++------------------------------
 1 file changed, 2 insertions(+), 30 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index a90aeb1..b251d28 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1791,42 +1791,14 @@
 # The operation can be stopped before it has completed using the
 # block-job-cancel command.
 #
-# @device: the name of the device which should be copied.
-#
-# @target: the target of the new image. If the file exists, or if it
-#          is a device, the existing file/device will be used as the new
-#          destination.  If it does not exist, a new file will be created.
-#
-# @format: #optional the format of the new destination, default is to
-#          probe if @mode is 'existing', else the format of the source
-#
-# @mode: #optional whether and how QEMU should create a new image, default is
-#        'absolute-paths'.
-#
-# @speed: #optional the maximum speed, in bytes per second
-#
-# @on-source-error: #optional the action to take on an error on the source,
-#                   default 'report'.  'stop' and 'enospc' can only be used
-#                   if the block device supports io-status (see BlockInfo).
-#
-# @on-target-error: #optional the action to take on an error on the target,
-#                   default 'report' (no limitations, since this applies to
-#                   a different block device than @device).
-#
-# Note that @on-source-error and @on-target-error only affect background I/O.
-# If an error occurs during a guest write request, the device's rerror/werror
-# actions will be used.
+# For the arguments, see the documentation of DriveBackup.
 #
 # Returns: nothing on success
 #          If @device is not a valid block device, DeviceNotFound
 #
 # Since 1.6
 ##
-{ 'command': 'drive-backup',
-  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
-            '*mode': 'NewImageMode', '*speed': 'int',
-            '*on-source-error': 'BlockdevOnError',
-            '*on-target-error': 'BlockdevOnError' } }
+{ 'command': 'drive-backup', 'data': 'DriveBackup' }
 
 ##
 # @drive-mirror
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

On 07/09/2013 02:05 AM, Kevin Wolf wrote:
> This removes duplicated definitions and documentation by reusing the
> existing data type.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> 
> To be applied on top of '[PATCH v3 0/3] qapi: Top-level type reference for
> command definitions'
> 
>  qapi-schema.json | 32 ++------------------------------
>  1 file changed, 2 insertions(+), 30 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Fam Zheng]

On Tue, 07/09 10:05, Kevin Wolf wrote:
> This removes duplicated definitions and documentation by reusing the
> existing data type.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> 
> To be applied on top of '[PATCH v3 0/3] qapi: Top-level type reference for
> command definitions'
> 
>  qapi-schema.json | 32 ++------------------------------
>  1 file changed, 2 insertions(+), 30 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a90aeb1..b251d28 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1791,42 +1791,14 @@
>  # The operation can be stopped before it has completed using the
>  # block-job-cancel command.
>  #
Are these lines, ...
> -# @device: the name of the device which should be copied.
> -#
> -# @target: the target of the new image. If the file exists, or if it
> -#          is a device, the existing file/device will be used as the new
> -#          destination.  If it does not exist, a new file will be created.
> -#
> -# @format: #optional the format of the new destination, default is to
> -#          probe if @mode is 'existing', else the format of the source
> -#
> -# @mode: #optional whether and how QEMU should create a new image, default is
> -#        'absolute-paths'.
> -#
> -# @speed: #optional the maximum speed, in bytes per second
> -#
> -# @on-source-error: #optional the action to take on an error on the source,
> -#                   default 'report'.  'stop' and 'enospc' can only be used
> -#                   if the block device supports io-status (see BlockInfo).
> -#
> -# @on-target-error: #optional the action to take on an error on the target,
> -#                   default 'report' (no limitations, since this applies to
> -#                   a different block device than @device).
> -#
> -# Note that @on-source-error and @on-target-error only affect background I/O.
> -# If an error occurs during a guest write request, the device's rerror/werror
> -# actions will be used.
> +# For the arguments, see the documentation of DriveBackup.
>  #
>  # Returns: nothing on success
>  #          If @device is not a valid block device, DeviceNotFound
>  #
>  # Since 1.6
>  ##
and these still duplication of those comments for type declaration?

Thanks

-- 
Fam

Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Kevin Wolf]

Am 09.07.2013 um 13:58 hat Fam Zheng geschrieben:
> On Tue, 07/09 10:05, Kevin Wolf wrote:
> > This removes duplicated definitions and documentation by reusing the
> > existing data type.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > 
> > To be applied on top of '[PATCH v3 0/3] qapi: Top-level type reference for
> > command definitions'
> > 
> >  qapi-schema.json | 32 ++------------------------------
> >  1 file changed, 2 insertions(+), 30 deletions(-)
> > 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index a90aeb1..b251d28 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -1791,42 +1791,14 @@
> >  # The operation can be stopped before it has completed using the
> >  # block-job-cancel command.
> >  #
> Are these lines, ...
> > -# @device: the name of the device which should be copied.
> > -#
> > -# @target: the target of the new image. If the file exists, or if it
> > -#          is a device, the existing file/device will be used as the new
> > -#          destination.  If it does not exist, a new file will be created.
> > -#
> > -# @format: #optional the format of the new destination, default is to
> > -#          probe if @mode is 'existing', else the format of the source
> > -#
> > -# @mode: #optional whether and how QEMU should create a new image, default is
> > -#        'absolute-paths'.
> > -#
> > -# @speed: #optional the maximum speed, in bytes per second
> > -#
> > -# @on-source-error: #optional the action to take on an error on the source,
> > -#                   default 'report'.  'stop' and 'enospc' can only be used
> > -#                   if the block device supports io-status (see BlockInfo).
> > -#
> > -# @on-target-error: #optional the action to take on an error on the target,
> > -#                   default 'report' (no limitations, since this applies to
> > -#                   a different block device than @device).
> > -#
> > -# Note that @on-source-error and @on-target-error only affect background I/O.
> > -# If an error occurs during a guest write request, the device's rerror/werror
> > -# actions will be used.
> > +# For the arguments, see the documentation of DriveBackup.
> >  #
> >  # Returns: nothing on success
> >  #          If @device is not a valid block device, DeviceNotFound
> >  #
> >  # Since 1.6
> >  ##
> and these still duplication of those comments for type declaration?

No, the DriveBackup documentation doesn't describe what the command does
or what it returns. You might argue that it should be there, but I think
that's out of scope for this patch and you can do it separately if you
like.

Kevin

Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 851 bytes --]

On 07/09/2013 06:04 AM, Kevin Wolf wrote:
>> and these still duplication of those comments for type declaration?
> 
> No, the DriveBackup documentation doesn't describe what the command does
> or what it returns. You might argue that it should be there, but I think
> that's out of scope for this patch and you can do it separately if you
> like.

Besides, the DriveBackup type is usable within the 'transaction'
command, where it does NOT have an associated return type (it is just
one of many actions, where ALL actions have no return, and the overall
'transaction' command either succeeds or fails).  I don't think any
separate patch is warranted here, unless you also make the same
arguments for 'blockdev-snapshot-sync'.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Kevin Wolf]

Am 09.07.2013 um 14:09 hat Eric Blake geschrieben:
> On 07/09/2013 06:04 AM, Kevin Wolf wrote:
> >> and these still duplication of those comments for type declaration?
> > 
> > No, the DriveBackup documentation doesn't describe what the command does
> > or what it returns. You might argue that it should be there, but I think
> > that's out of scope for this patch and you can do it separately if you
> > like.
> 
> Besides, the DriveBackup type is usable within the 'transaction'
> command, where it does NOT have an associated return type (it is just
> one of many actions, where ALL actions have no return, and the overall
> 'transaction' command either succeeds or fails).  I don't think any
> separate patch is warranted here, unless you also make the same
> arguments for 'blockdev-snapshot-sync'.

Ah, right, I forgot about this. So the return type should stay where it
is. What you still could move is the command description ("Start a
point-in-time copy [...]") because that one applies to the transaction
command as well.

Kevin

Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

On 07/09/2013 05:58 AM, Fam Zheng wrote:
> On Tue, 07/09 10:05, Kevin Wolf wrote:
>> This removes duplicated definitions and documentation by reusing the
>> existing data type.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---

>> +++ b/qapi-schema.json
>> @@ -1791,42 +1791,14 @@
>>  # The operation can be stopped before it has completed using the
>>  # block-job-cancel command.
>>  #
> Are these lines, ...

>> -# actions will be used.
>> +# For the arguments, see the documentation of DriveBackup.
>>  #
>>  # Returns: nothing on success
>>  #          If @device is not a valid block device, DeviceNotFound
>>  #
>>  # Since 1.6
>>  ##
> and these still duplication of those comments for type declaration?

Are you talking about the lines that were elided or the lines that
remain?  The lines that were elided are duplicates of the documentation
of the DriveBackup struct; the lines that remain (command overview,
Returns, and Since designations) must remain because they independently
document the 'drive-backup' command.  The trivial amount of remaining
duplication (basically, the Since 1.6 line) matches what was already
done for the blockdev-snapshot-sync command, because every entity (both
types and commands) should mention when they were introduced.  I see no
problem with the patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Fam Zheng]

On Tue, 07/09 06:07, Eric Blake wrote:
> On 07/09/2013 05:58 AM, Fam Zheng wrote:
> > On Tue, 07/09 10:05, Kevin Wolf wrote:
> >> This removes duplicated definitions and documentation by reusing the
> >> existing data type.
> >>
> >> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >> ---
> 
> >> +++ b/qapi-schema.json
> >> @@ -1791,42 +1791,14 @@
> >>  # The operation can be stopped before it has completed using the
> >>  # block-job-cancel command.
> >>  #
> > Are these lines, ...
> 
> >> -# actions will be used.
> >> +# For the arguments, see the documentation of DriveBackup.
> >>  #
> >>  # Returns: nothing on success
> >>  #          If @device is not a valid block device, DeviceNotFound
> >>  #
> >>  # Since 1.6
> >>  ##
> > and these still duplication of those comments for type declaration?
> 
> Are you talking about the lines that were elided or the lines that
> remain?  The lines that were elided are duplicates of the documentation
I thought there was a duplication for the remaining. Kevin explained
that too. Thanks.
> of the DriveBackup struct; the lines that remain (command overview,
> Returns, and Since designations) must remain because they independently
> document the 'drive-backup' command.  The trivial amount of remaining
> duplication (basically, the Since 1.6 line) matches what was already
> done for the blockdev-snapshot-sync command, because every entity (both
> types and commands) should mention when they were introduced.  I see no
> problem with the patch.
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 



-- 
Fam

Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments

[Luiz Capitulino]

On Tue,  9 Jul 2013 10:05:35 +0200
Kevin Wolf <kwolf@redhat.com> wrote:

> This removes duplicated definitions and documentation by reusing the
> existing data type.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Applied to the qmp branch, thanks.

> ---
> 
> To be applied on top of '[PATCH v3 0/3] qapi: Top-level type reference for
> command definitions'
> 
>  qapi-schema.json | 32 ++------------------------------
>  1 file changed, 2 insertions(+), 30 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a90aeb1..b251d28 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1791,42 +1791,14 @@
>  # The operation can be stopped before it has completed using the
>  # block-job-cancel command.
>  #
> -# @device: the name of the device which should be copied.
> -#
> -# @target: the target of the new image. If the file exists, or if it
> -#          is a device, the existing file/device will be used as the new
> -#          destination.  If it does not exist, a new file will be created.
> -#
> -# @format: #optional the format of the new destination, default is to
> -#          probe if @mode is 'existing', else the format of the source
> -#
> -# @mode: #optional whether and how QEMU should create a new image, default is
> -#        'absolute-paths'.
> -#
> -# @speed: #optional the maximum speed, in bytes per second
> -#
> -# @on-source-error: #optional the action to take on an error on the source,
> -#                   default 'report'.  'stop' and 'enospc' can only be used
> -#                   if the block device supports io-status (see BlockInfo).
> -#
> -# @on-target-error: #optional the action to take on an error on the target,
> -#                   default 'report' (no limitations, since this applies to
> -#                   a different block device than @device).
> -#
> -# Note that @on-source-error and @on-target-error only affect background I/O.
> -# If an error occurs during a guest write request, the device's rerror/werror
> -# actions will be used.
> +# For the arguments, see the documentation of DriveBackup.
>  #
>  # Returns: nothing on success
>  #          If @device is not a valid block device, DeviceNotFound
>  #
>  # Since 1.6
>  ##
> -{ 'command': 'drive-backup',
> -  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
> -            '*mode': 'NewImageMode', '*speed': 'int',
> -            '*on-source-error': 'BlockdevOnError',
> -            '*on-target-error': 'BlockdevOnError' } }
> +{ 'command': 'drive-backup', 'data': 'DriveBackup' }
>  
>  ##
>  # @drive-mirror