Re: [Qemu-devel] [PATCH] qmp: make block job command naming consistent

[Luiz Capitulino]

On Wed, 11 Apr 2012 16:27:10 +0100
Stefan Hajnoczi <address@hidden> wrote:

> The block streaming and job commands used '_' instead of '-' for reasons
> of compatibility with libvirt, which already included support for the
> '_' naming.  However, the semantics of block_job_cancel have changed and
> libvirt now needs to handle the new semantics.
> 
> Since the old semantics were never in a QEMU release we can still rename
> the commands to use '-' instead of '_'.  Libvirt is also happy because
> the new name can be used to distinguish QEMU binaries that support the
> latest block-job-cancel semantics from those that include a downstream
> block_job_cancel command.
> 
> Therefore, let's apply the QAPI/QMP naming rules to the block streaming
> and job commands.  QEMU 1.1 will be the first release with these
> commands so no upstream users can break.
> 
> Note that HMP commands are left with '_' because that is the convention
> there.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>

Applied to the qmp branch, thanks.

> ---
>  qapi-schema.json       |   15 ++++++++-------
>  qmp-commands.hx        |    6 +++---
>  tests/qemu-iotests/030 |   12 ++++++------
>  3 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index ace55f3..6499895 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1548,7 +1548,8 @@
>    'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
>              'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } }
>  
> -# @block_stream:
> +##
> +# @block-stream:
>  #
>  # Copy data from a backing file into a block device.
>  #
> @@ -1556,7 +1557,7 @@
>  # backing file has been copied.  This command returns immediately once 
> streaming
>  # has started.  The status of ongoing block streaming operations can be 
> checked
>  # with query-block-jobs.  The operation can be stopped before it has 
> completed
> -# using the block_job_cancel command.
> +# using the block-job-cancel command.
>  #
>  # If a base file is specified then sectors are not copied from that base 
> file and
>  # its backing chain.  When streaming completes the image file will have the 
> base
> @@ -1578,10 +1579,10 @@
>  #
>  # Since: 1.1
>  ##
> -{ 'command': 'block_stream', 'data': { 'device': 'str', '*base': 'str' } }
> +{ 'command': 'block-stream', 'data': { 'device': 'str', '*base': 'str' } }
>  
>  ##
> -# @block_job_set_speed:
> +# @block-job-set-speed:
>  #
>  # Set maximum speed for a background block operation.
>  #
> @@ -1599,11 +1600,11 @@
>  #
>  # Since: 1.1
>  ##
> -{ 'command': 'block_job_set_speed',
> +{ 'command': 'block-job-set-speed',
>    'data': { 'device': 'str', 'value': 'int' } }
>  
>  ##
> -# @block_job_cancel:
> +# @block-job-cancel:
>  #
>  # Stop an active block streaming operation.
>  #
> @@ -1629,7 +1630,7 @@
>  #
>  # Since: 1.1
>  ##
> -{ 'command': 'block_job_cancel', 'data': { 'device': 'str' } }
> +{ 'command': 'block-job-cancel', 'data': { 'device': 'str' } }
>  
>  ##
>  # @ObjectTypeInfo:
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index c09ee85..f972332 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -687,19 +687,19 @@ Example:
>  EQMP
>  
>      {
> -        .name       = "block_stream",
> +        .name       = "block-stream",
>          .args_type  = "device:B,base:s?",
>          .mhandler.cmd_new = qmp_marshal_input_block_stream,
>      },
>  
>      {
> -        .name       = "block_job_set_speed",
> +        .name       = "block-job-set-speed",
>          .args_type  = "device:B,value:o",
>          .mhandler.cmd_new = qmp_marshal_input_block_job_set_speed,
>      },
>  
>      {
> -        .name       = "block_job_cancel",
> +        .name       = "block-job-cancel",
>          .args_type  = "device:B",
>          .mhandler.cmd_new = qmp_marshal_input_block_job_cancel,
>      },
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 1faf984..978fd82 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -49,7 +49,7 @@ class TestSingleDrive(ImageStreamingTestCase):
>      def test_stream(self):
>          self.assert_no_active_streams()
>  
> -        result = self.vm.qmp('block_stream', device='drive0')
> +        result = self.vm.qmp('block-stream', device='drive0')
>          self.assert_qmp(result, 'return', {})
>  
>          completed = False
> @@ -68,7 +68,7 @@ class TestSingleDrive(ImageStreamingTestCase):
>                           'image file not fully populated after streaming')
>  
>      def test_device_not_found(self):
> -        result = self.vm.qmp('block_stream', device='nonexistent')
> +        result = self.vm.qmp('block-stream', device='nonexistent')
>          self.assert_qmp(result, 'error/class', 'DeviceNotFound')
>  
>  class TestStreamStop(ImageStreamingTestCase):
> @@ -90,14 +90,14 @@ class TestStreamStop(ImageStreamingTestCase):
>  
>          self.assert_no_active_streams()
>  
> -        result = self.vm.qmp('block_stream', device='drive0')
> +        result = self.vm.qmp('block-stream', device='drive0')
>          self.assert_qmp(result, 'return', {})
>  
>          time.sleep(1)
>          events = self.vm.get_qmp_events(wait=False)
>          self.assertEqual(events, [], 'unexpected QMP event: %s' % events)
>  
> -        self.vm.qmp('block_job_cancel', device='drive0')
> +        self.vm.qmp('block-job-cancel', device='drive0')
>          self.assert_qmp(result, 'return', {})
>  
>          cancelled = False
> @@ -129,10 +129,10 @@ class TestSetSpeed(ImageStreamingTestCase):
>      def perf_test_set_speed(self):
>          self.assert_no_active_streams()
>  
> -        result = self.vm.qmp('block_stream', device='drive0')
> +        result = self.vm.qmp('block-stream', device='drive0')
>          self.assert_qmp(result, 'return', {})
>  
> -        result = self.vm.qmp('block_job_set_speed', device='drive0', value=8 
> * 1024 * 1024)
> +        result = self.vm.qmp('block-job-set-speed', device='drive0', value=8 
> * 1024 * 1024)
>          self.assert_qmp(result, 'return', {})
>  
>          completed = False