diff mbox

[4/7] block: Add options QDict to bdrv_open_common()

Message ID 1362168821-25668-5-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf March 1, 2013, 8:13 p.m. UTC 
The options are passed down to the block drivers, which are supposed to
remove all options they have processed. Anything that is left over in
the end is an unknown option and results in an error.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

Comments

Eric Blake March 1, 2013, 10:01 p.m. UTC | #1 
On 03/01/2013 01:13 PM, Kevin Wolf wrote:
> The options are passed down to the block drivers, which are supposed to
> remove all options they have processed. Anything that is left over in
> the end is an unknown option and results in an error.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>
Stefan Hajnoczi March 4, 2013, 9:28 a.m. UTC | #2 
On Fri, Mar 01, 2013 at 09:13:38PM +0100, Kevin Wolf wrote:
> diff --git a/block.c b/block.c
> index bf93dd1..e0e3b99 100644
> --- a/block.c
> +++ b/block.c
> @@ -665,15 +665,18 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
>  
>  /*
>   * Common part for opening disk images and files
> + *
> + * Removes all processed options from *options.
>   */
>  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
> -    const char *filename,
> +    const char *filename, QDict *options,

The function already has an "options" local variable which will shadow:

    /* For snapshot=on, create a temporary qcow2 overlay */
    if (flags & BDRV_O_SNAPSHOT) {
        BlockDriverState *bs1;
        int64_t total_size;
        int is_protocol = 0;
        BlockDriver *bdrv_qcow2;
        QEMUOptionParameter *options;

This could be fixed in a follow-up patch.

Stefan
diff mbox

Patch

diff --git a/block.c b/block.c
index bf93dd1..e0e3b99 100644
--- a/block.c
+++ b/block.c
@@ -665,15 +665,18 @@  static int bdrv_open_flags(BlockDriverState *bs, int flags)
 
 /*
  * Common part for opening disk images and files
+ *
+ * Removes all processed options from *options.
  */
 static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
-    const char *filename,
+    const char *filename, QDict *options,
     int flags, BlockDriver *drv)
 {
     int ret, open_flags;
 
     assert(drv != NULL);
     assert(bs->file == NULL);
+    assert(options == NULL || bs->options != options);
 
     trace_bdrv_open_common(bs, filename, flags, drv->format_name);
 
@@ -710,7 +713,7 @@  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     } else {
         assert(file != NULL);
         bs->file = file;
-        ret = drv->bdrv_open(bs, NULL, open_flags);
+        ret = drv->bdrv_open(bs, options, open_flags);
     }
 
     if (ret < 0) {
@@ -752,7 +755,7 @@  int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags)
     }
 
     bs = bdrv_new("");
-    ret = bdrv_open_common(bs, NULL, filename, flags, drv);
+    ret = bdrv_open_common(bs, NULL, filename, NULL, flags, drv);
     if (ret < 0) {
         bdrv_delete(bs);
         return ret;
@@ -821,6 +824,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     }
 
     bs->options = options;
+    options = qdict_clone_shallow(options);
 
     /* For snapshot=on, create a temporary qcow2 overlay */
     if (flags & BDRV_O_SNAPSHOT) {
@@ -903,7 +907,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     }
 
     /* Open the image */
-    ret = bdrv_open_common(bs, file, filename, flags, drv);
+    ret = bdrv_open_common(bs, file, filename, options, flags, drv);
     if (ret < 0) {
         goto unlink_and_fail;
     }
@@ -917,11 +921,22 @@  int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     if ((flags & BDRV_O_NO_BACKING) == 0) {
         ret = bdrv_open_backing_file(bs);
         if (ret < 0) {
-            bdrv_close(bs);
-            return ret;
+            goto close_and_fail;
         }
     }
 
+    /* Check if any unknown options were used */
+    if (qdict_size(options) != 0) {
+        const QDictEntry *entry = qdict_first(options);
+        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by "
+            "device '%s' doesn't support the option '%s'",
+            drv->format_name, bs->device_name, entry->key);
+
+        ret = -EINVAL;
+        goto close_and_fail;
+    }
+    QDECREF(options);
+
     if (!bdrv_key_required(bs)) {
         bdrv_dev_change_media_cb(bs, true);
     }
@@ -942,8 +957,13 @@  unlink_and_fail:
     }
 fail:
     QDECREF(bs->options);
+    QDECREF(options);
     bs->options = NULL;
+    return ret;
 
+close_and_fail:
+    bdrv_close(bs);
+    QDECREF(options);
     return ret;
 }