diff mbox

[for-2.9,1/2] qapi/curl: Extend and fix blockdev-add schema

Message ID 20170331120431.1767-2-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz March 31, 2017, 12:04 p.m. UTC
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(-)

Comments

Jeff Cody March 31, 2017, 6:57 p.m. UTC | #1
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>

Reviewed-by: Jeff Cody <jcody@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
> 
>
Jeff Cody March 31, 2017, 7:27 p.m. UTC | #2
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
Eric Blake March 31, 2017, 7:27 p.m. UTC | #3
On 03/31/2017 07:04 AM, 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)
> +#

Matches runtime_opts of block/curl.c, modulo the fields that you
restricted to specific protocols by creating subtypes. Looks like you
covered everything correctly.

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

Patch

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',