Re: [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init()
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 --- 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 Thank you! Max
Re: [Qemu-devel] [PATCH v3 03/14] blockdev: Use blk_new_open() in blockdev_init()
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 > --- > 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? 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). > > > === 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. 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 -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
