diff mbox

[1/8] qapi: Drop QERR_UNKNOWN_BLOCK_FORMAT_FEATURE

Message ID 1447127063-4662-2-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Nov. 10, 2015, 3:44 a.m. UTC 
Just specifying a custom string is simpler in basically all places that
used it, and in addition, specifying the BB or node name is something we
generally do not do in other error messages when opening a BDS, so we
should not do it here.

This changes the output for iotest 036 (to the better, in my opinion),
so the reference output needs to be changed accordingly.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow.c               |  6 +-----
 block/qcow2.c              | 24 +++++-------------------
 block/qed.c                |  7 ++-----
 block/vmdk.c               |  7 ++-----
 include/qapi/qmp/qerror.h  |  3 ---
 tests/qemu-iotests/036.out | 16 ++++++++--------
 6 files changed, 18 insertions(+), 45 deletions(-)

Comments

Eric Blake Nov. 10, 2015, 3:59 a.m. UTC | #1 
On 11/09/2015 08:44 PM, Max Reitz wrote:
> Just specifying a custom string is simpler in basically all places that
> used it, and in addition, specifying the BB or node name is something we
> generally do not do in other error messages when opening a BDS, so we
> should not do it here.
> 
> This changes the output for iotest 036 (to the better, in my opinion),

s/to/for/ to make the idiomatic expression sound correct to a native speaker

> so the reference output needs to be changed accordingly.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow.c               |  6 +-----
>  block/qcow2.c              | 24 +++++-------------------
>  block/qed.c                |  7 ++-----
>  block/vmdk.c               |  7 ++-----
>  include/qapi/qmp/qerror.h  |  3 ---
>  tests/qemu-iotests/036.out | 16 ++++++++--------
>  6 files changed, 18 insertions(+), 45 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Alberto Garcia Dec. 1, 2015, 10:17 a.m. UTC | #2 
On Tue 10 Nov 2015 04:44:16 AM CET, Max Reitz <mreitz@redhat.com> wrote:
> Just specifying a custom string is simpler in basically all places that
> used it, and in addition, specifying the BB or node name is something we
> generally do not do in other error messages when opening a BDS, so we
> should not do it here.
>
> This changes the output for iotest 036 (to the better, in my opinion),
> so the reference output needs to be changed accordingly.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto
diff mbox

Patch

diff --git a/block/qcow.c b/block/qcow.c
index 635085e..2601954 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -119,11 +119,7 @@  static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
     if (header.version != QCOW_VERSION) {
-        char version[64];
-        snprintf(version, sizeof(version), "QCOW version %" PRIu32,
-                 header.version);
-        error_setg(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-                   bdrv_get_device_or_node_name(bs), "qcow", version);
+        error_setg(errp, "Unsupported qcow version %" PRIu32, header.version);
         ret = -ENOTSUP;
         goto fail;
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index 88f56c8..88e17aa 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -196,22 +196,8 @@  static void cleanup_unknown_header_ext(BlockDriverState *bs)
     }
 }
 
-static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
-    Error **errp, const char *fmt, ...)
-{
-    char msg[64];
-    va_list ap;
-
-    va_start(ap, fmt);
-    vsnprintf(msg, sizeof(msg), fmt, ap);
-    va_end(ap);
-
-    error_setg(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-               bdrv_get_device_or_node_name(bs), "qcow2", msg);
-}
-
-static void report_unsupported_feature(BlockDriverState *bs,
-    Error **errp, Qcow2Feature *table, uint64_t mask)
+static void report_unsupported_feature(Error **errp, Qcow2Feature *table,
+                                       uint64_t mask)
 {
     char *features = g_strdup("");
     char *old;
@@ -236,7 +222,7 @@  static void report_unsupported_feature(BlockDriverState *bs,
         g_free(old);
     }
 
-    report_unsupported(bs, errp, "%s", features);
+    error_setg(errp, "Unsupported qcow2 feature(s): %s", features);
     g_free(features);
 }
 
