On 2017-11-14 16:17, Markus Armbruster wrote:
> Max Reitz <mre...@redhat.com> writes:
>
>> Instead of converting all "backing": null instances into "backing": "",
>> handle a null value directly in bdrv_open_inherit().
>>
>> This enables explicitly null backing links for json:{} filenames.
>>
>> Signed-off-by: Max Reitz <mre...@redhat.com>
>> ---
>> block.c | 2 +-
>> blockdev.c | 14 --------------
>> tests/qemu-iotests/089 | 20 ++++++++++++++++++++
>> tests/qemu-iotests/089.out | 8 ++++++++
>> 4 files changed, 29 insertions(+), 15 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 85427df577..bc92ddd5a0 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2558,7 +2558,7 @@ static BlockDriverState *bdrv_open_inherit(const char
>> *filename,
>>
>> /* See cautionary note on accessing @options above */
>> backing = qdict_get_try_str(options, "backing");
>> - if (backing && *backing == '\0') {
>> + if (qdict_is_qnull(options, "backing") || (backing && *backing ==
>> '\0')) {
>> flags |= BDRV_O_NO_BACKING;
>> qdict_del(options, "backing");
>> }
>
> Consider a comment pointing out "" is deprecated.
Would not really be necessary after patch 3, but let's see how well
patch 3 is received. :-)
(And in any case, a comment certainly won't hurt.)
Max
> See also my reply to PATCH 1/3.
>
>> diff --git a/blockdev.c b/blockdev.c
>> index 56a6b24a0b..99ef618b39 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -3885,7 +3885,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error
>> **errp)
>> QObject *obj;
>> Visitor *v = qobject_output_visitor_new(&obj);
>> QDict *qdict;
>> - const QDictEntry *ent;
>> Error *local_err = NULL;
>>
>> visit_type_BlockdevOptions(v, NULL, &options, &local_err);
>> @@ -3899,19 +3898,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error
>> **errp)
>>
>> qdict_flatten(qdict);
>>
>> - /*
>> - * Rewrite "backing": null to "backing": ""
>> - * TODO Rewrite "" to null instead, and perhaps not even here
>> - */
>> - for (ent = qdict_first(qdict); ent; ent = qdict_next(qdict, ent)) {
>> - char *dot = strrchr(ent->key, '.');
>> -
>> - if (!strcmp(dot ? dot + 1 : ent->key, "backing")
>> - && qobject_type(ent->value) == QTYPE_QNULL) {
>> - qdict_put(qdict, ent->key, qstring_new());
>> - }
>> - }
>> -
>> if (!qdict_get_try_str(qdict, "node-name")) {
>> error_setg(errp, "'node-name' must be specified for the root node");
>> goto fail;
>> diff --git a/tests/qemu-iotests/089 b/tests/qemu-iotests/089
>> index 9bfe2307b3..832eac1248 100755
>> --- a/tests/qemu-iotests/089
>> +++ b/tests/qemu-iotests/089
>> @@ -82,6 +82,26 @@ $QEMU_IO_PROG --cache $CACHEMODE \
>> $QEMU_IO -c 'read -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
>>
>>
>> +echo
>> +echo "=== Testing correct handling of 'backing':null ==="
>> +echo
>> +
>> +_make_test_img -b "$TEST_IMG.base" $IMG_SIZE
>> +
>> +# This should read 42
>> +$QEMU_IO -c 'read -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
>> +
>> +# This should read 0
>> +$QEMU_IO -c 'read -P 0 0 512' "json:{\
>> + 'driver': '$IMGFMT',
>> + 'file': {
>> + 'driver': 'file',
>> + 'filename': '$TEST_IMG'
>> + },
>> + 'backing': null
>> +}" | _filter_qemu_io
>> +
>> +
>> # Taken from test 071
>> echo
>> echo "=== Testing blkdebug ==="
>> diff --git a/tests/qemu-iotests/089.out b/tests/qemu-iotests/089.out
>> index 18f5fdda7a..607a9c6d9c 100644
>> --- a/tests/qemu-iotests/089.out
>> +++ b/tests/qemu-iotests/089.out
>> @@ -19,6 +19,14 @@ Pattern verification failed at offset 0, 512 bytes
>> read 512/512 bytes at offset 0
>> 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>
>> +=== Testing correct handling of 'backing':null ===
>> +
>> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>> backing_file=TEST_DIR/t.IMGFMT.base
>> +read 512/512 bytes at offset 0
>> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +read 512/512 bytes at offset 0
>> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +
>> === Testing blkdebug ===
>>
>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
>
signature.asc
Description: OpenPGP digital signature
