Re: [Qemu-devel] [Qemu-block] [PATCH for-2.9 1/2] qapi/curl: Extend and fix blockdev-add schema

[Jeff Cody <jcody@redhat.com>]

On Fri, Mar 31, 2017 at 02:04:30PM +0200, Max Reitz wrote:
> The curl block driver accepts more options than just "filename"; also,
> the URL is actually expected to be passed through the "url" option
> instead of "filename".
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qapi/block-core.json | 103 ++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 94 insertions(+), 9 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index b5f0e9958c..033457ce86 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2737,16 +2737,101 @@
>              '*debug': 'int' } }
>  
>  ##
> -# @BlockdevOptionsCurl:
> +# @BlockdevOptionsCurlBase:
>  #
> -# Driver specific block device options for the curl backend.
> +# Driver specific block device options shared by all protocols supported by the
> +# curl backend.
>  #
> -# @filename:    path to the image file
> +# @url:                     URL of the image file
> +#
> +# @readahead:               Size of the read-ahead cache; must be a multiple of
> +#                           512 (defaults to 256 kB)
> +#
> +# @timeout:                 Timeout for connections, in seconds (defaults to 5)
> +#
> +# @username:                Username for authentication (defaults to none)
> +#
> +# @password-secret:         ID of a QCryptoSecret object providing a password
> +#                           for authentication (defaults to no password)
> +#
> +# @proxy-username:          Username for proxy authentication (defaults to none)
> +#
> +# @proxy-password-secret:   ID of a QCryptoSecret object providing a password
> +#                           for proxy authentication (defaults to no password)
> +#
> +# Since: 2.9
> +##
> +{ 'struct': 'BlockdevOptionsCurlBase',
> +  'data': { 'url': 'str',
> +            '*readahead': 'int',
> +            '*timeout': 'int',
> +            '*username': 'str',
> +            '*password-secret': 'str',
> +            '*proxy-username': 'str',
> +            '*proxy-password-secret': 'str' } }
> +
> +##
> +# @BlockdevOptionsCurlHttp:
> +#
> +# Driver specific block device options for HTTP connections over the curl
> +# backend.  URLs must start with "http://".
> +#
> +# @cookie:      List of cookies to set; format is
> +#               "name1=content1; name2=content2;" as explained by
> +#               CURLOPT_COOKIE(3). Defaults to no cookies.
> +#
> +# Since: 2.9
> +##
> +{ 'struct': 'BlockdevOptionsCurlHttp',
> +  'base': 'BlockdevOptionsCurlBase',
> +  'data': { '*cookie': 'str' } }
> +
> +##
> +# @BlockdevOptionsCurlHttps:
> +#
> +# Driver specific block device options for HTTPS connections over the curl
> +# backend.  URLs must start with "https://".
> +#
> +# @cookie:      List of cookies to set; format is
> +#               "name1=content1; name2=content2;" as explained by
> +#               CURLOPT_COOKIE(3). Defaults to no cookies.
> +#
> +# @sslverify:   Whether to verify the SSL certificate's validity (defaults to
> +#               true)
> +#
> +# Since: 2.9
> +##
> +{ 'struct': 'BlockdevOptionsCurlHttps',
> +  'base': 'BlockdevOptionsCurlBase',
> +  'data': { '*cookie': 'str',
> +            '*sslverify': 'bool' } }
> +
> +##
> +# @BlockdevOptionsCurlFtp:
> +#
> +# Driver specific block device options for FTP connections over the curl
> +# backend.  URLs must start with "ftp://".
> +#
> +# Since: 2.9
> +##
> +{ 'struct': 'BlockdevOptionsCurlFtp',
> +  'base': 'BlockdevOptionsCurlBase',
> +  'data': { } }
> +
> +##
> +# @BlockdevOptionsCurlFtps:
> +#
> +# Driver specific block device options for FTPS connections over the curl
> +# backend.  URLs must start with "ftps://".
> +#
> +# @sslverify:   Whether to verify the SSL certificate's validity (defaults to
> +#               true)
>  #
>  # Since: 2.9
>  ##
> -{ 'struct': 'BlockdevOptionsCurl',
> -  'data': { 'filename': 'str' } }
> +{ 'struct': 'BlockdevOptionsCurlFtps',
> +  'base': 'BlockdevOptionsCurlBase',
> +  'data': { '*sslverify': 'bool' } }
>  
>  ##
>  # @BlockdevOptionsNbd:
> @@ -2815,13 +2900,13 @@
>        'cloop':      'BlockdevOptionsGenericFormat',
>        'dmg':        'BlockdevOptionsGenericFormat',
>        'file':       'BlockdevOptionsFile',
> -      'ftp':        'BlockdevOptionsCurl',
> -      'ftps':       'BlockdevOptionsCurl',
> +      'ftp':        'BlockdevOptionsCurlFtp',
> +      'ftps':       'BlockdevOptionsCurlFtps',
>        'gluster':    'BlockdevOptionsGluster',
>        'host_cdrom': 'BlockdevOptionsFile',
>        'host_device':'BlockdevOptionsFile',
> -      'http':       'BlockdevOptionsCurl',
> -      'https':      'BlockdevOptionsCurl',
> +      'http':       'BlockdevOptionsCurlHttp',
> +      'https':      'BlockdevOptionsCurlHttps',
>        'iscsi':      'BlockdevOptionsIscsi',
>        'luks':       'BlockdevOptionsLUKS',
>        'nbd':        'BlockdevOptionsNbd',
> -- 
> 2.12.1
> 
>

I already applied this to my branch, but it occured to me that it would be
good to get a r-b from someone in the libvirt team (like Eric), with respect
to the schema.  So I'll hold off on sending a pull req for this series
for a bit.

-Jeff