Convert the warning about dangerous automatic probing of raw images to
warn_report().  Split its text to conform to conventions spelled out
in warn_report()'s contract.

Update expected output of qemu-iotest 109 accordingly.  Update
qemu-iotest 099's output filtering to keep filtering out the warning.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/raw-format.c         | 17 ++++++------
 tests/qemu-iotests/099     |  3 ++-
 tests/qemu-iotests/109.out | 55 +++++++++++++++++++++++---------------
 3 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/block/raw-format.c b/block/raw-format.c
index 6f6dc99b2c..d65fd0ffce 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -29,6 +29,7 @@
 #include "qemu/osdep.h"
 #include "block/block_int.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "qemu/option.h"
 
 typedef struct BDRVRawState {
@@ -436,14 +437,14 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
             bs->file->bs->supported_zero_flags);
 
     if (bs->probed && !bdrv_is_read_only(bs)) {
-        fprintf(stderr,
-                "WARNING: Image format was not specified for '%s' and probing "
-                "guessed raw.\n"
-                "         Automatically detecting the format is dangerous for "
-                "raw images, write operations on block 0 will be restricted.\n"
-                "         Specify the 'raw' format explicitly to remove the "
-                "restrictions.\n",
-                bs->file->bs->filename);
+        warn_report("Image format was not specified for '%s' and probing "
+                    "guessed raw",
+                    bs->file->bs->filename);
+        error_printf("Automatically detecting the format is dangerous for "
+                     "raw images, write\n"
+                     "operations on block 0 will be restricted.\n"
+                     "Specify the 'raw' format explicitly to remove the "
+                     "restrictions.\n");
     }
 
     ret = raw_read_options(options, bs, s, errp);
diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099
index caaf58eee5..f2a62448d8 100755
--- a/tests/qemu-iotests/099
+++ b/tests/qemu-iotests/099
@@ -57,7 +57,8 @@ function run_qemu()
     # which is how we can extract it)
     do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_imgfmt | _filter_qmp \
         | _filter_qemu | grep "drv0" \
-        | sed -e 's/^.*"file": "\(\(\\"\|[^"]\)*\)".*$/\1/' -e 's/\\"/"/g'
+        | sed -e 's/^.*"file": "\(\(\\"\|[^"]\)*\)".*$/\1/' -e 's/\\"/"/g' \
+	      -e '/probing guessed raw/d'
 }
 
 function test_qemu()
diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
index ad0ee6fb48..99add53271 100644
--- a/tests/qemu-iotests/109.out
+++ b/tests/qemu-iotests/109.out
@@ -5,8 +5,9 @@ QA output created by 109
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -43,8 +44,9 @@ Images are identical.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -81,8 +83,9 @@ Images are identical.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -119,8 +122,9 @@ Images are identical.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -157,8 +161,9 @@ Images are identical.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -195,8 +200,9 @@ Images are identical.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -232,8 +238,9 @@ Images are identical.
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -269,8 +276,9 @@ Images are identical.
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -306,8 +314,9 @@ Images are identical.
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -343,8 +352,9 @@ Images are identical.
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -380,8 +390,9 @@ Images are identical.
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 {"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
-         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
          Specify the 'raw' format explicitly to remove the restrictions.
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
-- 
2.17.1

Am 17.10.2018 um 10:27 hat Markus Armbruster geschrieben:
> Convert the warning about dangerous automatic probing of raw images to
> warn_report().  Split its text to conform to conventions spelled out
> in warn_report()'s contract.

One of the things the warn_report() documentation says is:

    It's wrong to call this in a QMP monitor.  Use error_setg() there.

Probably a bug in the comment (copy and paste from error_report())? The
same sentence is also there for info_report().

> Update expected output of qemu-iotest 109 accordingly.  Update
> qemu-iotest 099's output filtering to keep filtering out the warning.
> 
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

> --- a/tests/qemu-iotests/109.out
> +++ b/tests/qemu-iotests/109.out
> @@ -5,8 +5,9 @@ QA output created by 109
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>  Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
>  {"return": {}}
> -WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
> -         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
> +warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
> +Automatically detecting the format is dangerous for raw images, write
> +operations on block 0 will be restricted.
>           Specify the 'raw' format explicitly to remove the restrictions.

I honestly like the old formatting better, with indentation and without
the arbitrary \n in the error_printf() line that is probably shorter
than the warning line anyway.

Also, I know that qemu-iotests ignores whitespace for diffing the
result, but not adjusting the last line in the reference output still
looks a bit odd.

Kevin

Kevin Wolf <kwolf@redhat.com> writes:

> Am 17.10.2018 um 10:27 hat Markus Armbruster geschrieben:
>> Convert the warning about dangerous automatic probing of raw images to
>> warn_report().  Split its text to conform to conventions spelled out
>> in warn_report()'s contract.
>
> One of the things the warn_report() documentation says is:
>
>     It's wrong to call this in a QMP monitor.  Use error_setg() there.
>
> Probably a bug in the comment (copy and paste from error_report())? The
> same sentence is also there for info_report().

We could debate whether use of warn_report() in a QMP monitor is a good
idea, but the comment is definitely wrong: error_setg() can't do
warnings.  I'll fix this.

>> Update expected output of qemu-iotest 109 accordingly.  Update
>> qemu-iotest 099's output filtering to keep filtering out the warning.
>> 
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Max Reitz <mreitz@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
>> --- a/tests/qemu-iotests/109.out
>> +++ b/tests/qemu-iotests/109.out
>> @@ -5,8 +5,9 @@ QA output created by 109
>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>>  Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
>>  {"return": {}}
>> -WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw.
>> -         Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
>> +warning: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed raw
>> +Automatically detecting the format is dangerous for raw images, write
>> +operations on block 0 will be restricted.
>>           Specify the 'raw' format explicitly to remove the restrictions.
>
> I honestly like the old formatting better, with indentation and without
> the arbitrary \n in the error_printf() line that is probably shorter
> than the warning line anyway.

We don't indent an informational message following an error or a warning
elsewhere.  I'm willing to keep the indentation anyway.  You tell me.

I dislike text wider than some 70 characters.  I'm willing to keep it
that way anyway.  You tell me.

> Also, I know that qemu-iotests ignores whitespace for diffing the
> result, but not adjusting the last line in the reference output still
> looks a bit odd.

That's an accident.

Thanks!

I'm dropping this patch for now.

Nice formatting of hints is good.  Consistency is good.  The way to get
both is probably a more structured interface to print hints after
FOO_report().  But this goes beyond what this series tries to do.