Re: [Qemu-devel] [v2 5/5] qapi: allow blockdev-add for ssh

[Max Reitz]

On 15.10.2016 11:04, Ashijeet Acharya wrote:
> Introduce new object 'BlockdevOptionsSsh' in qapi/block-core.json to
> support blockdev-add for SSH network protocol driver. Use only 'struct
> InetSocketAddress' since SSH only supports connection over TCP.
> 
> Signed-off-by: Ashijeet Acharya <address@hidden>
> ---
>  qapi/block-core.json | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 9d797b8..2e8a390 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1716,7 +1716,8 @@
>              'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
>              'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co',
>              'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> -         'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
> +            'replication', 'ssh', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc',
> +            'vvfat' ] }
>  
>  ##
>  # @BlockdevOptionsFile
> @@ -1953,6 +1954,25 @@
>              '*vport': 'int',
>              '*segment': 'str' } }
>  
> +##
> +# @BlockdevoptionsSsh

Should be *BlockdevOptionsSsh.

> +#
> +# @server:              host address and port number

It could be argued that the port number is part of the host address. I'd
therefore just describe it as "host address" since you can specify some
other options to, like @ipv6.

> +#
> +# @path:                path to the image on the host
> +#
> +# @user:                user as which to connect

This can actually be an optional argument, and I'd make it one (it
defaults to the current local user name).

> +#
> +# @host_key_check       defines how and what to check the host key against

As you can see from other descriptions, we normally put an "#optional"
in front of descriptions of optional parameters, and it's also a good
idea to specify the default behavior or value, which in this case is
"yes" - intuitively I'd have expected "no", so you should probably
indeed make a note of that.

Max

> +#
> +# Since 2.8
> +##
> +{ 'struct': 'BlockdevoptionsSsh',
> +  'data': { 'server': 'InetSocketAddress',
> +            'path': 'str',
> +            'user': 'str',
> +            '*host_key_check': 'str' } }
> +
>  
>  ##
>  # @BlkdebugEvent
> @@ -2281,7 +2301,7 @@
>  # TODO rbd: Wait for structured options
>        'replication':'BlockdevOptionsReplication',
>  # TODO sheepdog: Wait for structured options
> -# TODO ssh: Should take InetSocketAddress for 'host'?
> +      'ssh':        'BlockdevoptionsSsh',
>        'tftp':       'BlockdevOptionsCurl',
>        'vdi':        'BlockdevOptionsGenericFormat',
>        'vhdx':       'BlockdevOptionsGenericFormat',
>

signature.asc
Description: OpenPGP digital signature