[Qemu-devel] [PATCH] block: Fail gracefully with missing filename

[Qemu-devel] [PATCH] block: Fail gracefully with missing filename

[Kevin Wolf]

This fixes a regression introduced in commit 2a05cbe42 ('block: Allow
block devices without files'):

$ qemu-system-x86_64 -drive driver=file
qemu-system-x86_64: block.c:892: bdrv_open_common: Assertion
`!drv->bdrv_needs_filename || filename != ((void *)0)' failed.

Now the respective check must be performed not only in bdrv_file_open(),
but also in bdrv_open().

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                    | 11 ++++++-----
 tests/qemu-iotests/051     | 12 ++++++++++++
 tests/qemu-iotests/051.out | 24 ++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index cb21a5f..ac0ccac 100644
--- a/block.c
+++ b/block.c
@@ -832,6 +832,12 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
         filename = qdict_get_try_str(options, "filename");
     }
 
+    if (drv->bdrv_needs_filename && !filename) {
+        error_setg(errp, "The '%s' block driver requires a file name",
+                   drv->format_name);
+        return -EINVAL;
+    }
+
     trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
 
     node_name = qdict_get_try_str(options, "node-name");
@@ -1031,11 +1037,6 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
             goto fail;
         }
         qdict_del(options, "filename");
-    } else if (drv->bdrv_needs_filename && !filename) {
-        error_setg(errp, "The '%s' block driver requires a file name",
-                   drv->format_name);
-        ret = -EINVAL;
-        goto fail;
     }
 
     if (!drv->bdrv_file_open) {
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index b23d91b..46345fb 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -172,6 +172,18 @@ run_qemu -drive file="$TEST_IMG",file.driver=file
 run_qemu -drive file="$TEST_IMG",file.driver=qcow2
 
 echo
+echo === Leaving out required options ===
+echo
+
+run_qemu -drive driver=file
+run_qemu -drive driver=nbd
+run_qemu -drive driver=raw
+run_qemu -drive file.driver=file
+run_qemu -drive file.driver=nbd
+run_qemu -drive file.driver=raw
+run_qemu -drive foo=bar
+
+echo
 echo === Parsing protocol from file name ===
 echo
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index d0c5173..30e2dbd 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -225,6 +225,30 @@ Testing: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2
 QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device '' doesn't support the option 'filename'
 
 
+=== Leaving out required options ===
+
+Testing: -drive driver=file
+QEMU_PROG: -drive driver=file: could not open disk image ide0-hd0: The 'file' block driver requires a file name
+
+Testing: -drive driver=nbd
+QEMU_PROG: -drive driver=nbd: could not open disk image ide0-hd0: Could not open image: Invalid argument
+
+Testing: -drive driver=raw
+QEMU_PROG: -drive driver=raw: could not open disk image ide0-hd0: Can't use 'raw' as a block driver for the protocol level
+
+Testing: -drive file.driver=file
+QEMU_PROG: -drive file.driver=file: could not open disk image ide0-hd0: The 'file' block driver requires a file name
+
+Testing: -drive file.driver=nbd
+QEMU_PROG: -drive file.driver=nbd: could not open disk image ide0-hd0: Could not open image: Invalid argument
+
+Testing: -drive file.driver=raw
+QEMU_PROG: -drive file.driver=raw: could not open disk image ide0-hd0: Can't use 'raw' as a block driver for the protocol level
+
+Testing: -drive foo=bar
+QEMU_PROG: -drive foo=bar: could not open disk image ide0-hd0: Must specify either driver or file
+
+
 === Parsing protocol from file name ===
 
 Testing: -hda foo:bar
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] block: Fail gracefully with missing filename

[Stefan Hajnoczi]

On Mon, Feb 03, 2014 at 02:55:31PM +0100, Kevin Wolf wrote:
> This fixes a regression introduced in commit 2a05cbe42 ('block: Allow
> block devices without files'):
> 
> $ qemu-system-x86_64 -drive driver=file
> qemu-system-x86_64: block.c:892: bdrv_open_common: Assertion
> `!drv->bdrv_needs_filename || filename != ((void *)0)' failed.
> 
> Now the respective check must be performed not only in bdrv_file_open(),
> but also in bdrv_open().
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                    | 11 ++++++-----
>  tests/qemu-iotests/051     | 12 ++++++++++++
>  tests/qemu-iotests/051.out | 24 ++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 5 deletions(-)

Ouch!  Nice to have tests that protect us in the future.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>