[Qemu-devel] [PATCH 0/2] block: Fix unset "filename" for certain drivers

[Qemu-devel] [PATCH 0/2] block: Fix unset "filename" for certain drivers

[Max Reitz]

For some protocol block drivers, the "filename" attribute in their BDSs
is unset due to bdrv_file_open() removing it from the options QDict
before bdrv_open_common() is able to copy it into the BDS. Fix this by
not removing it until until bdrv_open_common() has indeed copied it.


Max Reitz (2):
  block: Do not prematurely remove "filename"
  iotests: Add test for set "filename" for NBD

 block.c                    | 12 ++++++--
 tests/qemu-iotests/097     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/097.out | 13 +++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 95 insertions(+), 3 deletions(-)
 create mode 100755 tests/qemu-iotests/097
 create mode 100644 tests/qemu-iotests/097.out

-- 
2.0.0

[Qemu-devel] [PATCH 1/2] block: Do not prematurely remove "filename"

[Max Reitz]

If "filename" is removed from the options QDict before entering
bdrv_open_common(), it cannot be stored in the BDS. Therefore, wait
until it has been copied there and remove it from the options only
afterwards.

This fixes "filename" in the BDS being empty for block drivers which do
not need the filename because they have parsed it already (e.g. NBD).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 43abe96..1fdfa66 100644
--- a/block.c
+++ b/block.c
@@ -954,6 +954,14 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
         bs->filename[0] = '\0';
     }
 
+    if (!drv->bdrv_needs_filename) {
+        qdict_del(options, "filename");
+        /* The pointer "filename" may reference the just deleted QDict entry; in
+         * any case, it is no longer needed, so indicate that by setting it to
+         * NULL. */
+        filename = NULL;
+    }
+
     bs->drv = drv;
     bs->opaque = g_malloc0(drv->instance_size);
 
@@ -1070,9 +1078,7 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
             goto fail;
         }
 
-        if (!drv->bdrv_needs_filename) {
-            qdict_del(*options, "filename");
-        } else {
+        if (drv->bdrv_needs_filename) {
             filename = qdict_get_str(*options, "filename");
         }
     }
-- 
2.0.0

[Qemu-devel] [PATCH 2/2] iotests: Add test for set "filename" for NBD

[Max Reitz]

Add a new test for qemu-iotests which checks whether the "filename" (and
consequently the "file") attribute is set for images which are opened
over NBD.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/097     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/097.out | 13 +++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 86 insertions(+)
 create mode 100755 tests/qemu-iotests/097
 create mode 100644 tests/qemu-iotests/097.out

diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
new file mode 100755
index 0000000..c471ef2
--- /dev/null
+++ b/tests/qemu-iotests/097
@@ -0,0 +1,72 @@
+#!/bin/bash
+#
+# Test case for correct filename attribute for NBD
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto nbd
+_supported_os Linux
+
+function do_run_qemu()
+{
+    $QEMU -nographic -qmp stdio -serial none "$@"
+}
+
+function run_qemu()
+{
+    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp | sed -e "s#$TEST_IMG#TEST_IMG#g"
+}
+
+IMG_SIZE=128K
+
+echo
+echo '=== Testing NBD filename ("filename" and "file" should be set to TEST_IMG) ==='
+echo
+
+_make_test_img $IMG_SIZE
+
+run_qemu -drive file=$TEST_IMG,format=raw,if=none <<QMP
+{ 'execute': 'qmp_capabilities' }
+{ 'execute': 'query-block' }
+{ 'execute': 'quit' }
+QMP
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
new file mode 100644
index 0000000..8ee6680
--- /dev/null
+++ b/tests/qemu-iotests/097.out
@@ -0,0 +1,13 @@
+QA output created by 097
+
+=== Testing NBD filename ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 
+QMP_VERSION
+{"return": {}}
+{"return": [{"device": "none0", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 131072, "filename": "TEST_IMG", "format": "raw"}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_IMG", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 0f07440..5447660 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -99,3 +99,4 @@
 090 rw auto quick
 091 rw auto
 092 rw auto quick
+097 rw auto
-- 
2.0.0

Re: [Qemu-devel] [PATCH 1/2] block: Do not prematurely remove "filename"

[Benoît Canet]

The Wednesday 18 Jun 2014 à 00:14:09 (+0200), Max Reitz wrote :
> If "filename" is removed from the options QDict before entering
> bdrv_open_common(), it cannot be stored in the BDS. Therefore, wait
> until it has been copied there and remove it from the options only
> afterwards.
> 
> This fixes "filename" in the BDS being empty for block drivers which do
> not need the filename because they have parsed it already (e.g. NBD).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 43abe96..1fdfa66 100644
> --- a/block.c
> +++ b/block.c
> @@ -954,6 +954,14 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>          bs->filename[0] = '\0';
>      }
>  
> +    if (!drv->bdrv_needs_filename) {
> +        qdict_del(options, "filename");
> +        /* The pointer "filename" may reference the just deleted QDict entry; in
> +         * any case, it is no longer needed, so indicate that by setting it to
> +         * NULL. */
> +        filename = NULL;
> +    }
> +
>      bs->drv = drv;
>      bs->opaque = g_malloc0(drv->instance_size);
>  
> @@ -1070,9 +1078,7 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
>              goto fail;
>          }
>  
> -        if (!drv->bdrv_needs_filename) {
> -            qdict_del(*options, "filename");
> -        } else {
> +        if (drv->bdrv_needs_filename) {
>              filename = qdict_get_str(*options, "filename");
>          }
>      }
> -- 
> 2.0.0
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