@@ -853,7 +839,7 @@  static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
     if (header.version < 2 || header.version > 3) {
-        report_unsupported(bs, errp, "QCOW version %" PRIu32, header.version);
+        error_setg(errp, "Unsupported qcow2 version %" PRIu32, header.version);
         ret = -ENOTSUP;
         goto fail;
     }
@@ -933,7 +919,7 @@  static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         void *feature_table = NULL;
         qcow2_read_extensions(bs, header.header_length, ext_end,
                               &feature_table, NULL);
-        report_unsupported_feature(bs, errp, feature_table,
+        report_unsupported_feature(errp, feature_table,
                                    s->incompatible_features &
                                    ~QCOW2_INCOMPAT_MASK);
         ret = -ENOTSUP;
diff --git a/block/qed.c b/block/qed.c
index 5ea05d4..c15f0be 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -398,11 +398,8 @@  static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
     }
     if (s->header.features & ~QED_FEATURE_MASK) {
         /* image uses unsupported feature bits */
-        char buf[64];
-        snprintf(buf, sizeof(buf), "%" PRIx64,
-            s->header.features & ~QED_FEATURE_MASK);
-        error_setg(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-                   bdrv_get_device_or_node_name(bs), "QED", buf);
+        error_setg(errp, "Unsupported QED features: %" PRIx64,
+                   s->header.features & ~QED_FEATURE_MASK);
         return -ENOTSUP;
     }
     if (!qed_is_cluster_size_valid(s->header.cluster_size)) {
diff --git a/block/vmdk.c b/block/vmdk.c
index 6f819e4..bcce9a7 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -645,11 +645,8 @@  static int vmdk_open_vmdk4(BlockDriverState *bs,
     }
 
     if (le32_to_cpu(header.version) > 3) {
-        char buf[64];
-        snprintf(buf, sizeof(buf), "VMDK version %" PRId32,
-                 le32_to_cpu(header.version));
-        error_setg(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-                   bdrv_get_device_or_node_name(bs), "vmdk", buf);
+        error_setg(errp, "Unsupported VMDK version %" PRIu32,
+                   le32_to_cpu(header.version));
         return -ENOTSUP;
     } else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) {
         /* VMware KB 2064959 explains that version 3 added support for
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index f601499..d08652a 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -100,9 +100,6 @@ 
 #define QERR_UNDEFINED_ERROR \
     "An undefined error has occurred"
 
-#define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
-    "'%s' uses a %s feature which is not supported by this qemu version: %s"
-
 #define QERR_UNSUPPORTED \
     "this feature or command is not currently supported"
 
diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
index 5616e37..73856f5 100644
--- a/tests/qemu-iotests/036.out
+++ b/tests/qemu-iotests/036.out
@@ -22,18 +22,18 @@  autoclear_features        0x0
 refcount_order            4
 header_length             104
 
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Unknown incompatible feature: 8000000000000000
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Test feature
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT feature(s): Unknown incompatible feature: 8000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT feature(s): Test feature
 
 === Image with multiple incompatible feature bits ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Unknown incompatible feature: e000000000000000
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Test feature, Unknown incompatible feature: 6000000000000000
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Test feature, Unknown incompatible feature: c000000000000000
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: test1, test2, Unknown incompatible feature: 8000000000000000
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: test1, test2, test3
-qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: test2, Unknown incompatible feature: a000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT feature(s): Unknown incompatible feature: e000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT feature(s): Test feature, Unknown incompatible feature: 6000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT feature(s): Test feature, Unknown incompatible feature: c000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT feature(s): test1, test2, Unknown incompatible feature: 8000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT feature(s): test1, test2, test3
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported IMGFMT feature(s): test2, Unknown incompatible feature: a000000000000000
 === Create image with unknown autoclear feature bit ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864