Re: [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init()

[Max Reitz <mreitz@redhat.com>]

On 2015-01-26 at 17:37, Eric Blake wrote:
> On 01/26/2015 08:00 AM, Max Reitz wrote:
>> Due to different error propagation, this breaks tests 051 and 087; fix
>> their output.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   blockdev.c                 | 92 +++++++++++++++++++++-------------------------
>>   tests/qemu-iotests/051.out | 60 +++++++++++++++---------------
>>   tests/qemu-iotests/087.out |  8 ++--
>>   3 files changed, 76 insertions(+), 84 deletions(-)
>>   
>>   Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2
>> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: could not open disk image TEST_DIR/t.qcow2: Driver specified twice
>> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: Cannot specify both 'driver' and 'format'
> Is it possible to specify driver=qcow2,format=qcow2?  Should it be?

No, it isn't, and in my opinion it shouldn't be (just a personal 
feeling, though).

> Either way, are we testing the outcome of that?  (that is, there is a
> difference between two competing options, and two spellings of the same
> option - I could go for either rejecting the duplication, or for
> allowing it when the two are the same, whichever is easier, but would
> like to make sure it is tested so we know if we change our minds later
> whether we are risking a regression).

No, we aren't yet. Albeit not really related to this series, it is a 
good point, so I'll probably just add a test case in v4.

>>   
>>   
>>   === Specifying both an option and its legacy alias ===
>> @@ -323,13 +323,13 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,readonly=on,read-only=off: 'read-only' a
>>   === Parsing protocol from file name ===
>>   
>>   Testing: -hda foo:bar
>> -QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: Unknown protocol
>> +QEMU_PROG: -hda foo:bar: Unknown protocol
> Not the fault of this patch, but can this error message be improved?
> Even 'Unknown protocol: foo' would read better.

Indeed, I'll fix it.

> All of the other shorter error messages still seem to read fine, and the
> decrease in verbosity could be argued as a feature.  So overall, I'm
> fine with this patch.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>

Thank you!

Max