[Qemu-devel] [PATCH v2 1/9] block: Create bdrv_fill_options()

[Kevin Wolf <kwolf@redhat.com>]

The idea of bdrv_fill_options() is to convert every parameter for
opening images, in particular the filename and flags, to entries in the
options QDict.

This patch starts with moving the filename parsing and driver probing
part from bdrv_file_open() to the new function.

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

diff --git a/block.c b/block.c
index a3d09fa..f1c3b57 100644
--- a/block.c
+++ b/block.c
@@ -1006,77 +1006,103 @@ free_and_fail:
 }
 
 /*
- * Opens a file using a protocol (file, host_device, nbd, ...)
- *
- * options is an indirect pointer to a QDict of options to pass to the block
- * drivers, or pointer to NULL for an empty set of options. If this function
- * takes ownership of the QDict reference, it will set *options to NULL;
- * otherwise, it will contain unused/unrecognized options after this function
- * returns. Then, the caller is responsible for freeing it. If it intends to
- * reuse the QDict, QINCREF() should be called beforehand.
+ * Fills in default options for opening images and converts the legacy
+ * filename/flags pair to option QDict entries.
  */
-static int bdrv_file_open(BlockDriverState *bs, const char *filename,
-                          QDict **options, int flags, Error **errp)
+static int bdrv_fill_options(QDict **options, const char *filename,
+                             Error **errp)
 {
-    BlockDriver *drv;
     const char *drvname;
     bool parse_filename = false;
     Error *local_err = NULL;
-    int ret;
+    BlockDriver *drv;
 
     /* Fetch the file name from the options QDict if necessary */
-    if (!filename) {
-        filename = qdict_get_try_str(*options, "filename");
-    } else if (filename && !qdict_haskey(*options, "filename")) {
-        qdict_put(*options, "filename", qstring_from_str(filename));
-        parse_filename = true;
-    } else {
-        error_setg(errp, "Can't specify 'file' and 'filename' options at the "
-                   "same time");
-        ret = -EINVAL;
-        goto fail;
+    if (filename) {
+        if (!qdict_haskey(*options, "filename")) {
+            qdict_put(*options, "filename", qstring_from_str(filename));
+            parse_filename = true;
+        } else {
+            error_setg(errp, "Can't specify 'file' and 'filename' options at "
+                             "the same time");
+            return -EINVAL;
+        }
     }
 
     /* Find the right block driver */
+    filename = qdict_get_try_str(*options, "filename");
     drvname = qdict_get_try_str(*options, "driver");
-    if (drvname) {
-        drv = bdrv_find_format(drvname);
-        if (!drv) {
-            error_setg(errp, "Unknown driver '%s'", drvname);
-        }
-        qdict_del(*options, "driver");
-    } else if (filename) {
-        drv = bdrv_find_protocol(filename, parse_filename);
-        if (!drv) {
-            error_setg(errp, "Unknown protocol");
+
+    if (!drvname) {
+        if (filename) {
+            drv = bdrv_find_protocol(filename, parse_filename);
+            if (!drv) {
+                error_setg(errp, "Unknown protocol");
+                return -EINVAL;
+            }
+
+            drvname = drv->format_name;
+            qdict_put(*options, "driver", qstring_from_str(drvname));
+        } else {
+            error_setg(errp, "Must specify either driver or file");
+            return -EINVAL;
         }
-    } else {
-        error_setg(errp, "Must specify either driver or file");
-        drv = NULL;
     }
 
+    drv = bdrv_find_format(drvname);
     if (!drv) {
-        /* errp has been set already */
-        ret = -ENOENT;
-        goto fail;
+        error_setg(errp, "Unknown driver '%s'", drvname);
+        return -ENOENT;
     }
 
-    /* Parse the filename and open it */
+    /* Driver-specific filename parsing */
     if (drv->bdrv_parse_filename && parse_filename) {
         drv->bdrv_parse_filename(filename, *options, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
-            ret = -EINVAL;
-            goto fail;
+            return -EINVAL;
         }
 
         if (!drv->bdrv_needs_filename) {
             qdict_del(*options, "filename");
-        } else {
-            filename = qdict_get_str(*options, "filename");
         }
     }
 
+    return 0;
+}
+
+/*
+ * Opens a file using a protocol (file, host_device, nbd, ...)
+ *
+ * options is an indirect pointer to a QDict of options to pass to the block
+ * drivers, or pointer to NULL for an empty set of options. If this function
+ * takes ownership of the QDict reference, it will set *options to NULL;
+ * otherwise, it will contain unused/unrecognized options after this function
+ * returns. Then, the caller is responsible for freeing it. If it intends to
+ * reuse the QDict, QINCREF() should be called beforehand.
+ */
+static int bdrv_file_open(BlockDriverState *bs, const char *filename,
+                          QDict **options, int flags, Error **errp)
+{
+    BlockDriver *drv;
+    const char *drvname;
+    Error *local_err = NULL;
+    int ret;
+
+    ret = bdrv_fill_options(options, filename, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+    filename = qdict_get_try_str(*options, "filename");
+    drvname = qdict_get_str(*options, "driver");
+
+    drv = bdrv_find_format(drvname);
+    assert(drv);
+    qdict_del(*options, "driver");
+
+    /* Open the file */
     if (!drv->bdrv_file_open) {
         ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err);
         *options = NULL;
-- 
1.8.3.1