Re: [Qemu-devel] [PATCH 2/2] iotests: Add test for set "filename" for NBD

[Benoît Canet]

The Wednesday 18 Jun 2014 à 00:14:10 (+0200), Max Reitz wrote :
> Add a new test for qemu-iotests which checks whether the "filename" (and
> consequently the "file") attribute is set for images which are opened
> over NBD.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/097     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/097.out | 13 +++++++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 86 insertions(+)
>  create mode 100755 tests/qemu-iotests/097
>  create mode 100644 tests/qemu-iotests/097.out
> 
> diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
> new file mode 100755
> index 0000000..c471ef2
> --- /dev/null
> +++ b/tests/qemu-iotests/097
> @@ -0,0 +1,72 @@
> +#!/bin/bash
> +#
> +# Test case for correct filename attribute for NBD
> +#
> +# Copyright (C) 2014 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +# creator
> +owner=mreitz@redhat.com
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +here="$PWD"
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +
> +_cleanup()
> +{
> +	_cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt generic
> +_supported_proto nbd
> +_supported_os Linux
> +
> +function do_run_qemu()
> +{
> +    $QEMU -nographic -qmp stdio -serial none "$@"
> +}
> +
> +function run_qemu()
> +{
> +    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp | sed -e "s#$TEST_IMG#TEST_IMG#g"
> +}
> +
> +IMG_SIZE=128K
> +
> +echo
> +echo '=== Testing NBD filename ("filename" and "file" should be set to TEST_IMG) ==='
> +echo
> +
> +_make_test_img $IMG_SIZE
> +
> +run_qemu -drive file=$TEST_IMG,format=raw,if=none <<QMP
> +{ 'execute': 'qmp_capabilities' }
> +{ 'execute': 'query-block' }
> +{ 'execute': 'quit' }
> +QMP
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
> new file mode 100644
> index 0000000..8ee6680
> --- /dev/null
> +++ b/tests/qemu-iotests/097.out
> @@ -0,0 +1,13 @@
> +QA output created by 097
> +
> +=== Testing NBD filename ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 
> +QMP_VERSION
> +{"return": {}}
> +{"return": [{"device": "none0", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 131072, "filename": "TEST_IMG", "format": "raw"}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_IMG", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}

Is the real filename really TEST_IMG ? or is it some result of the filename
being passed in $TEST_IMG ? it's confusing.

> +{"return": {}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
> +*** done
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 0f07440..5447660 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -99,3 +99,4 @@
>  090 rw auto quick
>  091 rw auto
>  092 rw auto quick
> +097 rw auto
> -- 
> 2.0.0
> 
> 

Re: [Qemu-devel] [PATCH 2/2] iotests: Add test for set "filename" for NBD

[Max Reitz]

On 18.06.2014 15:47, Benoît Canet wrote:
> The Wednesday 18 Jun 2014 à 00:14:10 (+0200), Max Reitz wrote :
>> Add a new test for qemu-iotests which checks whether the "filename" (and
>> consequently the "file") attribute is set for images which are opened
>> over NBD.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/097     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/097.out | 13 +++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   3 files changed, 86 insertions(+)
>>   create mode 100755 tests/qemu-iotests/097
>>   create mode 100644 tests/qemu-iotests/097.out
>>
>> diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
>> new file mode 100755
>> index 0000000..c471ef2
>> --- /dev/null
>> +++ b/tests/qemu-iotests/097
>> @@ -0,0 +1,72 @@
>> +#!/bin/bash
>> +#
>> +# Test case for correct filename attribute for NBD
>> +#
>> +# Copyright (C) 2014 Red Hat, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +# creator
>> +owner=mreitz@redhat.com
>> +
>> +seq="$(basename $0)"
>> +echo "QA output created by $seq"
>> +
>> +here="$PWD"
>> +tmp=/tmp/$$
>> +status=1	# failure is the default!
>> +
>> +_cleanup()
>> +{
>> +	_cleanup_test_img
>> +}
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +# get standard environment, filters and checks
>> +. ./common.rc
>> +. ./common.filter
>> +
>> +_supported_fmt generic
>> +_supported_proto nbd
>> +_supported_os Linux
>> +
>> +function do_run_qemu()
>> +{
>> +    $QEMU -nographic -qmp stdio -serial none "$@"
>> +}
>> +
>> +function run_qemu()
>> +{
>> +    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp | sed -e "s#$TEST_IMG#TEST_IMG#g"
>> +}
>> +
>> +IMG_SIZE=128K
>> +
>> +echo
>> +echo '=== Testing NBD filename ("filename" and "file" should be set to TEST_IMG) ==='
>> +echo
>> +
>> +_make_test_img $IMG_SIZE
>> +
>> +run_qemu -drive file=$TEST_IMG,format=raw,if=none <<QMP
>> +{ 'execute': 'qmp_capabilities' }
>> +{ 'execute': 'query-block' }
>> +{ 'execute': 'quit' }
>> +QMP
>> +
>> +# success, all done
>> +echo "*** done"
>> +rm -f $seq.full
>> +status=0
>> diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
>> new file mode 100644
>> index 0000000..8ee6680
>> --- /dev/null
>> +++ b/tests/qemu-iotests/097.out
>> @@ -0,0 +1,13 @@
>> +QA output created by 097
>> +
>> +=== Testing NBD filename ===
>> +
>> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
>> +QMP_VERSION
>> +{"return": {}}
>> +{"return": [{"device": "none0", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 131072, "filename": "TEST_IMG", "format": "raw"}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_IMG", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
> Is the real filename really TEST_IMG ? or is it some result of the filename
> being passed in $TEST_IMG ? it's confusing.

It's the result of '| sed -e "s#$TEST_IMG#TEST_IMG#g"' in run_qemu() 
from the test (which replaces the image filename by "TEST_IMG").

Max

>> +{"return": {}}
>> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
>> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
>> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
>> +*** done
>> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
>> index 0f07440..5447660 100644
>> --- a/tests/qemu-iotests/group
>> +++ b/tests/qemu-iotests/group
>> @@ -99,3 +99,4 @@
>>   090 rw auto quick
>>   091 rw auto
>>   092 rw auto quick
>> +097 rw auto
>> -- 
>> 2.0.0
>>
>>

Re: [Qemu-devel] [PATCH 0/2] block: Fix unset "filename" for certain drivers

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

On Wed, Jun 18, 2014 at 12:14:08AM +0200, Max Reitz wrote:
> For some protocol block drivers, the "filename" attribute in their BDSs
> is unset due to bdrv_file_open() removing it from the options QDict
> before bdrv_open_common() is able to copy it into the BDS. Fix this by
> not removing it until until bdrv_open_common() has indeed copied it.
> 
> 
> Max Reitz (2):
>   block: Do not prematurely remove "filename"
>   iotests: Add test for set "filename" for NBD
> 
>  block.c                    | 12 ++++++--
>  tests/qemu-iotests/097     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/097.out | 13 +++++++++
>  tests/qemu-iotests/group   |  1 +
>  4 files changed, 95 insertions(+), 3 deletions(-)
>  create mode 100755 tests/qemu-iotests/097
>  create mode 100644 tests/qemu-iotests/097.out

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/2] block: Fix unset "filename" for certain drivers

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1413 bytes --]

On Wed, Jun 18, 2014 at 12:14:08AM +0200, Max Reitz wrote:
> For some protocol block drivers, the "filename" attribute in their BDSs
> is unset due to bdrv_file_open() removing it from the options QDict
> before bdrv_open_common() is able to copy it into the BDS. Fix this by
> not removing it until until bdrv_open_common() has indeed copied it.
> 
> 
> Max Reitz (2):
>   block: Do not prematurely remove "filename"
>   iotests: Add test for set "filename" for NBD
> 
>  block.c                    | 12 ++++++--
>  tests/qemu-iotests/097     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/097.out | 13 +++++++++
>  tests/qemu-iotests/group   |  1 +
>  4 files changed, 95 insertions(+), 3 deletions(-)
>  create mode 100755 tests/qemu-iotests/097
>  create mode 100644 tests/qemu-iotests/097.out

Please check qemu-iotests 051.  Dropping from block queue for now.

051 5s ... - output mismatch (see 051.out.bad)
--- 051.out	2014-06-20 15:12:57.034266987 +0800
+++ 051.out.bad	2014-06-23 16:34:37.880591037 +0800
@@ -237,7 +237,8 @@
 (qemu) quit
 
 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'
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) quit

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/2] block: Fix unset "filename" for certain drivers

[Max Reitz]

On 23.06.2014 10:35, Stefan Hajnoczi wrote:
> On Wed, Jun 18, 2014 at 12:14:08AM +0200, Max Reitz wrote:
>> For some protocol block drivers, the "filename" attribute in their BDSs
>> is unset due to bdrv_file_open() removing it from the options QDict
>> before bdrv_open_common() is able to copy it into the BDS. Fix this by
>> not removing it until until bdrv_open_common() has indeed copied it.
>>
>>
>> Max Reitz (2):
>>    block: Do not prematurely remove "filename"
>>    iotests: Add test for set "filename" for NBD
>>
>>   block.c                    | 12 ++++++--
>>   tests/qemu-iotests/097     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/097.out | 13 +++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   4 files changed, 95 insertions(+), 3 deletions(-)
>>   create mode 100755 tests/qemu-iotests/097
>>   create mode 100644 tests/qemu-iotests/097.out
> Please check qemu-iotests 051.  Dropping from block queue for now.

Thanks for catching this. The problem was me always removing the 
"filename" entry from the QDict if the driver could not handle it; 
however, to maintain the current behavior, it should only be removed if 
it has been parsed before. I'll send a v2.

Max