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

[Max Reitz]

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 <address@hidden>
> > ---
> >   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 <address@hidden>

Thank you!

Max