Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments
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 kw...@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 ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments
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 kw...@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
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 kw...@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
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 kw...@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 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments
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 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] qapi-schema: Use existing type for drive-backup arguments
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
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 kw...@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
On Tue, 9 Jul 2013 10:05:35 +0200
Kevin Wolf kw...@redhat.com wrote:
This removes duplicated definitions and documentation by reusing the
existing data type.
Signed-off-by: Kevin Wolf kw...@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
