[Qemu-devel] [PATCH 10/11] block: Allow omitting the file name when using driver-specific options

[Kevin Wolf]

After this patch, using -drive with an empty file name continues to open
the file if driver-specific options are used. If no driver-specific
options are specified, the semantics stay as it was: It defines a drive
without an inserted medium.

In order to achieve this, bdrv_open() must be made safe to work with a
NULL filename parameter. The assumption that is made is that only block
drivers which implement bdrv_parse_filename() support using driver
specific options and could therefore work without a filename. These
drivers must make sure to cope with NULL in their implementation of
.bdrv_open() (this is only NBD for now). For all other drivers, the
block layer code will make sure to error out before calling into their
code - they can't possibly work without a filename.

Now an NBD connection can be opened like this:

  qemu-system-x86_64 -drive file.driver=nbd,file.port=1234,file.host=::1

Signed-off-by: Kevin Wolf <address@hidden>
---
 block.c                   | 49 +++++++++++++++++++++++++++++++++++++++--------
 block/nbd.c               |  1 -
 blockdev.c                | 10 +++++++---
 include/block/block_int.h |  3 +++
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/block.c b/block.c
index abe6bd5..ae46ca2 100644
--- a/block.c
+++ b/block.c
@@ -688,7 +688,11 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
         bdrv_enable_copy_on_read(bs);
     }
 
-    pstrcpy(bs->filename, sizeof(bs->filename), filename);
+    if (filename != NULL) {
+        pstrcpy(bs->filename, sizeof(bs->filename), filename);
+    } else {
+        bs->filename[0] = '\0';
+    }
 
     if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
         return -ENOTSUP;
@@ -708,6 +712,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
             bdrv_swap(file, bs);
             ret = 0;
         } else {
+            assert(drv->bdrv_parse_filename || filename != NULL);
             ret = drv->bdrv_file_open(bs, filename, options, open_flags);
         }
     } else {
@@ -727,6 +732,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 
 #ifndef _WIN32
     if (bs->is_temporary) {
+        assert(filename != NULL);
         unlink(filename);
     }
 #endif
@@ -753,14 +759,9 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
 {
     BlockDriverState *bs;
     BlockDriver *drv;
+    const char *drvname;
     int ret;
 
-    drv = bdrv_find_protocol(filename);
-    if (!drv) {
-        QDECREF(options);
-        return -ENOENT;
-    }
-
     /* NULL means an empty set of options */
     if (options == NULL) {
         options = qdict_new();
@@ -770,7 +771,26 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
     bs->options = options;
     options = qdict_clone_shallow(options);
 
-    if (drv->bdrv_parse_filename) {
+    /* Find the right block driver */
+    drvname = qdict_get_try_str(options, "driver");
+    if (drvname) {
+        drv = bdrv_find_whitelisted_format(drvname);
+        qdict_del(options, "driver");
+    } else if (filename) {
+        drv = bdrv_find_protocol(filename);
+    } else {
+        qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                      "Must specify either driver or file");
+        drv = NULL;
+    }
+
+    if (!drv) {
+        ret = -ENOENT;
+        goto fail;
+    }
+
+    /* Parse the filename and open it */
+    if (drv->bdrv_parse_filename && filename) {
         Error *local_err = NULL;
         drv->bdrv_parse_filename(filename, options, &local_err);
         if (error_is_set(&local_err)) {
@@ -779,6 +799,12 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
             ret = -EINVAL;
             goto fail;
         }
+    } else if (!drv->bdrv_parse_filename && !filename) {
+        qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                      "The '%s' block driver requires a file name",
+                      drv->format_name);
+        ret = -EINVAL;
+        goto fail;
     }
 
     ret = bdrv_open_common(bs, NULL, filename, options, flags, drv);
@@ -899,6 +925,13 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
QDict *options,
         QEMUOptionParameter *create_options;
         char backing_filename[PATH_MAX];
 
+        if (qdict_size(options) != 0) {
+            error_report("Can't use snapshot=on with driver-specific options");
+            ret = -EINVAL;
+            goto fail;
+        }
+        assert(filename != NULL);
+
         /* if snapshot, we create a temporary backing file and open it
            instead of opening 'filename' directly */
 
diff --git a/block/nbd.c b/block/nbd.c
index 9858f06..218df6a 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -174,7 +174,6 @@ static void nbd_parse_filename(const char *filename, QDict 
*options,
 
     /* extract the host_spec - fail if it's not nbd:... */
     if (!strstart(file, "nbd:", &host_spec)) {
-        error_setg(errp, "File name string for NBD must start with 'nbd:'");
         goto out;
     }
 
diff --git a/blockdev.c b/blockdev.c
index 09f76b7..a0b2fb4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -658,7 +658,11 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
BlockInterfaceType block_default_type)
         abort();
     }
     if (!file || !*file) {
-        return dinfo;
+        if (qdict_size(bs_opts)) {
+            file = NULL;
+        } else {
+            return dinfo;
+        }
     }
     if (snapshot) {
         /* always use cache=unsafe with snapshot */
@@ -697,10 +701,10 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
BlockInterfaceType block_default_type)
     if (ret < 0) {
         if (ret == -EMEDIUMTYPE) {
             error_report("could not open disk image %s: not in %s format",
-                         file, drv->format_name);
+                         file ?: dinfo->id, drv->format_name);
         } else {
             error_report("could not open disk image %s: %s",
-                         file, strerror(-ret));
+                         file ?: dinfo->id, strerror(-ret));
         }
         goto err;
     }
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1b06a11..0986a2d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -75,6 +75,9 @@ struct BlockDriver {
     int instance_size;
     int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
     int (*bdrv_probe_device)(const char *filename);
+
+    /* Any driver implementing this callback is expected to be able to handle
+     * NULL file names in its .bdrv_open() implementation */
     void (*bdrv_parse_filename)(const char *filename, QDict *options, Error 
**errp);
 
     /* For handling image reopen for split or non-split files */
-- 
1.8.1.4