[Qemu-devel] [PATCH 00/15] Clean up use of error_printf()

[Qemu-devel] [PATCH 00/15] Clean up use of error_printf()

[Markus Armbruster]

This series cleans up two kinds of error_printf() misuse:

* Errors and warnings should be reported with error_report() and
  warn_report().

* Help output should be go to stdout, not stderr.

This is obviously for 4.1.  If nobody objects, I'll take the whole
series through my tree.

Markus Armbruster (15):
  qemu-img: Use error_vreport() in error_exit()
  block/ssh: Do not report read/write/flush errors to the user
  char-pty: Drop "char device redirected to" message
  loader-fit: Wean off error_printf()
  mips/boston: Report errors with error_report(), not error_printf()
  pci: Report fatal errors with error_report(), not error_printf()
  hpet: Report warnings with warn_report(), not error_printf()
  vfio: Report warnings with warn_report(), not error_printf()
  s390x/kvm: Report warnings with warn_report(), not error_printf()
  vl: Make -machine $TYPE,help and -accel help print to stdout
  monitor error: Make printf()-like functions return a value
  qemu-print: New qemu_printf(), qemu_vprintf() etc.
  blockdev: Make -drive format=help print to stdout
  char: Make -chardev help print to stdout
  monitor: Simplify how -device/device_add print help

 MAINTAINERS                 |  2 ++
 block/ssh.c                 | 36 +++++++------------
 block/trace-events          |  3 ++
 blockdev.c                  |  9 ++---
 chardev/char-pty.c          |  2 --
 chardev/char.c              |  3 +-
 hw/core/loader-fit.c        | 62 +++++++++++++++++++--------------
 hw/mips/boston.c            |  6 ++--
 hw/pci/pci.c                |  2 +-
 hw/timer/hpet.c             |  2 +-
 hw/vfio/pci.c               | 19 ++++++----
 include/monitor/monitor.h   |  7 ++--
 include/qemu/error-report.h |  8 ++---
 include/qemu/qemu-print.h   | 19 ++++++++++
 monitor.c                   | 69 ++++++++++++++++++-------------------
 qdev-monitor.c              | 36 ++++++++-----------
 qemu-img.c                  |  6 ++--
 stubs/error-printf.c        | 10 +++---
 stubs/monitor.c             |  5 +++
 target/s390x/kvm.c          |  2 +-
 util/Makefile.objs          |  1 +
 util/qemu-error.c           | 12 ++++---
 util/qemu-print.c           | 42 ++++++++++++++++++++++
 vl.c                        | 10 +++---
 24 files changed, 219 insertions(+), 154 deletions(-)
 create mode 100644 include/qemu/qemu-print.h
 create mode 100644 util/qemu-print.c

-- 
2.17.2

[Qemu-devel] [PATCH 01/15] qemu-img: Use error_vreport() in error_exit()

[Markus Armbruster]

error_exit() uses low-level error_printf() to report errors.
Modernize it to use error_vreport().

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qemu-img.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index aa6f81f1ea..55201fb913 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -85,13 +85,11 @@ static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
 {
     va_list ap;
 
-    error_printf("qemu-img: ");
-
     va_start(ap, fmt);
-    error_vprintf(fmt, ap);
+    error_vreport(fmt, ap);
     va_end(ap);
 
-    error_printf("\nTry 'qemu-img --help' for more information\n");
+    error_printf("Try 'qemu-img --help' for more information\n");
     exit(EXIT_FAILURE);
 }
 
-- 
2.17.2

[Qemu-devel] [PATCH 01/15] qemu-img: Use error_vreport() in error_exit()

[Markus Armbruster]

error_exit() uses low-level error_printf() to report errors.
Modernize it to use error_vreport().

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qemu-img.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index aa6f81f1ea..55201fb913 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -85,13 +85,11 @@ static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
 {
     va_list ap;
 
-    error_printf("qemu-img: ");
-
     va_start(ap, fmt);
-    error_vprintf(fmt, ap);
+    error_vreport(fmt, ap);
     va_end(ap);
 
-    error_printf("\nTry 'qemu-img --help' for more information\n");
+    error_printf("Try 'qemu-img --help' for more information\n");
     exit(EXIT_FAILURE);
 }
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH 01/15] qemu-img: Use error_vreport() in error_exit()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

On 4/8/19 3:36 AM, Markus Armbruster wrote:
> error_exit() uses low-level error_printf() to report errors.
> Modernize it to use error_vreport().
> 
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: qemu-block@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qemu-img.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/qemu-img.c b/qemu-img.c
> index aa6f81f1ea..55201fb913 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -85,13 +85,11 @@ static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) error_exit(const char *fmt, ...)
>  {
>      va_list ap;
>  
> -    error_printf("qemu-img: ");
> -
>      va_start(ap, fmt);
> -    error_vprintf(fmt, ap);
> +    error_vreport(fmt, ap);
>      va_end(ap);
>  
> -    error_printf("\nTry 'qemu-img --help' for more information\n");
> +    error_printf("Try 'qemu-img --help' for more information\n");
>      exit(EXIT_FAILURE);
>  }
>  
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user

[Markus Armbruster]

Callbacks ssh_co_readv(), ssh_co_writev(), ssh_co_flush() report
errors to the user with error_printf().  They shouldn't, it's their
caller's job.  Replace by a suitable trace point.

Perhaps we should convert this part of the block driver interface to
Error, so block drivers can pass more detail to their callers.  Not
today.

Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/ssh.c        | 36 ++++++++++++------------------------
 block/trace-events |  3 +++
 2 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 190ef95300..382fc04fbf 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -159,31 +159,19 @@ sftp_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...)
     g_free(msg);
 }
 
-static void GCC_FMT_ATTR(2, 3)
-sftp_error_report(BDRVSSHState *s, const char *fs, ...)
+static void sftp_error_trace(BDRVSSHState *s, const char *op)
 {
-    va_list args;
+    char *ssh_err;
+    int ssh_err_code;
+    unsigned long sftp_err_code;
 
-    va_start(args, fs);
-    error_vprintf(fs, args);
-
-    if ((s)->sftp) {
-        char *ssh_err;
-        int ssh_err_code;
-        unsigned long sftp_err_code;
-
-        /* This is not an errno.  See <libssh2.h>. */
-        ssh_err_code = libssh2_session_last_error(s->session,
+    /* This is not an errno.  See <libssh2.h>. */
+    ssh_err_code = libssh2_session_last_error(s->session,
                                                   &ssh_err, NULL, 0);
-        /* See <libssh2_sftp.h>. */
-        sftp_err_code = libssh2_sftp_last_error((s)->sftp);
+    /* See <libssh2_sftp.h>. */
+    sftp_err_code = libssh2_sftp_last_error((s)->sftp);
 
-        error_printf(": %s (libssh2 error code: %d, sftp error code: %lu)",
-                     ssh_err, ssh_err_code, sftp_err_code);
-    }
-
-    va_end(args);
-    error_printf("\n");
+    trace_sftp_error(op, ssh_err, ssh_err_code, sftp_err_code);
 }
 
 static int parse_uri(const char *filename, QDict *options, Error **errp)
@@ -1035,7 +1023,7 @@ static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs,
             goto again;
         }
         if (r < 0) {
-            sftp_error_report(s, "read failed");
+            sftp_error_trace(s, "read");
             s->offset = -1;
             return -EIO;
         }
@@ -1105,7 +1093,7 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
             goto again;
         }
         if (r < 0) {
-            sftp_error_report(s, "write failed");
+            sftp_error_trace(s, "write");
             s->offset = -1;
             return -EIO;
         }
@@ -1188,7 +1176,7 @@ static coroutine_fn int ssh_flush(BDRVSSHState *s, BlockDriverState *bs)
         return 0;
     }
     if (r < 0) {
-        sftp_error_report(s, "fsync failed");
+        sftp_error_trace(s, "fsync");
         return -EIO;
     }
 
diff --git a/block/trace-events b/block/trace-events
index 7335a42540..79ccd8d824 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -208,3 +208,6 @@ sheepdog_co_rw_vector_new(uint64_t oid) "new oid 0x%" PRIx64
 sheepdog_snapshot_create_info(const char *sn_name, const char *id, const char *name, int64_t size, int is_snapshot) "sn_info: name %s id_str %s s: name %s vm_state_size %" PRId64 " " "is_snapshot %d"
 sheepdog_snapshot_create(const char *sn_name, const char *id) "%s %s"
 sheepdog_snapshot_create_inode(const char *name, uint32_t snap, uint32_t vdi) "s->inode: name %s snap_id 0x%" PRIx32 " vdi 0x%" PRIx32
+
+# ssh.c
+sftp_error(const char *op, const char *ssh_err, int ssh_err_code, unsigned long sftp_err_code) "%s failed: %s (libssh2 error code: %d, sftp error code: %lu)"
-- 
2.17.2

[Qemu-devel] [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user

[Markus Armbruster]

Callbacks ssh_co_readv(), ssh_co_writev(), ssh_co_flush() report
errors to the user with error_printf().  They shouldn't, it's their
caller's job.  Replace by a suitable trace point.

Perhaps we should convert this part of the block driver interface to
Error, so block drivers can pass more detail to their callers.  Not
today.

Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/ssh.c        | 36 ++++++++++++------------------------
 block/trace-events |  3 +++
 2 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 190ef95300..382fc04fbf 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -159,31 +159,19 @@ sftp_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...)
     g_free(msg);
 }
 
-static void GCC_FMT_ATTR(2, 3)
-sftp_error_report(BDRVSSHState *s, const char *fs, ...)
+static void sftp_error_trace(BDRVSSHState *s, const char *op)
 {
-    va_list args;
+    char *ssh_err;
+    int ssh_err_code;
+    unsigned long sftp_err_code;
 
-    va_start(args, fs);
-    error_vprintf(fs, args);
-
-    if ((s)->sftp) {
-        char *ssh_err;
-        int ssh_err_code;
-        unsigned long sftp_err_code;
-
-        /* This is not an errno.  See <libssh2.h>. */
-        ssh_err_code = libssh2_session_last_error(s->session,
+    /* This is not an errno.  See <libssh2.h>. */
+    ssh_err_code = libssh2_session_last_error(s->session,
                                                   &ssh_err, NULL, 0);
-        /* See <libssh2_sftp.h>. */
-        sftp_err_code = libssh2_sftp_last_error((s)->sftp);
+    /* See <libssh2_sftp.h>. */
+    sftp_err_code = libssh2_sftp_last_error((s)->sftp);
 
-        error_printf(": %s (libssh2 error code: %d, sftp error code: %lu)",
-                     ssh_err, ssh_err_code, sftp_err_code);
-    }
-
-    va_end(args);
-    error_printf("\n");
+    trace_sftp_error(op, ssh_err, ssh_err_code, sftp_err_code);
 }
 
 static int parse_uri(const char *filename, QDict *options, Error **errp)
@@ -1035,7 +1023,7 @@ static coroutine_fn int ssh_read(BDRVSSHState *s, BlockDriverState *bs,
             goto again;
         }
         if (r < 0) {
-            sftp_error_report(s, "read failed");
+            sftp_error_trace(s, "read");
             s->offset = -1;
             return -EIO;
         }
@@ -1105,7 +1093,7 @@ static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
             goto again;
         }
         if (r < 0) {
-            sftp_error_report(s, "write failed");
+            sftp_error_trace(s, "write");
             s->offset = -1;
             return -EIO;
         }
@@ -1188,7 +1176,7 @@ static coroutine_fn int ssh_flush(BDRVSSHState *s, BlockDriverState *bs)
         return 0;
     }
     if (r < 0) {
-        sftp_error_report(s, "fsync failed");
+        sftp_error_trace(s, "fsync");
         return -EIO;
     }
 
diff --git a/block/trace-events b/block/trace-events
index 7335a42540..79ccd8d824 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -208,3 +208,6 @@ sheepdog_co_rw_vector_new(uint64_t oid) "new oid 0x%" PRIx64
 sheepdog_snapshot_create_info(const char *sn_name, const char *id, const char *name, int64_t size, int is_snapshot) "sn_info: name %s id_str %s s: name %s vm_state_size %" PRId64 " " "is_snapshot %d"
 sheepdog_snapshot_create(const char *sn_name, const char *id) "%s %s"
 sheepdog_snapshot_create_inode(const char *name, uint32_t snap, uint32_t vdi) "s->inode: name %s snap_id 0x%" PRIx32 " vdi 0x%" PRIx32
+
+# ssh.c
+sftp_error(const char *op, const char *ssh_err, int ssh_err_code, unsigned long sftp_err_code) "%s failed: %s (libssh2 error code: %d, sftp error code: %lu)"
-- 
2.17.2

[Qemu-devel] Whither qemu's ssh driver? (was: Re: [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user)

[Richard W.M. Jones]

I don't know much about this patch which looks like internal qemu
rearrangements so I guess fine.  However I do have a few things to say
about the ssh driver ...

As you know I wrote this a few years ago, and it uses libssh2.
libssh2 has not evolved as quickly as we'd like and it may be better
to use libssh instead -- despite the names, these are two separate and
unrelated libraries.  libssh supports a wider range of SSH encryption
and has more features.  It's generally more likely to work against a
random SSH server.  It has also been through the FIPS process.  Indeed
Red Hat made the decision to switch exclusively to libssh in RHEL 8,
if that carries any weight.

Pino posted a libssh2 -> libssh conversion patch a while back, but it
has been somewhat stuck in review.  If I recall the latest concern was
whether it performs as well as the libssh2 version.

  https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html

In the meantime I added libssh support to nbdkit.  nbdkit can be used
as a complete replacement for qemu's ssh driver.

  nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
  qemu -hda nbd:unix:tmpdirXXXXXX/sock

In fact it's somewhat superior (IMHO) because all of the tricky code
handling libssh runs outside qemu in a separate process, improving
isolation and potentially allowing separate, restrictive security
policies to be applied.  For example it would no longer be necessary
to give qemu permission to connect to remote SSH servers.

Could we make this really smooth somehow?  nbdkit has a concept
[https://www.mankier.com/1/nbdkit-captive] where we make it easy to
manage external commands owned by nbdkit.  Is there an equivalent
feature of qemu where:

  qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
       -drive file.driver=nbd,file.socket=nbd1

would run the command but also allocate a socket and kill the
subcommand on exit (of qemu)?

Basically I'm trying to think about how to make this a reality:

  https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

[Qemu-devel] Whither qemu's ssh driver? (was: Re: [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user)

[Richard W.M. Jones]

I don't know much about this patch which looks like internal qemu
rearrangements so I guess fine.  However I do have a few things to say
about the ssh driver ...

As you know I wrote this a few years ago, and it uses libssh2.
libssh2 has not evolved as quickly as we'd like and it may be better
to use libssh instead -- despite the names, these are two separate and
unrelated libraries.  libssh supports a wider range of SSH encryption
and has more features.  It's generally more likely to work against a
random SSH server.  It has also been through the FIPS process.  Indeed
Red Hat made the decision to switch exclusively to libssh in RHEL 8,
if that carries any weight.

Pino posted a libssh2 -> libssh conversion patch a while back, but it
has been somewhat stuck in review.  If I recall the latest concern was
whether it performs as well as the libssh2 version.

  https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html

In the meantime I added libssh support to nbdkit.  nbdkit can be used
as a complete replacement for qemu's ssh driver.

  nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
  qemu -hda nbd:unix:tmpdirXXXXXX/sock

In fact it's somewhat superior (IMHO) because all of the tricky code
handling libssh runs outside qemu in a separate process, improving
isolation and potentially allowing separate, restrictive security
policies to be applied.  For example it would no longer be necessary
to give qemu permission to connect to remote SSH servers.

Could we make this really smooth somehow?  nbdkit has a concept
[https://www.mankier.com/1/nbdkit-captive] where we make it easy to
manage external commands owned by nbdkit.  Is there an equivalent
feature of qemu where:

  qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
       -drive file.driver=nbd,file.socket=nbd1

would run the command but also allocate a socket and kill the
subcommand on exit (of qemu)?

Basically I'm trying to think about how to make this a reality:

  https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [Qemu-devel] Whither qemu's ssh driver?

[Markus Armbruster]

"Richard W.M. Jones" <rjones@redhat.com> writes:

> I don't know much about this patch which looks like internal qemu
> rearrangements so I guess fine.  However I do have a few things to say
> about the ssh driver ...
>
> As you know I wrote this a few years ago, and it uses libssh2.
> libssh2 has not evolved as quickly as we'd like and it may be better
> to use libssh instead -- despite the names, these are two separate and
> unrelated libraries.  libssh supports a wider range of SSH encryption
> and has more features.  It's generally more likely to work against a
> random SSH server.  It has also been through the FIPS process.  Indeed
> Red Hat made the decision to switch exclusively to libssh in RHEL 8,
> if that carries any weight.
>
> Pino posted a libssh2 -> libssh conversion patch a while back, but it
> has been somewhat stuck in review.  If I recall the latest concern was
> whether it performs as well as the libssh2 version.
>
>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html

I doubt we'd need "as well as" for this driver.  But Max observed a
factor of five with v4.  Pino reported improvements with v5 ("no more
than 200%"), and some more with libssh master instead of 0.7, but didn't
quantify those.  To make progress, we need a rebased patch with actual
performance numbers, I think.

> In the meantime I added libssh support to nbdkit.  nbdkit can be used
> as a complete replacement for qemu's ssh driver.
>
>   nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
>   qemu -hda nbd:unix:tmpdirXXXXXX/sock
>
> In fact it's somewhat superior (IMHO) because all of the tricky code
> handling libssh runs outside qemu in a separate process, improving
> isolation and potentially allowing separate, restrictive security
> policies to be applied.  For example it would no longer be necessary
> to give qemu permission to connect to remote SSH servers.

Good point.

> Could we make this really smooth somehow?  nbdkit has a concept
> [https://www.mankier.com/1/nbdkit-captive] where we make it easy to
> manage external commands owned by nbdkit.  Is there an equivalent
> feature of qemu where:
>
>   qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
>        -drive file.driver=nbd,file.socket=nbd1
>
> would run the command but also allocate a socket and kill the
> subcommand on exit (of qemu)?

I'm not aware of general infrastructure to run helper processes.  But
I'm sure we could come up with something.

> Basically I'm trying to think about how to make this a reality:
>
>   https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png

Looks like you're also targeting curl.c's drivers.  Any others?

Got a backward compatibility story other than "let's deprecate these
drivers"?

Re: [Qemu-devel] Whither qemu's ssh driver?

[Markus Armbruster]

"Richard W.M. Jones" <rjones@redhat.com> writes:

> I don't know much about this patch which looks like internal qemu
> rearrangements so I guess fine.  However I do have a few things to say
> about the ssh driver ...
>
> As you know I wrote this a few years ago, and it uses libssh2.
> libssh2 has not evolved as quickly as we'd like and it may be better
> to use libssh instead -- despite the names, these are two separate and
> unrelated libraries.  libssh supports a wider range of SSH encryption
> and has more features.  It's generally more likely to work against a
> random SSH server.  It has also been through the FIPS process.  Indeed
> Red Hat made the decision to switch exclusively to libssh in RHEL 8,
> if that carries any weight.
>
> Pino posted a libssh2 -> libssh conversion patch a while back, but it
> has been somewhat stuck in review.  If I recall the latest concern was
> whether it performs as well as the libssh2 version.
>
>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html

I doubt we'd need "as well as" for this driver.  But Max observed a
factor of five with v4.  Pino reported improvements with v5 ("no more
than 200%"), and some more with libssh master instead of 0.7, but didn't
quantify those.  To make progress, we need a rebased patch with actual
performance numbers, I think.

> In the meantime I added libssh support to nbdkit.  nbdkit can be used
> as a complete replacement for qemu's ssh driver.
>
>   nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
>   qemu -hda nbd:unix:tmpdirXXXXXX/sock
>
> In fact it's somewhat superior (IMHO) because all of the tricky code
> handling libssh runs outside qemu in a separate process, improving
> isolation and potentially allowing separate, restrictive security
> policies to be applied.  For example it would no longer be necessary
> to give qemu permission to connect to remote SSH servers.

Good point.

> Could we make this really smooth somehow?  nbdkit has a concept
> [https://www.mankier.com/1/nbdkit-captive] where we make it easy to
> manage external commands owned by nbdkit.  Is there an equivalent
> feature of qemu where:
>
>   qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
>        -drive file.driver=nbd,file.socket=nbd1
>
> would run the command but also allocate a socket and kill the
> subcommand on exit (of qemu)?

I'm not aware of general infrastructure to run helper processes.  But
I'm sure we could come up with something.

> Basically I'm trying to think about how to make this a reality:
>
>   https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png

Looks like you're also targeting curl.c's drivers.  Any others?

Got a backward compatibility story other than "let's deprecate these
drivers"?

Re: [Qemu-devel] Whither qemu's ssh driver?

[Richard W.M. Jones]

On Mon, Apr 08, 2019 at 08:07:00PM +0200, Markus Armbruster wrote:
> "Richard W.M. Jones" <rjones@redhat.com> writes:
> 
> > I don't know much about this patch which looks like internal qemu
> > rearrangements so I guess fine.  However I do have a few things to say
> > about the ssh driver ...
> >
> > As you know I wrote this a few years ago, and it uses libssh2.
> > libssh2 has not evolved as quickly as we'd like and it may be better
> > to use libssh instead -- despite the names, these are two separate and
> > unrelated libraries.  libssh supports a wider range of SSH encryption
> > and has more features.  It's generally more likely to work against a
> > random SSH server.  It has also been through the FIPS process.  Indeed
> > Red Hat made the decision to switch exclusively to libssh in RHEL 8,
> > if that carries any weight.
> >
> > Pino posted a libssh2 -> libssh conversion patch a while back, but it
> > has been somewhat stuck in review.  If I recall the latest concern was
> > whether it performs as well as the libssh2 version.
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html
> 
> I doubt we'd need "as well as" for this driver.  But Max observed a
> factor of five with v4.  Pino reported improvements with v5 ("no more
> than 200%"), and some more with libssh master instead of 0.7, but didn't
> quantify those.  To make progress, we need a rebased patch with actual
> performance numbers, I think.
> 
> > In the meantime I added libssh support to nbdkit.  nbdkit can be used
> > as a complete replacement for qemu's ssh driver.
> >
> >   nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
> >   qemu -hda nbd:unix:tmpdirXXXXXX/sock
> >
> > In fact it's somewhat superior (IMHO) because all of the tricky code
> > handling libssh runs outside qemu in a separate process, improving
> > isolation and potentially allowing separate, restrictive security
> > policies to be applied.  For example it would no longer be necessary
> > to give qemu permission to connect to remote SSH servers.
> 
> Good point.
> 
> > Could we make this really smooth somehow?  nbdkit has a concept
> > [https://www.mankier.com/1/nbdkit-captive] where we make it easy to
> > manage external commands owned by nbdkit.  Is there an equivalent
> > feature of qemu where:
> >
> >   qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
> >        -drive file.driver=nbd,file.socket=nbd1
> >
> > would run the command but also allocate a socket and kill the
> > subcommand on exit (of qemu)?
> 
> I'm not aware of general infrastructure to run helper processes.  But
> I'm sure we could come up with something.
> 
> > Basically I'm trying to think about how to make this a reality:
> >
> >   https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png
> 
> Looks like you're also targeting curl.c's drivers.  Any others?

As you know I wrote a read-only version of vvfat so it could be
deprecated in qemu.  It doesn't do the vvfat write madness however.

While there are other interesting plugins for nbdkit, we are steering
clear of needlessly duplicating qemu block functionality.  There would
have to be a reason to do it, and I think the reasons are clear enough
for ssh, curl and vvfat.

> Got a backward compatibility story other than "let's deprecate these
> drivers"?

That's if we do deprecate them in qemu at all.  We can have both if
people would prefer that.  If not I suppose we could have a
replacement stub driver which would run nbdkit.  It adds a dependency,
but nbdkit is intended to have very minimal deps.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] Whither qemu's ssh driver?

[Richard W.M. Jones]

On Mon, Apr 08, 2019 at 08:07:00PM +0200, Markus Armbruster wrote:
> "Richard W.M. Jones" <rjones@redhat.com> writes:
> 
> > I don't know much about this patch which looks like internal qemu
> > rearrangements so I guess fine.  However I do have a few things to say
> > about the ssh driver ...
> >
> > As you know I wrote this a few years ago, and it uses libssh2.
> > libssh2 has not evolved as quickly as we'd like and it may be better
> > to use libssh instead -- despite the names, these are two separate and
> > unrelated libraries.  libssh supports a wider range of SSH encryption
> > and has more features.  It's generally more likely to work against a
> > random SSH server.  It has also been through the FIPS process.  Indeed
> > Red Hat made the decision to switch exclusively to libssh in RHEL 8,
> > if that carries any weight.
> >
> > Pino posted a libssh2 -> libssh conversion patch a while back, but it
> > has been somewhat stuck in review.  If I recall the latest concern was
> > whether it performs as well as the libssh2 version.
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html
> 
> I doubt we'd need "as well as" for this driver.  But Max observed a
> factor of five with v4.  Pino reported improvements with v5 ("no more
> than 200%"), and some more with libssh master instead of 0.7, but didn't
> quantify those.  To make progress, we need a rebased patch with actual
> performance numbers, I think.
> 
> > In the meantime I added libssh support to nbdkit.  nbdkit can be used
> > as a complete replacement for qemu's ssh driver.
> >
> >   nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
> >   qemu -hda nbd:unix:tmpdirXXXXXX/sock
> >
> > In fact it's somewhat superior (IMHO) because all of the tricky code
> > handling libssh runs outside qemu in a separate process, improving
> > isolation and potentially allowing separate, restrictive security
> > policies to be applied.  For example it would no longer be necessary
> > to give qemu permission to connect to remote SSH servers.
> 
> Good point.
> 
> > Could we make this really smooth somehow?  nbdkit has a concept
> > [https://www.mankier.com/1/nbdkit-captive] where we make it easy to
> > manage external commands owned by nbdkit.  Is there an equivalent
> > feature of qemu where:
> >
> >   qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
> >        -drive file.driver=nbd,file.socket=nbd1
> >
> > would run the command but also allocate a socket and kill the
> > subcommand on exit (of qemu)?
> 
> I'm not aware of general infrastructure to run helper processes.  But
> I'm sure we could come up with something.
> 
> > Basically I'm trying to think about how to make this a reality:
> >
> >   https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png
> 
> Looks like you're also targeting curl.c's drivers.  Any others?

As you know I wrote a read-only version of vvfat so it could be
deprecated in qemu.  It doesn't do the vvfat write madness however.

While there are other interesting plugins for nbdkit, we are steering
clear of needlessly duplicating qemu block functionality.  There would
have to be a reason to do it, and I think the reasons are clear enough
for ssh, curl and vvfat.

> Got a backward compatibility story other than "let's deprecate these
> drivers"?

That's if we do deprecate them in qemu at all.  We can have both if
people would prefer that.  If not I suppose we could have a
replacement stub driver which would run nbdkit.  It adds a dependency,
but nbdkit is intended to have very minimal deps.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [Qemu-devel] Whither qemu's ssh driver? (was: Re: [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user)

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 2928 bytes --]

On 08.04.19 19:22, Richard W.M. Jones wrote:
> I don't know much about this patch which looks like internal qemu
> rearrangements so I guess fine.  However I do have a few things to say
> about the ssh driver ...
> 
> As you know I wrote this a few years ago, and it uses libssh2.
> libssh2 has not evolved as quickly as we'd like and it may be better
> to use libssh instead -- despite the names, these are two separate and
> unrelated libraries.  libssh supports a wider range of SSH encryption
> and has more features.  It's generally more likely to work against a
> random SSH server.  It has also been through the FIPS process.  Indeed
> Red Hat made the decision to switch exclusively to libssh in RHEL 8,
> if that carries any weight.
> 
> Pino posted a libssh2 -> libssh conversion patch a while back, but it
> has been somewhat stuck in review.  If I recall the latest concern was
> whether it performs as well as the libssh2 version.
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html
> 
> In the meantime I added libssh support to nbdkit.  nbdkit can be used
> as a complete replacement for qemu's ssh driver.
> 
>   nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
>   qemu -hda nbd:unix:tmpdirXXXXXX/sock
> 
> In fact it's somewhat superior (IMHO) because all of the tricky code
> handling libssh runs outside qemu in a separate process, improving
> isolation and potentially allowing separate, restrictive security
> policies to be applied.  For example it would no longer be necessary
> to give qemu permission to connect to remote SSH servers.
> 
> Could we make this really smooth somehow?  nbdkit has a concept
> [https://www.mankier.com/1/nbdkit-captive] where we make it easy to
> manage external commands owned by nbdkit.  Is there an equivalent
> feature of qemu where:
> 
>   qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
>        -drive file.driver=nbd,file.socket=nbd1
> 
> would run the command but also allocate a socket and kill the
> subcommand on exit (of qemu)?
> 
> Basically I'm trying to think about how to make this a reality:
> 
>   https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png
> 
> Rich.

I don’t disagree with anything you say.  I would prefer to move the less
well maintained drivers (for which there is no strict performance
requirement) into a separate process.  nbdkit is perfectly suited for
that, and the drivers are there, as you say (ssh, curl, vvfat).

Having a nicer interface in qemu would make the transition simple,
because we could tell users exactly how to change their command line so
their use case continues to work.  I’m not sure whether it really works,
though, because I don’t think there is such a simple replacement for
being able to simply pass "ssh://host/path" to qemu and have it work.

But I think it’s still worth it.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] Whither qemu's ssh driver? (was: Re: [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user)

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 2928 bytes --]

On 08.04.19 19:22, Richard W.M. Jones wrote:
> I don't know much about this patch which looks like internal qemu
> rearrangements so I guess fine.  However I do have a few things to say
> about the ssh driver ...
> 
> As you know I wrote this a few years ago, and it uses libssh2.
> libssh2 has not evolved as quickly as we'd like and it may be better
> to use libssh instead -- despite the names, these are two separate and
> unrelated libraries.  libssh supports a wider range of SSH encryption
> and has more features.  It's generally more likely to work against a
> random SSH server.  It has also been through the FIPS process.  Indeed
> Red Hat made the decision to switch exclusively to libssh in RHEL 8,
> if that carries any weight.
> 
> Pino posted a libssh2 -> libssh conversion patch a while back, but it
> has been somewhat stuck in review.  If I recall the latest concern was
> whether it performs as well as the libssh2 version.
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html
> 
> In the meantime I added libssh support to nbdkit.  nbdkit can be used
> as a complete replacement for qemu's ssh driver.
> 
>   nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
>   qemu -hda nbd:unix:tmpdirXXXXXX/sock
> 
> In fact it's somewhat superior (IMHO) because all of the tricky code
> handling libssh runs outside qemu in a separate process, improving
> isolation and potentially allowing separate, restrictive security
> policies to be applied.  For example it would no longer be necessary
> to give qemu permission to connect to remote SSH servers.
> 
> Could we make this really smooth somehow?  nbdkit has a concept
> [https://www.mankier.com/1/nbdkit-captive] where we make it easy to
> manage external commands owned by nbdkit.  Is there an equivalent
> feature of qemu where:
> 
>   qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
>        -drive file.driver=nbd,file.socket=nbd1
> 
> would run the command but also allocate a socket and kill the
> subcommand on exit (of qemu)?
> 
> Basically I'm trying to think about how to make this a reality:
> 
>   https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png
> 
> Rich.

I don’t disagree with anything you say.  I would prefer to move the less
well maintained drivers (for which there is no strict performance
requirement) into a separate process.  nbdkit is perfectly suited for
that, and the drivers are there, as you say (ssh, curl, vvfat).

Having a nicer interface in qemu would make the transition simple,
because we could tell users exactly how to change their command line so
their use case continues to work.  I’m not sure whether it really works,
though, because I don’t think there is such a simple replacement for
being able to simply pass "ssh://host/path" to qemu and have it work.

But I think it’s still worth it.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] Whither qemu's ssh driver?

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> On 08.04.19 19:22, Richard W.M. Jones wrote:
>> I don't know much about this patch which looks like internal qemu
>> rearrangements so I guess fine.  However I do have a few things to say
>> about the ssh driver ...
>> 
>> As you know I wrote this a few years ago, and it uses libssh2.
>> libssh2 has not evolved as quickly as we'd like and it may be better
>> to use libssh instead -- despite the names, these are two separate and
>> unrelated libraries.  libssh supports a wider range of SSH encryption
>> and has more features.  It's generally more likely to work against a
>> random SSH server.  It has also been through the FIPS process.  Indeed
>> Red Hat made the decision to switch exclusively to libssh in RHEL 8,
>> if that carries any weight.
>> 
>> Pino posted a libssh2 -> libssh conversion patch a while back, but it
>> has been somewhat stuck in review.  If I recall the latest concern was
>> whether it performs as well as the libssh2 version.
>> 
>>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html
>> 
>> In the meantime I added libssh support to nbdkit.  nbdkit can be used
>> as a complete replacement for qemu's ssh driver.
>> 
>>   nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
>>   qemu -hda nbd:unix:tmpdirXXXXXX/sock
>> 
>> In fact it's somewhat superior (IMHO) because all of the tricky code
>> handling libssh runs outside qemu in a separate process, improving
>> isolation and potentially allowing separate, restrictive security
>> policies to be applied.  For example it would no longer be necessary
>> to give qemu permission to connect to remote SSH servers.
>> 
>> Could we make this really smooth somehow?  nbdkit has a concept
>> [https://www.mankier.com/1/nbdkit-captive] where we make it easy to
>> manage external commands owned by nbdkit.  Is there an equivalent
>> feature of qemu where:
>> 
>>   qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
>>        -drive file.driver=nbd,file.socket=nbd1
>> 
>> would run the command but also allocate a socket and kill the
>> subcommand on exit (of qemu)?
>> 
>> Basically I'm trying to think about how to make this a reality:
>> 
>>   https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png
>> 
>> Rich.
>
> I don’t disagree with anything you say.  I would prefer to move the less
> well maintained drivers (for which there is no strict performance
> requirement) into a separate process.  nbdkit is perfectly suited for
> that, and the drivers are there, as you say (ssh, curl, vvfat).
>
> Having a nicer interface in qemu would make the transition simple,
> because we could tell users exactly how to change their command line so
> their use case continues to work.  I’m not sure whether it really works,
> though, because I don’t think there is such a simple replacement for
> being able to simply pass "ssh://host/path" to qemu and have it work.
>
> But I think it’s still worth it.

I guess that boils down to "patches welcome".

For v1, I wouldn't worry about making the transition simple.  Just show
us some working code.

Re: [Qemu-devel] Whither qemu's ssh driver?

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> On 08.04.19 19:22, Richard W.M. Jones wrote:
>> I don't know much about this patch which looks like internal qemu
>> rearrangements so I guess fine.  However I do have a few things to say
>> about the ssh driver ...
>> 
>> As you know I wrote this a few years ago, and it uses libssh2.
>> libssh2 has not evolved as quickly as we'd like and it may be better
>> to use libssh instead -- despite the names, these are two separate and
>> unrelated libraries.  libssh supports a wider range of SSH encryption
>> and has more features.  It's generally more likely to work against a
>> random SSH server.  It has also been through the FIPS process.  Indeed
>> Red Hat made the decision to switch exclusively to libssh in RHEL 8,
>> if that carries any weight.
>> 
>> Pino posted a libssh2 -> libssh conversion patch a while back, but it
>> has been somewhat stuck in review.  If I recall the latest concern was
>> whether it performs as well as the libssh2 version.
>> 
>>   https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg07267.html
>> 
>> In the meantime I added libssh support to nbdkit.  nbdkit can be used
>> as a complete replacement for qemu's ssh driver.
>> 
>>   nbdkit ssh host=foo.example.com disk.img -U tmpdirXXXXXX/sock
>>   qemu -hda nbd:unix:tmpdirXXXXXX/sock
>> 
>> In fact it's somewhat superior (IMHO) because all of the tricky code
>> handling libssh runs outside qemu in a separate process, improving
>> isolation and potentially allowing separate, restrictive security
>> policies to be applied.  For example it would no longer be necessary
>> to give qemu permission to connect to remote SSH servers.
>> 
>> Could we make this really smooth somehow?  nbdkit has a concept
>> [https://www.mankier.com/1/nbdkit-captive] where we make it easy to
>> manage external commands owned by nbdkit.  Is there an equivalent
>> feature of qemu where:
>> 
>>   qemu -object exec,id=nbd1,cmd='nbdkit -f -U $sock ssh ...' \
>>        -drive file.driver=nbd,file.socket=nbd1
>> 
>> would run the command but also allocate a socket and kill the
>> subcommand on exit (of qemu)?
>> 
>> Basically I'm trying to think about how to make this a reality:
>> 
>>   https://rwmj.files.wordpress.com/2018/10/drawing2-svg.png
>> 
>> Rich.
>
> I don’t disagree with anything you say.  I would prefer to move the less
> well maintained drivers (for which there is no strict performance
> requirement) into a separate process.  nbdkit is perfectly suited for
> that, and the drivers are there, as you say (ssh, curl, vvfat).
>
> Having a nicer interface in qemu would make the transition simple,
> because we could tell users exactly how to change their command line so
> their use case continues to work.  I’m not sure whether it really works,
> though, because I don’t think there is such a simple replacement for
> being able to simply pass "ssh://host/path" to qemu and have it work.
>
> But I think it’s still worth it.

I guess that boils down to "patches welcome".

For v1, I wouldn't worry about making the transition simple.  Just show
us some working code.

Re: [Qemu-devel] [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]

On 4/8/19 3:36 AM, Markus Armbruster wrote:
> Callbacks ssh_co_readv(), ssh_co_writev(), ssh_co_flush() report
> errors to the user with error_printf().  They shouldn't, it's their
> caller's job.  Replace by a suitable trace point.
> 
> Perhaps we should convert this part of the block driver interface to
> Error, so block drivers can pass more detail to their callers.  Not
> today.
> 
> Cc: "Richard W.M. Jones" <rjones@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: qemu-block@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/ssh.c        | 36 ++++++++++++------------------------
>  block/trace-events |  3 +++
>  2 files changed, 15 insertions(+), 24 deletions(-)
> 

> -static void GCC_FMT_ATTR(2, 3)
> -sftp_error_report(BDRVSSHState *s, const char *fs, ...)
> +static void sftp_error_trace(BDRVSSHState *s, const char *op)
>  {
> -    va_list args;
> +    char *ssh_err;
> +    int ssh_err_code;
> +    unsigned long sftp_err_code;
>  
> -    va_start(args, fs);
> -    error_vprintf(fs, args);
> -
> -    if ((s)->sftp) {

The old code was conditional,

> -        char *ssh_err;
> -        int ssh_err_code;
> -        unsigned long sftp_err_code;
> -
> -        /* This is not an errno.  See <libssh2.h>. */
> -        ssh_err_code = libssh2_session_last_error(s->session,
> +    /* This is not an errno.  See <libssh2.h>. */
> +    ssh_err_code = libssh2_session_last_error(s->session,
>                                                    &ssh_err, NULL, 0);

Indentation looks off now.

> -        /* See <libssh2_sftp.h>. */
> -        sftp_err_code = libssh2_sftp_last_error((s)->sftp);
> +    /* See <libssh2_sftp.h>. */
> +    sftp_err_code = libssh2_sftp_last_error((s)->sftp);

but it appears the new code can call libssh2_sftp_last_error(NULL). Am I
missing something, or could that be a problem?

/me rescans the full file...

Okay, connect_to_ssh() won't succeed unless s->sftp is set, and it
appears that all callers to the trace function can only be reached if
connect succeeded.

Indentation fixup can be done by maintainer,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 4/8/19 3:36 AM, Markus Armbruster wrote:
>> Callbacks ssh_co_readv(), ssh_co_writev(), ssh_co_flush() report
>> errors to the user with error_printf().  They shouldn't, it's their
>> caller's job.  Replace by a suitable trace point.
>> 
>> Perhaps we should convert this part of the block driver interface to
>> Error, so block drivers can pass more detail to their callers.  Not
>> today.
>> 
>> Cc: "Richard W.M. Jones" <rjones@redhat.com>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Max Reitz <mreitz@redhat.com>
>> Cc: qemu-block@nongnu.org
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  block/ssh.c        | 36 ++++++++++++------------------------
>>  block/trace-events |  3 +++
>>  2 files changed, 15 insertions(+), 24 deletions(-)
>> 
>
>> -static void GCC_FMT_ATTR(2, 3)
>> -sftp_error_report(BDRVSSHState *s, const char *fs, ...)
>> +static void sftp_error_trace(BDRVSSHState *s, const char *op)
>>  {
>> -    va_list args;
>> +    char *ssh_err;
>> +    int ssh_err_code;
>> +    unsigned long sftp_err_code;
>>  
>> -    va_start(args, fs);
>> -    error_vprintf(fs, args);
>> -
>> -    if ((s)->sftp) {
>
> The old code was conditional,
>
>> -        char *ssh_err;
>> -        int ssh_err_code;
>> -        unsigned long sftp_err_code;
>> -
>> -        /* This is not an errno.  See <libssh2.h>. */
>> -        ssh_err_code = libssh2_session_last_error(s->session,
>> +    /* This is not an errno.  See <libssh2.h>. */
>> +    ssh_err_code = libssh2_session_last_error(s->session,
>>                                                    &ssh_err, NULL, 0);
>
> Indentation looks off now.

Will fix.

>> -        /* See <libssh2_sftp.h>. */
>> -        sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>> +    /* See <libssh2_sftp.h>. */
>> +    sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>
> but it appears the new code can call libssh2_sftp_last_error(NULL). Am I
> missing something, or could that be a problem?
>
> /me rescans the full file...
>
> Okay, connect_to_ssh() won't succeed unless s->sftp is set, and it
> appears that all callers to the trace function can only be reached if
> connect succeeded.

I can add to the commit message

    While there, drop the unreachable !s->sftp case.

> Indentation fixup can be done by maintainer,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [Qemu-devel] [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 4/8/19 3:36 AM, Markus Armbruster wrote:
>> Callbacks ssh_co_readv(), ssh_co_writev(), ssh_co_flush() report
>> errors to the user with error_printf().  They shouldn't, it's their
>> caller's job.  Replace by a suitable trace point.
>> 
>> Perhaps we should convert this part of the block driver interface to
>> Error, so block drivers can pass more detail to their callers.  Not
>> today.
>> 
>> Cc: "Richard W.M. Jones" <rjones@redhat.com>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Max Reitz <mreitz@redhat.com>
>> Cc: qemu-block@nongnu.org
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  block/ssh.c        | 36 ++++++++++++------------------------
>>  block/trace-events |  3 +++
>>  2 files changed, 15 insertions(+), 24 deletions(-)
>> 
>
>> -static void GCC_FMT_ATTR(2, 3)
>> -sftp_error_report(BDRVSSHState *s, const char *fs, ...)
>> +static void sftp_error_trace(BDRVSSHState *s, const char *op)
>>  {
>> -    va_list args;
>> +    char *ssh_err;
>> +    int ssh_err_code;
>> +    unsigned long sftp_err_code;
>>  
>> -    va_start(args, fs);
>> -    error_vprintf(fs, args);
>> -
>> -    if ((s)->sftp) {
>
> The old code was conditional,
>
>> -        char *ssh_err;
>> -        int ssh_err_code;
>> -        unsigned long sftp_err_code;
>> -
>> -        /* This is not an errno.  See <libssh2.h>. */
>> -        ssh_err_code = libssh2_session_last_error(s->session,
>> +    /* This is not an errno.  See <libssh2.h>. */
>> +    ssh_err_code = libssh2_session_last_error(s->session,
>>                                                    &ssh_err, NULL, 0);
>
> Indentation looks off now.

Will fix.

>> -        /* See <libssh2_sftp.h>. */
>> -        sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>> +    /* See <libssh2_sftp.h>. */
>> +    sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>
> but it appears the new code can call libssh2_sftp_last_error(NULL). Am I
> missing something, or could that be a problem?
>
> /me rescans the full file...
>
> Okay, connect_to_ssh() won't succeed unless s->sftp is set, and it
> appears that all callers to the trace function can only be reached if
> connect succeeded.

I can add to the commit message

    While there, drop the unreachable !s->sftp case.

> Indentation fixup can be done by maintainer,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

[Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Markus Armbruster]

char_pty_open() prints a "char device redirected to PTY_NAME (label
LABEL)" message to the current monitor or else to stderr.  No other
ChardevClass::open() prints anything on success.  Drop the message.

Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 chardev/char-pty.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index b034332edd..a48d3e5d20 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -211,8 +211,6 @@ static void char_pty_open(Chardev *chr,
     qemu_set_nonblock(master_fd);
 
     chr->filename = g_strdup_printf("pty:%s", pty_name);
-    error_printf("char device redirected to %s (label %s)\n",
-                 pty_name, chr->label);
 
     s = PTY_CHARDEV(chr);
     s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));
-- 
2.17.2

Re: [Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Marc-André Lureau]

Hi

On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> char_pty_open() prints a "char device redirected to PTY_NAME (label
> LABEL)" message to the current monitor or else to stderr.  No other
> ChardevClass::open() prints anything on success.  Drop the message.
>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  chardev/char-pty.c | 2 --
>  1 file changed, 2 deletions(-)
>

I guess that was printed for convenience.

Allocated pty can be read from "info chardev", so I suppose we can
make qemu more silent. Fine with me:

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index b034332edd..a48d3e5d20 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -211,8 +211,6 @@ static void char_pty_open(Chardev *chr,
>      qemu_set_nonblock(master_fd);
>
>      chr->filename = g_strdup_printf("pty:%s", pty_name);
> -    error_printf("char device redirected to %s (label %s)\n",
> -                 pty_name, chr->label);
>
>      s = PTY_CHARDEV(chr);
>      s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));
> --
> 2.17.2
>

Re: [Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Marc-André Lureau]

Hi

On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> char_pty_open() prints a "char device redirected to PTY_NAME (label
> LABEL)" message to the current monitor or else to stderr.  No other
> ChardevClass::open() prints anything on success.  Drop the message.
>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  chardev/char-pty.c | 2 --
>  1 file changed, 2 deletions(-)
>

I guess that was printed for convenience.

Allocated pty can be read from "info chardev", so I suppose we can
make qemu more silent. Fine with me:

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index b034332edd..a48d3e5d20 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -211,8 +211,6 @@ static void char_pty_open(Chardev *chr,
>      qemu_set_nonblock(master_fd);
>
>      chr->filename = g_strdup_printf("pty:%s", pty_name);
> -    error_printf("char device redirected to %s (label %s)\n",
> -                 pty_name, chr->label);
>
>      s = PTY_CHARDEV(chr);
>      s->ioc = QIO_CHANNEL(qio_channel_file_new_fd(master_fd));
> --
> 2.17.2
>

Re: [Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> char_pty_open() prints a "char device redirected to PTY_NAME (label
>> LABEL)" message to the current monitor or else to stderr.  No other
>> ChardevClass::open() prints anything on success.  Drop the message.
>>
>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  chardev/char-pty.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>
> I guess that was printed for convenience.
>
> Allocated pty can be read from "info chardev", so I suppose we can
> make qemu more silent. Fine with me:
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks!

If we should decide the message is still useful enough to be worth
keeping, I could direct it to stdout instead of dropping it.

Re: [Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Philippe Mathieu-Daudé]

On 4/8/19 2:31 PM, Markus Armbruster wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
>> Hi
>>
>> On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>>>
>>> char_pty_open() prints a "char device redirected to PTY_NAME (label
>>> LABEL)" message to the current monitor or else to stderr.  No other
>>> ChardevClass::open() prints anything on success.  Drop the message.
>>>
>>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  chardev/char-pty.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>
>> I guess that was printed for convenience.
>>
>> Allocated pty can be read from "info chardev", so I suppose we can
>> make qemu more silent. Fine with me:
>>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Thanks!
> 
> If we should decide the message is still useful enough to be worth
> keeping, I could direct it to stdout instead of dropping it.

I prefer this, if Marc-André is still OK.

Re: [Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Marc-André Lureau]

Hi

On Tue, Apr 9, 2019 at 12:40 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 4/8/19 2:31 PM, Markus Armbruster wrote:
> > Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> >
> >> Hi
> >>
> >> On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
> >>>
> >>> char_pty_open() prints a "char device redirected to PTY_NAME (label
> >>> LABEL)" message to the current monitor or else to stderr.  No other
> >>> ChardevClass::open() prints anything on success.  Drop the message.
> >>>
> >>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >>> ---
> >>>  chardev/char-pty.c | 2 --
> >>>  1 file changed, 2 deletions(-)
> >>>
> >>
> >> I guess that was printed for convenience.
> >>
> >> Allocated pty can be read from "info chardev", so I suppose we can
> >> make qemu more silent. Fine with me:
> >>
> >> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Thanks!
> >
> > If we should decide the message is still useful enough to be worth
> > keeping, I could direct it to stdout instead of dropping it.
>
> I prefer this, if Marc-André is still OK.

I don't mind, but I wonder if we really have users that care. Power
users can easily get the associated pty from a monitor query.


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Marc-André Lureau]

Hi

On Tue, Apr 9, 2019 at 12:40 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 4/8/19 2:31 PM, Markus Armbruster wrote:
> > Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> >
> >> Hi
> >>
> >> On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
> >>>
> >>> char_pty_open() prints a "char device redirected to PTY_NAME (label
> >>> LABEL)" message to the current monitor or else to stderr.  No other
> >>> ChardevClass::open() prints anything on success.  Drop the message.
> >>>
> >>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >>> ---
> >>>  chardev/char-pty.c | 2 --
> >>>  1 file changed, 2 deletions(-)
> >>>
> >>
> >> I guess that was printed for convenience.
> >>
> >> Allocated pty can be read from "info chardev", so I suppose we can
> >> make qemu more silent. Fine with me:
> >>
> >> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Thanks!
> >
> > If we should decide the message is still useful enough to be worth
> > keeping, I could direct it to stdout instead of dropping it.
>
> I prefer this, if Marc-André is still OK.

I don't mind, but I wonder if we really have users that care. Power
users can easily get the associated pty from a monitor query.


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Daniel P. Berrangé]

On Tue, Apr 09, 2019 at 01:25:43PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Apr 9, 2019 at 12:40 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
> >
> > On 4/8/19 2:31 PM, Markus Armbruster wrote:
> > > Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> > >
> > >> Hi
> > >>
> > >> On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
> > >>>
> > >>> char_pty_open() prints a "char device redirected to PTY_NAME (label
> > >>> LABEL)" message to the current monitor or else to stderr.  No other
> > >>> ChardevClass::open() prints anything on success.  Drop the message.
> > >>>
> > >>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> > >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> > >>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > >>> ---
> > >>>  chardev/char-pty.c | 2 --
> > >>>  1 file changed, 2 deletions(-)
> > >>>
> > >>
> > >> I guess that was printed for convenience.
> > >>
> > >> Allocated pty can be read from "info chardev", so I suppose we can
> > >> make qemu more silent. Fine with me:
> > >>
> > >> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Thanks!
> > >
> > > If we should decide the message is still useful enough to be worth
> > > keeping, I could direct it to stdout instead of dropping it.
> >
> > I prefer this, if Marc-André is still OK.
> 
> I don't mind, but I wonder if we really have users that care. Power
> users can easily get the associated pty from a monitor query.

IMHO if people want predictable filenames for chardevs without needing to
lookup the name via the monitor, then UNIX sockets fill that need.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 03/15] char-pty: Drop "char device redirected to" message

[Daniel P. Berrangé]

On Tue, Apr 09, 2019 at 01:25:43PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Apr 9, 2019 at 12:40 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
> >
> > On 4/8/19 2:31 PM, Markus Armbruster wrote:
> > > Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> > >
> > >> Hi
> > >>
> > >> On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
> > >>>
> > >>> char_pty_open() prints a "char device redirected to PTY_NAME (label
> > >>> LABEL)" message to the current monitor or else to stderr.  No other
> > >>> ChardevClass::open() prints anything on success.  Drop the message.
> > >>>
> > >>> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> > >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> > >>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > >>> ---
> > >>>  chardev/char-pty.c | 2 --
> > >>>  1 file changed, 2 deletions(-)
> > >>>
> > >>
> > >> I guess that was printed for convenience.
> > >>
> > >> Allocated pty can be read from "info chardev", so I suppose we can
> > >> make qemu more silent. Fine with me:
> > >>
> > >> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Thanks!
> > >
> > > If we should decide the message is still useful enough to be worth
> > > keeping, I could direct it to stdout instead of dropping it.
> >
> > I prefer this, if Marc-André is still OK.
> 
> I don't mind, but I wonder if we really have users that care. Power
> users can easily get the associated pty from a monitor query.

IMHO if people want predictable filenames for chardevs without needing to
lookup the name via the monitor, then UNIX sockets fill that need.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH 04/15] loader-fit: Wean off error_printf()

[Markus Armbruster]

load_fit() reports errors with error_printf() instead of
error_report().  Worse, it even reports errors it actually recovers
from, in fit_cfg_compatible() and fit_load_fdt().  Messed up in
initial commit 51b58561c1d.

Convert the helper functions for load_fit() to Error.  Make sure each
failure path sets an error.

Fix fit_cfg_compatible() and fit_load_fdt() not to report errors they
actually recover from.

Convert load_fit() to error_report().

Cc: Paul Burton <pburton@wavecomp.com>
Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/core/loader-fit.c | 62 +++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c
index 447f60857d..f27b6af942 100644
--- a/hw/core/loader-fit.c
+++ b/hw/core/loader-fit.c
@@ -18,6 +18,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/units.h"
 #include "exec/memory.h"
 #include "hw/loader.h"
@@ -33,7 +34,7 @@
 #define FIT_LOADER_MAX_PATH (128)
 
 static const void *fit_load_image_alloc(const void *itb, const char *name,
-                                        int *poff, size_t *psz)
+                                        int *poff, size_t *psz, Error **errp)
 {
     const void *data;
     const char *comp;
@@ -46,6 +47,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
     off = fdt_path_offset(itb, path);
     if (off < 0) {
+        error_setg(errp, "can't find node %s", path);
         return NULL;
     }
     if (poff) {
@@ -54,6 +56,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
     data = fdt_getprop(itb, off, "data", &sz);
     if (!data) {
+        error_setg(errp, "can't get %s/data", path);
         return NULL;
     }
 
@@ -73,7 +76,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
         uncomp_len = gunzip(uncomp_data, uncomp_len, (void *) data, sz);
         if (uncomp_len < 0) {
-            error_printf("unable to decompress %s image\n", name);
+            error_setg(errp, "unable to decompress %s image", name);
             g_free(uncomp_data);
             return NULL;
         }
@@ -85,18 +88,19 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
         return data;
     }
 
-    error_printf("unknown compression '%s'\n", comp);
+    error_setg(errp, "unknown compression '%s'", comp);
     return NULL;
 }
 
 static int fit_image_addr(const void *itb, int img, const char *name,
-                          hwaddr *addr)
+                          hwaddr *addr, Error **errp)
 {
     const void *prop;
     int len;
 
     prop = fdt_getprop(itb, img, name, &len);
     if (!prop) {
+        error_setg(errp, "can't find %s address", name);
         return -ENOENT;
     }
 
@@ -108,13 +112,14 @@ static int fit_image_addr(const void *itb, int img, const char *name,
         *addr = fdt64_to_cpu(*(fdt64_t *)prop);
         return 0;
     default:
-        error_printf("invalid %s address length %d\n", name, len);
+        error_setg(errp, "invalid %s address length %d", name, len);
         return -EINVAL;
     }
 }
 
 static int fit_load_kernel(const struct fit_loader *ldr, const void *itb,
-                           int cfg, void *opaque, hwaddr *pend)
+                           int cfg, void *opaque, hwaddr *pend,
+                           Error **errp)
 {
     const char *name;
     const void *data;
@@ -126,26 +131,26 @@ static int fit_load_kernel(const struct fit_loader *ldr, const void *itb,
 
     name = fdt_getprop(itb, cfg, "kernel", NULL);
     if (!name) {
-        error_printf("no kernel specified by FIT configuration\n");
+        error_setg(errp, "no kernel specified by FIT configuration");
         return -EINVAL;
     }
 
-    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz);
+    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
     if (!data) {
-        error_printf("unable to load kernel image from FIT\n");
+        error_prepend(errp, "unable to load kernel image from FIT: ");
         return -EINVAL;
     }
 
-    err = fit_image_addr(itb, img_off, "load", &load_addr);
+    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
     if (err) {
-        error_printf("unable to read kernel load address from FIT\n");
+        error_prepend(errp, "unable to read kernel load address from FIT: ");
         ret = err;
         goto out;
     }
 
-    err = fit_image_addr(itb, img_off, "entry", &entry_addr);
+    err = fit_image_addr(itb, img_off, "entry", &entry_addr, errp);
     if (err) {
-        error_printf("unable to read kernel entry address from FIT\n");
+        error_prepend(errp, "unable to read kernel entry address from FIT: ");
         ret = err;
         goto out;
     }
@@ -172,7 +177,7 @@ out:
 
 static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
                         int cfg, void *opaque, const void *match_data,
-                        hwaddr kernel_end)
+                        hwaddr kernel_end, Error **errp)
 {
     const char *name;
     const void *data;
@@ -187,16 +192,18 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
         return 0;
     }
 
-    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz);
+    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
     if (!data) {
-        error_printf("unable to load FDT image from FIT\n");
+        error_prepend(errp, "unable to load FDT image from FIT: ");
         return -EINVAL;
     }
 
-    err = fit_image_addr(itb, img_off, "load", &load_addr);
+    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
     if (err == -ENOENT) {
         load_addr = ROUND_UP(kernel_end, 64 * KiB) + (10 * MiB);
+        error_free(*errp);
     } else if (err) {
+        error_prepend(errp, "unable to read FDT load address from FIT: ");
         ret = err;
         goto out;
     }
@@ -229,7 +236,7 @@ static bool fit_cfg_compatible(const void *itb, int cfg, const char *compat)
         return false;
     }
 
-    fdt = fit_load_image_alloc(itb, fdt_name, NULL, NULL);
+    fdt = fit_load_image_alloc(itb, fdt_name, NULL, NULL, NULL);
     if (!fdt) {
         return false;
     }
@@ -252,11 +259,12 @@ out:
 
 int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
 {
+    Error *err = NULL;
     const struct fit_loader_match *match;
     const void *itb, *match_data = NULL;
     const char *def_cfg_name;
     char path[FIT_LOADER_MAX_PATH];
-    int itb_size, configs, cfg_off, off, err;
+    int itb_size, configs, cfg_off, off;
     hwaddr kernel_end;
     int ret;
 
@@ -267,6 +275,7 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
 
     configs = fdt_path_offset(itb, "/configurations");
     if (configs < 0) {
+        error_report("can't find node /configurations");
         ret = configs;
         goto out;
     }
@@ -301,20 +310,21 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
     }
 
     if (cfg_off < 0) {
-        /* couldn't find a configuration to use */
+        error_report("can't find configuration");
         ret = cfg_off;
         goto out;
     }
 
-    err = fit_load_kernel(ldr, itb, cfg_off, opaque, &kernel_end);
-    if (err) {
-        ret = err;
+    ret = fit_load_kernel(ldr, itb, cfg_off, opaque, &kernel_end, &err);
+    if (ret) {
+        error_report_err(err);
         goto out;
     }
 
-    err = fit_load_fdt(ldr, itb, cfg_off, opaque, match_data, kernel_end);
-    if (err) {
-        ret = err;
+    ret = fit_load_fdt(ldr, itb, cfg_off, opaque, match_data, kernel_end,
+                       &err);
+    if (ret) {
+        error_report_err(err);
         goto out;
     }
 
-- 
2.17.2

[Qemu-devel] [PATCH 04/15] loader-fit: Wean off error_printf()

[Markus Armbruster]

load_fit() reports errors with error_printf() instead of
error_report().  Worse, it even reports errors it actually recovers
from, in fit_cfg_compatible() and fit_load_fdt().  Messed up in
initial commit 51b58561c1d.

Convert the helper functions for load_fit() to Error.  Make sure each
failure path sets an error.

Fix fit_cfg_compatible() and fit_load_fdt() not to report errors they
actually recover from.

Convert load_fit() to error_report().

Cc: Paul Burton <pburton@wavecomp.com>
Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/core/loader-fit.c | 62 +++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c
index 447f60857d..f27b6af942 100644
--- a/hw/core/loader-fit.c
+++ b/hw/core/loader-fit.c
@@ -18,6 +18,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/units.h"
 #include "exec/memory.h"
 #include "hw/loader.h"
@@ -33,7 +34,7 @@
 #define FIT_LOADER_MAX_PATH (128)
 
 static const void *fit_load_image_alloc(const void *itb, const char *name,
-                                        int *poff, size_t *psz)
+                                        int *poff, size_t *psz, Error **errp)
 {
     const void *data;
     const char *comp;
@@ -46,6 +47,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
     off = fdt_path_offset(itb, path);
     if (off < 0) {
+        error_setg(errp, "can't find node %s", path);
         return NULL;
     }
     if (poff) {
@@ -54,6 +56,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
     data = fdt_getprop(itb, off, "data", &sz);
     if (!data) {
+        error_setg(errp, "can't get %s/data", path);
         return NULL;
     }
 
@@ -73,7 +76,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
 
         uncomp_len = gunzip(uncomp_data, uncomp_len, (void *) data, sz);
         if (uncomp_len < 0) {
-            error_printf("unable to decompress %s image\n", name);
+            error_setg(errp, "unable to decompress %s image", name);
             g_free(uncomp_data);
             return NULL;
         }
@@ -85,18 +88,19 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
         return data;
     }
 
-    error_printf("unknown compression '%s'\n", comp);
+    error_setg(errp, "unknown compression '%s'", comp);
     return NULL;
 }
 
 static int fit_image_addr(const void *itb, int img, const char *name,
-                          hwaddr *addr)
+                          hwaddr *addr, Error **errp)
 {
     const void *prop;
     int len;
 
     prop = fdt_getprop(itb, img, name, &len);
     if (!prop) {
+        error_setg(errp, "can't find %s address", name);
         return -ENOENT;
     }
 
@@ -108,13 +112,14 @@ static int fit_image_addr(const void *itb, int img, const char *name,
         *addr = fdt64_to_cpu(*(fdt64_t *)prop);
         return 0;
     default:
-        error_printf("invalid %s address length %d\n", name, len);
+        error_setg(errp, "invalid %s address length %d", name, len);
         return -EINVAL;
     }
 }
 
 static int fit_load_kernel(const struct fit_loader *ldr, const void *itb,
-                           int cfg, void *opaque, hwaddr *pend)
+                           int cfg, void *opaque, hwaddr *pend,
+                           Error **errp)
 {
     const char *name;
     const void *data;
@@ -126,26 +131,26 @@ static int fit_load_kernel(const struct fit_loader *ldr, const void *itb,
 
     name = fdt_getprop(itb, cfg, "kernel", NULL);
     if (!name) {
-        error_printf("no kernel specified by FIT configuration\n");
+        error_setg(errp, "no kernel specified by FIT configuration");
         return -EINVAL;
     }
 
-    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz);
+    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
     if (!data) {
-        error_printf("unable to load kernel image from FIT\n");
+        error_prepend(errp, "unable to load kernel image from FIT: ");
         return -EINVAL;
     }
 
-    err = fit_image_addr(itb, img_off, "load", &load_addr);
+    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
     if (err) {
-        error_printf("unable to read kernel load address from FIT\n");
+        error_prepend(errp, "unable to read kernel load address from FIT: ");
         ret = err;
         goto out;
     }
 
-    err = fit_image_addr(itb, img_off, "entry", &entry_addr);
+    err = fit_image_addr(itb, img_off, "entry", &entry_addr, errp);
     if (err) {
-        error_printf("unable to read kernel entry address from FIT\n");
+        error_prepend(errp, "unable to read kernel entry address from FIT: ");
         ret = err;
         goto out;
     }
@@ -172,7 +177,7 @@ out:
 
 static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
                         int cfg, void *opaque, const void *match_data,
-                        hwaddr kernel_end)
+                        hwaddr kernel_end, Error **errp)
 {
     const char *name;
     const void *data;
@@ -187,16 +192,18 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
         return 0;
     }
 
-    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz);
+    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
     if (!data) {
-        error_printf("unable to load FDT image from FIT\n");
+        error_prepend(errp, "unable to load FDT image from FIT: ");
         return -EINVAL;
     }
 
-    err = fit_image_addr(itb, img_off, "load", &load_addr);
+    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
     if (err == -ENOENT) {
         load_addr = ROUND_UP(kernel_end, 64 * KiB) + (10 * MiB);
+        error_free(*errp);
     } else if (err) {
+        error_prepend(errp, "unable to read FDT load address from FIT: ");
         ret = err;
         goto out;
     }
@@ -229,7 +236,7 @@ static bool fit_cfg_compatible(const void *itb, int cfg, const char *compat)
         return false;
     }
 
-    fdt = fit_load_image_alloc(itb, fdt_name, NULL, NULL);
+    fdt = fit_load_image_alloc(itb, fdt_name, NULL, NULL, NULL);
     if (!fdt) {
         return false;
     }
@@ -252,11 +259,12 @@ out:
 
 int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
 {
+    Error *err = NULL;
     const struct fit_loader_match *match;
     const void *itb, *match_data = NULL;
     const char *def_cfg_name;
     char path[FIT_LOADER_MAX_PATH];
-    int itb_size, configs, cfg_off, off, err;
+    int itb_size, configs, cfg_off, off;
     hwaddr kernel_end;
     int ret;
 
@@ -267,6 +275,7 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
 
     configs = fdt_path_offset(itb, "/configurations");
     if (configs < 0) {
+        error_report("can't find node /configurations");
         ret = configs;
         goto out;
     }
@@ -301,20 +310,21 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
     }
 
     if (cfg_off < 0) {
-        /* couldn't find a configuration to use */
+        error_report("can't find configuration");
         ret = cfg_off;
         goto out;
     }
 
-    err = fit_load_kernel(ldr, itb, cfg_off, opaque, &kernel_end);
-    if (err) {
-        ret = err;
+    ret = fit_load_kernel(ldr, itb, cfg_off, opaque, &kernel_end, &err);
+    if (ret) {
+        error_report_err(err);
         goto out;
     }
 
-    err = fit_load_fdt(ldr, itb, cfg_off, opaque, match_data, kernel_end);
-    if (err) {
-        ret = err;
+    ret = fit_load_fdt(ldr, itb, cfg_off, opaque, match_data, kernel_end,
+                       &err);
+    if (ret) {
+        error_report_err(err);
         goto out;
     }
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH 04/15] loader-fit: Wean off error_printf()

[Philippe Mathieu-Daudé]

On 4/8/19 10:36 AM, Markus Armbruster wrote:
> load_fit() reports errors with error_printf() instead of
> error_report().  Worse, it even reports errors it actually recovers
> from, in fit_cfg_compatible() and fit_load_fdt().  Messed up in
> initial commit 51b58561c1d.
> 
> Convert the helper functions for load_fit() to Error.  Make sure each
> failure path sets an error.
> 
> Fix fit_cfg_compatible() and fit_load_fdt() not to report errors they
> actually recover from.
> 
> Convert load_fit() to error_report().
> 
> Cc: Paul Burton <pburton@wavecomp.com>
> Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/core/loader-fit.c | 62 +++++++++++++++++++++++++-------------------
>  1 file changed, 36 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c
> index 447f60857d..f27b6af942 100644
> --- a/hw/core/loader-fit.c
> +++ b/hw/core/loader-fit.c
> @@ -18,6 +18,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "qemu/units.h"
>  #include "exec/memory.h"
>  #include "hw/loader.h"
> @@ -33,7 +34,7 @@
>  #define FIT_LOADER_MAX_PATH (128)
>  
>  static const void *fit_load_image_alloc(const void *itb, const char *name,
> -                                        int *poff, size_t *psz)
> +                                        int *poff, size_t *psz, Error **errp)
>  {
>      const void *data;
>      const char *comp;
> @@ -46,6 +47,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
>  
>      off = fdt_path_offset(itb, path);
>      if (off < 0) {
> +        error_setg(errp, "can't find node %s", path);
>          return NULL;
>      }
>      if (poff) {
> @@ -54,6 +56,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
>  
>      data = fdt_getprop(itb, off, "data", &sz);
>      if (!data) {
> +        error_setg(errp, "can't get %s/data", path);
>          return NULL;
>      }
>  
> @@ -73,7 +76,7 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
>  
>          uncomp_len = gunzip(uncomp_data, uncomp_len, (void *) data, sz);
>          if (uncomp_len < 0) {
> -            error_printf("unable to decompress %s image\n", name);
> +            error_setg(errp, "unable to decompress %s image", name);
>              g_free(uncomp_data);
>              return NULL;
>          }
> @@ -85,18 +88,19 @@ static const void *fit_load_image_alloc(const void *itb, const char *name,
>          return data;
>      }
>  
> -    error_printf("unknown compression '%s'\n", comp);
> +    error_setg(errp, "unknown compression '%s'", comp);
>      return NULL;
>  }
>  
>  static int fit_image_addr(const void *itb, int img, const char *name,
> -                          hwaddr *addr)
> +                          hwaddr *addr, Error **errp)
>  {
>      const void *prop;
>      int len;
>  
>      prop = fdt_getprop(itb, img, name, &len);
>      if (!prop) {
> +        error_setg(errp, "can't find %s address", name);
>          return -ENOENT;
>      }
>  
> @@ -108,13 +112,14 @@ static int fit_image_addr(const void *itb, int img, const char *name,
>          *addr = fdt64_to_cpu(*(fdt64_t *)prop);
>          return 0;
>      default:
> -        error_printf("invalid %s address length %d\n", name, len);
> +        error_setg(errp, "invalid %s address length %d", name, len);
>          return -EINVAL;
>      }
>  }
>  
>  static int fit_load_kernel(const struct fit_loader *ldr, const void *itb,
> -                           int cfg, void *opaque, hwaddr *pend)
> +                           int cfg, void *opaque, hwaddr *pend,
> +                           Error **errp)
>  {
>      const char *name;
>      const void *data;
> @@ -126,26 +131,26 @@ static int fit_load_kernel(const struct fit_loader *ldr, const void *itb,
>  
>      name = fdt_getprop(itb, cfg, "kernel", NULL);
>      if (!name) {
> -        error_printf("no kernel specified by FIT configuration\n");
> +        error_setg(errp, "no kernel specified by FIT configuration");
>          return -EINVAL;
>      }
>  
> -    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz);
> +    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
>      if (!data) {
> -        error_printf("unable to load kernel image from FIT\n");
> +        error_prepend(errp, "unable to load kernel image from FIT: ");
>          return -EINVAL;
>      }
>  
> -    err = fit_image_addr(itb, img_off, "load", &load_addr);
> +    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
>      if (err) {
> -        error_printf("unable to read kernel load address from FIT\n");
> +        error_prepend(errp, "unable to read kernel load address from FIT: ");
>          ret = err;
>          goto out;
>      }
>  
> -    err = fit_image_addr(itb, img_off, "entry", &entry_addr);
> +    err = fit_image_addr(itb, img_off, "entry", &entry_addr, errp);
>      if (err) {
> -        error_printf("unable to read kernel entry address from FIT\n");
> +        error_prepend(errp, "unable to read kernel entry address from FIT: ");
>          ret = err;
>          goto out;
>      }
> @@ -172,7 +177,7 @@ out:
>  
>  static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
>                          int cfg, void *opaque, const void *match_data,
> -                        hwaddr kernel_end)
> +                        hwaddr kernel_end, Error **errp)
>  {
>      const char *name;
>      const void *data;
> @@ -187,16 +192,18 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
>          return 0;
>      }
>  
> -    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz);
> +    load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
>      if (!data) {
> -        error_printf("unable to load FDT image from FIT\n");
> +        error_prepend(errp, "unable to load FDT image from FIT: ");
>          return -EINVAL;
>      }
>  
> -    err = fit_image_addr(itb, img_off, "load", &load_addr);
> +    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
>      if (err == -ENOENT) {
>          load_addr = ROUND_UP(kernel_end, 64 * KiB) + (10 * MiB);
> +        error_free(*errp);
>      } else if (err) {
> +        error_prepend(errp, "unable to read FDT load address from FIT: ");
>          ret = err;
>          goto out;
>      }
> @@ -229,7 +236,7 @@ static bool fit_cfg_compatible(const void *itb, int cfg, const char *compat)
>          return false;
>      }
>  
> -    fdt = fit_load_image_alloc(itb, fdt_name, NULL, NULL);
> +    fdt = fit_load_image_alloc(itb, fdt_name, NULL, NULL, NULL);
>      if (!fdt) {
>          return false;
>      }
> @@ -252,11 +259,12 @@ out:
>  
>  int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
>  {
> +    Error *err = NULL;
>      const struct fit_loader_match *match;
>      const void *itb, *match_data = NULL;
>      const char *def_cfg_name;
>      char path[FIT_LOADER_MAX_PATH];
> -    int itb_size, configs, cfg_off, off, err;
> +    int itb_size, configs, cfg_off, off;
>      hwaddr kernel_end;
>      int ret;
>  
> @@ -267,6 +275,7 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
>  
>      configs = fdt_path_offset(itb, "/configurations");
>      if (configs < 0) {
> +        error_report("can't find node /configurations");
>          ret = configs;
>          goto out;
>      }
> @@ -301,20 +310,21 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque)
>      }
>  
>      if (cfg_off < 0) {
> -        /* couldn't find a configuration to use */
> +        error_report("can't find configuration");
>          ret = cfg_off;
>          goto out;
>      }
>  
> -    err = fit_load_kernel(ldr, itb, cfg_off, opaque, &kernel_end);
> -    if (err) {
> -        ret = err;
> +    ret = fit_load_kernel(ldr, itb, cfg_off, opaque, &kernel_end, &err);
> +    if (ret) {
> +        error_report_err(err);
>          goto out;
>      }
>  
> -    err = fit_load_fdt(ldr, itb, cfg_off, opaque, match_data, kernel_end);
> -    if (err) {
> -        ret = err;
> +    ret = fit_load_fdt(ldr, itb, cfg_off, opaque, match_data, kernel_end,
> +                       &err);
> +    if (ret) {
> +        error_report_err(err);
>          goto out;
>      }

Very nice cleanup, thanks!

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[Qemu-devel] [PATCH 05/15] mips/boston: Report errors with error_report(), not error_printf()

[Markus Armbruster]

Cc: Paul Burton <pburton@wavecomp.com>
Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/mips/boston.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index e5bab3cadc..a8b29f62f5 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -528,21 +528,21 @@ static void boston_mach_init(MachineState *machine)
         fw_size = load_image_targphys(machine->firmware,
                                       0x1fc00000, 4 * MiB);
         if (fw_size == -1) {
-            error_printf("unable to load firmware image '%s'\n",
+            error_report("unable to load firmware image '%s'",
                           machine->firmware);
             exit(1);
         }
     } else if (machine->kernel_filename) {
         fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
         if (fit_err) {
-            error_printf("unable to load FIT image\n");
+            error_report("unable to load FIT image");
             exit(1);
         }
 
         gen_firmware(memory_region_get_ram_ptr(flash) + 0x7c00000,
                      s->kernel_entry, s->fdt_base, is_64b);
     } else if (!qtest_enabled()) {
-        error_printf("Please provide either a -kernel or -bios argument\n");
+        error_report("Please provide either a -kernel or -bios argument");
         exit(1);
     }
 }
-- 
2.17.2

[Qemu-devel] [PATCH 05/15] mips/boston: Report errors with error_report(), not error_printf()

[Markus Armbruster]

Cc: Paul Burton <pburton@wavecomp.com>
Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/mips/boston.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index e5bab3cadc..a8b29f62f5 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -528,21 +528,21 @@ static void boston_mach_init(MachineState *machine)
         fw_size = load_image_targphys(machine->firmware,
                                       0x1fc00000, 4 * MiB);
         if (fw_size == -1) {
-            error_printf("unable to load firmware image '%s'\n",
+            error_report("unable to load firmware image '%s'",
                           machine->firmware);
             exit(1);
         }
     } else if (machine->kernel_filename) {
         fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
         if (fit_err) {
-            error_printf("unable to load FIT image\n");
+            error_report("unable to load FIT image");
             exit(1);
         }
 
         gen_firmware(memory_region_get_ram_ptr(flash) + 0x7c00000,
                      s->kernel_entry, s->fdt_base, is_64b);
     } else if (!qtest_enabled()) {
-        error_printf("Please provide either a -kernel or -bios argument\n");
+        error_report("Please provide either a -kernel or -bios argument");
         exit(1);
     }
 }
-- 
2.17.2

Re: [Qemu-devel] [PATCH 05/15] mips/boston: Report errors with error_report(), not error_printf()

[Philippe Mathieu-Daudé]

On 4/8/19 10:36 AM, Markus Armbruster wrote:
> Cc: Paul Burton <pburton@wavecomp.com>
> Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/mips/boston.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index e5bab3cadc..a8b29f62f5 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -528,21 +528,21 @@ static void boston_mach_init(MachineState *machine)
>          fw_size = load_image_targphys(machine->firmware,
>                                        0x1fc00000, 4 * MiB);
>          if (fw_size == -1) {
> -            error_printf("unable to load firmware image '%s'\n",
> +            error_report("unable to load firmware image '%s'",
>                            machine->firmware);
>              exit(1);
>          }
>      } else if (machine->kernel_filename) {
>          fit_err = load_fit(&boston_fit_loader, machine->kernel_filename, s);
>          if (fit_err) {
> -            error_printf("unable to load FIT image\n");
> +            error_report("unable to load FIT image");
>              exit(1);
>          }
>  
>          gen_firmware(memory_region_get_ram_ptr(flash) + 0x7c00000,
>                       s->kernel_entry, s->fdt_base, is_64b);
>      } else if (!qtest_enabled()) {
> -        error_printf("Please provide either a -kernel or -bios argument\n");
> +        error_report("Please provide either a -kernel or -bios argument");
>          exit(1);
>      }
>  }
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[Qemu-devel] [PATCH 06/15] pci: Report fatal errors with error_report(), not error_printf()

[Markus Armbruster]

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 35451c1e99..a74995b596 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -927,7 +927,7 @@ static uint16_t pci_req_id_cache_extract(PCIReqIDCache *cache)
         result = PCI_BUILD_BDF(bus_n, 0);
         break;
     default:
-        error_printf("Invalid PCI requester ID cache type: %d\n",
+        error_report("Invalid PCI requester ID cache type: %d",
                      cache->type);
         exit(1);
         break;
-- 
2.17.2

[Qemu-devel] [PATCH 06/15] pci: Report fatal errors with error_report(), not error_printf()

[Markus Armbruster]

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 35451c1e99..a74995b596 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -927,7 +927,7 @@ static uint16_t pci_req_id_cache_extract(PCIReqIDCache *cache)
         result = PCI_BUILD_BDF(bus_n, 0);
         break;
     default:
-        error_printf("Invalid PCI requester ID cache type: %d\n",
+        error_report("Invalid PCI requester ID cache type: %d",
                      cache->type);
         exit(1);
         break;
-- 
2.17.2

Re: [Qemu-devel] [PATCH 06/15] pci: Report fatal errors with error_report(), not error_printf()

[Marcel Apfelbaum]

On 4/8/19 11:36 AM, Markus Armbruster wrote:
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hw/pci/pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 35451c1e99..a74995b596 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -927,7 +927,7 @@ static uint16_t pci_req_id_cache_extract(PCIReqIDCache *cache)
>           result = PCI_BUILD_BDF(bus_n, 0);
>           break;
>       default:
> -        error_printf("Invalid PCI requester ID cache type: %d\n",
> +        error_report("Invalid PCI requester ID cache type: %d",
>                        cache->type);
>           exit(1);
>           break;

Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>

Thanks,
Marcel

[Qemu-devel] [PATCH 07/15] hpet: Report warnings with warn_report(), not error_printf()

[Markus Armbruster]

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/timer/hpet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index d97436bc7b..41024f39fb 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -744,7 +744,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
     HPETTimer *timer;
 
     if (!s->intcap) {
-        error_printf("Hpet's intcap not initialized.\n");
+        warn_report("Hpet's intcap not initialized");
     }
     if (hpet_cfg.count == UINT8_MAX) {
         /* first instance */
-- 
2.17.2

[Qemu-devel] [PATCH 07/15] hpet: Report warnings with warn_report(), not error_printf()

[Markus Armbruster]

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/timer/hpet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index d97436bc7b..41024f39fb 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -744,7 +744,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
     HPETTimer *timer;
 
     if (!s->intcap) {
-        error_printf("Hpet's intcap not initialized.\n");
+        warn_report("Hpet's intcap not initialized");
     }
     if (hpet_cfg.count == UINT8_MAX) {
         /* first instance */
-- 
2.17.2

[Qemu-devel] [PATCH 08/15] vfio: Report warnings with warn_report(), not error_printf()

[Markus Armbruster]

Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/vfio/pci.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 504019c458..0142819ea6 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -947,8 +947,10 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
     if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
         /* Since pci handles romfile, just print a message and return */
         if (vfio_blacklist_opt_rom(vdev) && vdev->pdev.romfile) {
-            error_printf("Warning : Device at %s is known to cause system instability issues during option rom execution. Proceeding anyway since user specified romfile\n",
-                         vdev->vbasedev.name);
+            warn_report("Device at %s is known to cause system instability"
+                        " issues during option rom execution",
+                        vdev->vbasedev.name);
+            error_printf("Proceeding anyway since user specified romfile\n");
         }
         return;
     }
@@ -973,11 +975,16 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
 
     if (vfio_blacklist_opt_rom(vdev)) {
         if (dev->opts && qemu_opt_get(dev->opts, "rombar")) {
-            error_printf("Warning : Device at %s is known to cause system instability issues during option rom execution. Proceeding anyway since user specified non zero value for rombar\n",
-                         vdev->vbasedev.name);
+            warn_report("Device at %s is known to cause system instability"
+                        " issues during option rom execution",
+                        vdev->vbasedev.name);
+            error_printf("Proceeding anyway since user specified"
+                         " non zero value for rombar\n");
         } else {
-            error_printf("Warning : Rom loading for device at %s has been disabled due to system instability issues. Specify rombar=1 or romfile to force\n",
-                         vdev->vbasedev.name);
+            warn_report("Rom loading for device at %s has been disabled"
+                        " due to system instability issues",
+                        vdev->vbasedev.name);
+            error_printf("Specify rombar=1 or romfile to force\n");
             return;
         }
     }
-- 
2.17.2

[Qemu-devel] [PATCH 09/15] s390x/kvm: Report warnings with warn_report(), not error_printf()

[Markus Armbruster]

kvm_s390_mem_op() can fail in two ways: when !cap_mem_op, it returns
-ENOSYS, and when kvm_vcpu_ioctl() fails, it returns -errno set by
ioctl().  Its caller s390_cpu_virt_mem_rw() recovers from both
failures.

kvm_s390_mem_op() prints "KVM_S390_MEM_OP failed" with error_printf()
in the latter failure mode.  Since this is obviously a warning, use
warn_report().

Perhaps the reporting should be left to the caller.  It could warn on
failure other than -ENOSYS.

Cc: Thomas Huth <thuth@redhat.com>
Cc: qemu-s390x@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 target/s390x/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 19530fb94e..2c6e35b5aa 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -782,7 +782,7 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,
 
     ret = kvm_vcpu_ioctl(CPU(cpu), KVM_S390_MEM_OP, &mem_op);
     if (ret < 0) {
-        error_printf("KVM_S390_MEM_OP failed: %s\n", strerror(-ret));
+        warn_report("KVM_S390_MEM_OP failed: %s", strerror(-ret));
     }
     return ret;
 }
-- 
2.17.2

Re: [Qemu-devel] [PATCH 09/15] s390x/kvm: Report warnings with warn_report(), not error_printf()

[Thomas Huth]

On 08/04/2019 10.36, Markus Armbruster wrote:
> kvm_s390_mem_op() can fail in two ways: when !cap_mem_op, it returns
> -ENOSYS, and when kvm_vcpu_ioctl() fails, it returns -errno set by
> ioctl().  Its caller s390_cpu_virt_mem_rw() recovers from both
> failures.
> 
> kvm_s390_mem_op() prints "KVM_S390_MEM_OP failed" with error_printf()
> in the latter failure mode.  Since this is obviously a warning, use
> warn_report().
> 
> Perhaps the reporting should be left to the caller.  It could warn on
> failure other than -ENOSYS.
> 
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: qemu-s390x@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  target/s390x/kvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 19530fb94e..2c6e35b5aa 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -782,7 +782,7 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,
>  
>      ret = kvm_vcpu_ioctl(CPU(cpu), KVM_S390_MEM_OP, &mem_op);
>      if (ret < 0) {
> -        error_printf("KVM_S390_MEM_OP failed: %s\n", strerror(-ret));
> +        warn_report("KVM_S390_MEM_OP failed: %s", strerror(-ret));
>      }
>      return ret;
>  }
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 09/15] s390x/kvm: Report warnings with warn_report(), not error_printf()

[Cornelia Huck]

On Mon, 8 Apr 2019 10:39:40 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 08/04/2019 10.36, Markus Armbruster wrote:
> > kvm_s390_mem_op() can fail in two ways: when !cap_mem_op, it returns
> > -ENOSYS, and when kvm_vcpu_ioctl() fails, it returns -errno set by
> > ioctl().  Its caller s390_cpu_virt_mem_rw() recovers from both
> > failures.
> > 
> > kvm_s390_mem_op() prints "KVM_S390_MEM_OP failed" with error_printf()
> > in the latter failure mode.  Since this is obviously a warning, use
> > warn_report().
> > 
> > Perhaps the reporting should be left to the caller.  It could warn on
> > failure other than -ENOSYS.
> > 
> > Cc: Thomas Huth <thuth@redhat.com>
> > Cc: qemu-s390x@nongnu.org
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
> >  target/s390x/kvm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> > index 19530fb94e..2c6e35b5aa 100644
> > --- a/target/s390x/kvm.c
> > +++ b/target/s390x/kvm.c
> > @@ -782,7 +782,7 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,
> >  
> >      ret = kvm_vcpu_ioctl(CPU(cpu), KVM_S390_MEM_OP, &mem_op);
> >      if (ret < 0) {
> > -        error_printf("KVM_S390_MEM_OP failed: %s\n", strerror(-ret));
> > +        warn_report("KVM_S390_MEM_OP failed: %s", strerror(-ret));
> >      }
> >      return ret;
> >  }
> >   
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

(Markus, I think you wanted to take this?)

Re: [Qemu-devel] [PATCH 09/15] s390x/kvm: Report warnings with warn_report(), not error_printf()

[Cornelia Huck]

On Mon, 8 Apr 2019 10:39:40 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 08/04/2019 10.36, Markus Armbruster wrote:
> > kvm_s390_mem_op() can fail in two ways: when !cap_mem_op, it returns
> > -ENOSYS, and when kvm_vcpu_ioctl() fails, it returns -errno set by
> > ioctl().  Its caller s390_cpu_virt_mem_rw() recovers from both
> > failures.
> > 
> > kvm_s390_mem_op() prints "KVM_S390_MEM_OP failed" with error_printf()
> > in the latter failure mode.  Since this is obviously a warning, use
> > warn_report().
> > 
> > Perhaps the reporting should be left to the caller.  It could warn on
> > failure other than -ENOSYS.
> > 
> > Cc: Thomas Huth <thuth@redhat.com>
> > Cc: qemu-s390x@nongnu.org
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
> >  target/s390x/kvm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> > index 19530fb94e..2c6e35b5aa 100644
> > --- a/target/s390x/kvm.c
> > +++ b/target/s390x/kvm.c
> > @@ -782,7 +782,7 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,
> >  
> >      ret = kvm_vcpu_ioctl(CPU(cpu), KVM_S390_MEM_OP, &mem_op);
> >      if (ret < 0) {
> > -        error_printf("KVM_S390_MEM_OP failed: %s\n", strerror(-ret));
> > +        warn_report("KVM_S390_MEM_OP failed: %s", strerror(-ret));
> >      }
> >      return ret;
> >  }
> >   
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

(Markus, I think you wanted to take this?)

Re: [Qemu-devel] [PATCH 09/15] s390x/kvm: Report warnings with warn_report(), not error_printf()

[Markus Armbruster]

Cornelia Huck <cohuck@redhat.com> writes:

> On Mon, 8 Apr 2019 10:39:40 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> On 08/04/2019 10.36, Markus Armbruster wrote:
>> > kvm_s390_mem_op() can fail in two ways: when !cap_mem_op, it returns
>> > -ENOSYS, and when kvm_vcpu_ioctl() fails, it returns -errno set by
>> > ioctl().  Its caller s390_cpu_virt_mem_rw() recovers from both
>> > failures.
>> > 
>> > kvm_s390_mem_op() prints "KVM_S390_MEM_OP failed" with error_printf()
>> > in the latter failure mode.  Since this is obviously a warning, use
>> > warn_report().
>> > 
>> > Perhaps the reporting should be left to the caller.  It could warn on
>> > failure other than -ENOSYS.
>> > 
>> > Cc: Thomas Huth <thuth@redhat.com>
>> > Cc: qemu-s390x@nongnu.org
>> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> > ---
>> >  target/s390x/kvm.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> > index 19530fb94e..2c6e35b5aa 100644
>> > --- a/target/s390x/kvm.c
>> > +++ b/target/s390x/kvm.c
>> > @@ -782,7 +782,7 @@ int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,
>> >  
>> >      ret = kvm_vcpu_ioctl(CPU(cpu), KVM_S390_MEM_OP, &mem_op);
>> >      if (ret < 0) {
>> > -        error_printf("KVM_S390_MEM_OP failed: %s\n", strerror(-ret));
>> > +        warn_report("KVM_S390_MEM_OP failed: %s", strerror(-ret));
>> >      }
>> >      return ret;
>> >  }
>> >   
>> 
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>
> (Markus, I think you wanted to take this?)

I intend to take the complete series through my tree if nobody objects.

Thanks!

[Qemu-devel] [PATCH 10/15] vl: Make -machine $TYPE, help and -accel help print to stdout

[Markus Armbruster]

Command line help help explicitly requested by the user should be
printed to stdout, not stderr.  We do elsewhere.  Adjust -machine
$TYPE,help and -accel help to match: use printf() instead of
error_printf().

Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 vl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/vl.c b/vl.c
index c696ad2a13..792ef36001 100644
--- a/vl.c
+++ b/vl.c
@@ -1556,12 +1556,12 @@ static int machine_help_func(QemuOpts *opts, MachineState *machine)
             continue;
         }
 
-        error_printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name,
-                     prop->name, prop->type);
+        printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name,
+                    prop->name, prop->type);
         if (prop->description) {
-            error_printf(" (%s)\n", prop->description);
+            printf(" (%s)\n", prop->description);
         } else {
-            error_printf("\n");
+            printf("\n");
         }
     }
 
@@ -3643,7 +3643,7 @@ int main(int argc, char **argv, char **envp)
                                                      optarg, true);
                 optarg = qemu_opt_get(accel_opts, "accel");
                 if (!optarg || is_help_option(optarg)) {
-                    error_printf("Possible accelerators: kvm, xen, hax, tcg\n");
+                    printf("Possible accelerators: kvm, xen, hax, tcg\n");
                     exit(0);
                 }
                 opts = qemu_opts_create(qemu_find_opts("machine"), NULL,
-- 
2.17.2

Re: [Qemu-devel] [PATCH 10/15] vl: Make -machine $TYPE, help and -accel help print to stdout

[Marcel Apfelbaum]

Hi Markus,

On 4/8/19 11:36 AM, Markus Armbruster wrote:
> Command line help help explicitly requested by the user should be
> printed to stdout, not stderr.  We do elsewhere.  Adjust -machine
> $TYPE,help and -accel help to match: use printf() instead of
> error_printf().
>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   vl.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index c696ad2a13..792ef36001 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1556,12 +1556,12 @@ static int machine_help_func(QemuOpts *opts, MachineState *machine)
>               continue;
>           }
>   
> -        error_printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name,
> -                     prop->name, prop->type);
> +        printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name,
> +                    prop->name, prop->type);

Does the line above have an alignment issue?

Anyway,

Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>

Thanks,
Marcel

>           if (prop->description) {
> -            error_printf(" (%s)\n", prop->description);
> +            printf(" (%s)\n", prop->description);
>           } else {
> -            error_printf("\n");
> +            printf("\n");
>           }
>       }
>   
> @@ -3643,7 +3643,7 @@ int main(int argc, char **argv, char **envp)
>                                                        optarg, true);
>                   optarg = qemu_opt_get(accel_opts, "accel");
>                   if (!optarg || is_help_option(optarg)) {
> -                    error_printf("Possible accelerators: kvm, xen, hax, tcg\n");
> +                    printf("Possible accelerators: kvm, xen, hax, tcg\n");
>                       exit(0);
>                   }
>                   opts = qemu_opts_create(qemu_find_opts("machine"), NULL,

Re: [Qemu-devel] [PATCH 10/15] vl: Make -machine $TYPE, help and -accel help print to stdout

[Markus Armbruster]

Marcel Apfelbaum <marcel.apfelbaum@gmail.com> writes:

> Hi Markus,
>
> On 4/8/19 11:36 AM, Markus Armbruster wrote:
>> Command line help help explicitly requested by the user should be
>> printed to stdout, not stderr.  We do elsewhere.  Adjust -machine
>> $TYPE,help and -accel help to match: use printf() instead of
>> error_printf().
>>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   vl.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index c696ad2a13..792ef36001 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1556,12 +1556,12 @@ static int machine_help_func(QemuOpts *opts, MachineState *machine)
>>               continue;
>>           }
>>   -        error_printf("%s.%s=%s",
>> MACHINE_GET_CLASS(machine)->name,
>> -                     prop->name, prop->type);
>> +        printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name,
>> +                    prop->name, prop->type);
>
> Does the line above have an alignment issue?

Oops!  Will fix.

> Anyway,
>
> Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>

Thank you!

[Qemu-devel] [PATCH 11/15] monitor error: Make printf()-like functions return a value

[Markus Armbruster]

printf() & friends return the number of characters written on success,
negative value on error.

monitor_printf(), monitor_vfprintf(), monitor_vprintf(),
error_printf(), error_printf_unless_qmp(), error_vprintf(), and
error_vprintf_unless_qmp() return void.  Some of them carry a TODO
comment asking for int instead.

Improve them to return int like printf() does.

This makes our use of monitor_printf() as fprintf_function slightly
less dirty: the function cast no longer adds a return value that isn't
there.  It still changes a parameter's pointer type.  That will be
addressed in a future commit.

monitor_vfprintf() always returns zero.  Improve it to return the
proper value.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/monitor/monitor.h   |  8 ++---
 include/qemu/error-report.h |  8 ++---
 monitor.c                   | 61 ++++++++++++++++++++-----------------
 stubs/error-printf.c        | 10 +++---
 util/qemu-error.c           | 12 +++++---
 5 files changed, 54 insertions(+), 45 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index c1b40a9cac..e4c3717454 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -28,9 +28,9 @@ void monitor_resume(Monitor *mon);
 int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
 int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp);
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
     GCC_FMT_ATTR(2, 0);
-void monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+int monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 int monitor_fprintf(FILE *stream, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void monitor_flush(Monitor *mon);
 int monitor_set_cpu(int cpu_index);
@@ -48,7 +48,7 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
 void monitor_fdset_dup_fd_remove(int dup_fd);
 int monitor_fdset_dup_fd_find(int dup_fd);
 
-void monitor_vfprintf(FILE *stream,
-                      const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
+int monitor_vfprintf(FILE *stream,
+                     const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
 
 #endif /* MONITOR_H */
diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index 0a8d9cc9ea..5db4e5db0d 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -30,10 +30,10 @@ void loc_set_none(void);
 void loc_set_cmdline(char **argv, int idx, int cnt);
 void loc_set_file(const char *fname, int lno);
 
-void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
-void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
-void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
-void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
+int error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
+int error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
+int error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
+int error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 void error_set_progname(const char *argv0);
 
 void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
diff --git a/monitor.c b/monitor.c
index 4807bbe811..7b4a78d798 100644
--- a/monitor.c
+++ b/monitor.c
@@ -430,15 +430,14 @@ void monitor_flush(Monitor *mon)
 }
 
 /* flush at every end of line */
-static void monitor_puts(Monitor *mon, const char *str)
+static int monitor_puts(Monitor *mon, const char *str)
 {
+    int i;
     char c;
 
     qemu_mutex_lock(&mon->mon_lock);
-    for(;;) {
-        c = *str++;
-        if (c == '\0')
-            break;
+    for (i = 0; str[i]; i++) {
+        c = str[i];
         if (c == '\n') {
             qstring_append_chr(mon->outbuf, '\r');
         }
@@ -448,39 +447,48 @@ static void monitor_puts(Monitor *mon, const char *str)
         }
     }
     qemu_mutex_unlock(&mon->mon_lock);
+
+    return i;
 }
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 {
     char *buf;
+    int n;
 
     if (!mon)
-        return;
+        return -1;
 
     if (monitor_is_qmp(mon)) {
-        return;
+        return -1;
     }
 
     buf = g_strdup_vprintf(fmt, ap);
-    monitor_puts(mon, buf);
+    n = monitor_puts(mon, buf);
     g_free(buf);
+    return n;
 }
 
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_printf(Monitor *mon, const char *fmt, ...)
 {
+    int ret;
+
     va_list ap;
     va_start(ap, fmt);
-    monitor_vprintf(mon, fmt, ap);
+    ret = monitor_vprintf(mon, fmt, ap);
     va_end(ap);
+    return ret;
 }
 
 int monitor_fprintf(FILE *stream, const char *fmt, ...)
 {
+    int ret;
+
     va_list ap;
     va_start(ap, fmt);
-    monitor_vprintf((Monitor *)stream, fmt, ap);
+    ret = monitor_vprintf((Monitor *)stream, fmt, ap);
     va_end(ap);
-    return 0;
+    return ret;
 }
 
 static void qmp_send_response(Monitor *mon, const QDict *rsp)
@@ -4535,35 +4543,32 @@ static void monitor_readline_flush(void *opaque)
 
 /*
  * Print to current monitor if we have one, else to stream.
- * TODO should return int, so callers can calculate width, but that
- * requires surgery to monitor_vprintf().  Left for another day.
  */
-void monitor_vfprintf(FILE *stream, const char *fmt, va_list ap)
+int monitor_vfprintf(FILE *stream, const char *fmt, va_list ap)
 {
     if (cur_mon && !monitor_cur_is_qmp()) {
-        monitor_vprintf(cur_mon, fmt, ap);
-    } else {
-        vfprintf(stream, fmt, ap);
+        return monitor_vprintf(cur_mon, fmt, ap);
     }
+    return vfprintf(stream, fmt, ap);
 }
 
 /*
  * Print to current monitor if we have one, else to stderr.
- * TODO should return int, so callers can calculate width, but that
- * requires surgery to monitor_vprintf().  Left for another day.
  */
-void error_vprintf(const char *fmt, va_list ap)
+int error_vprintf(const char *fmt, va_list ap)
 {
-    monitor_vfprintf(stderr, fmt, ap);
+    return monitor_vfprintf(stderr, fmt, ap);
 }
 
-void error_vprintf_unless_qmp(const char *fmt, va_list ap)
+int error_vprintf_unless_qmp(const char *fmt, va_list ap)
 {
-    if (cur_mon && !monitor_cur_is_qmp()) {
-        monitor_vprintf(cur_mon, fmt, ap);
-    } else if (!cur_mon) {
-        vfprintf(stderr, fmt, ap);
+    if (!cur_mon) {
+        return vfprintf(stderr, fmt, ap);
     }
+    if (!monitor_cur_is_qmp()) {
+        return monitor_vprintf(cur_mon, fmt, ap);
+    }
+    return -1;
 }
 
 static void monitor_list_append(Monitor *mon)
diff --git a/stubs/error-printf.c b/stubs/error-printf.c
index 99c6406668..537298209d 100644
--- a/stubs/error-printf.c
+++ b/stubs/error-printf.c
@@ -2,19 +2,19 @@
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 
-void error_vprintf(const char *fmt, va_list ap)
+int error_vprintf(const char *fmt, va_list ap)
 {
     if (g_test_initialized() && !g_test_subprocess() &&
         getenv("QTEST_SILENT_ERRORS")) {
         char *msg = g_strdup_vprintf(fmt, ap);
         g_test_message("%s", msg);
         g_free(msg);
-    } else {
-        vfprintf(stderr, fmt, ap);
+        return strlen(msg);
     }
+    return vfprintf(stderr, fmt, ap);
 }
 
-void error_vprintf_unless_qmp(const char *fmt, va_list ap)
+int error_vprintf_unless_qmp(const char *fmt, va_list ap)
 {
-    error_vprintf(fmt, ap);
+    return error_vprintf(fmt, ap);
 }
diff --git a/util/qemu-error.c b/util/qemu-error.c
index fcbe8a1f74..36eb3ea239 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -24,22 +24,26 @@ typedef enum {
     REPORT_TYPE_INFO,
 } report_type;
 
-void error_printf(const char *fmt, ...)
+int error_printf(const char *fmt, ...)
 {
     va_list ap;
+    int ret;
 
     va_start(ap, fmt);
-    error_vprintf(fmt, ap);
+    ret = error_vprintf(fmt, ap);
     va_end(ap);
+    return ret;
 }
 
-void error_printf_unless_qmp(const char *fmt, ...)
+int error_printf_unless_qmp(const char *fmt, ...)
 {
     va_list ap;
+    int ret;
 
     va_start(ap, fmt);
-    error_vprintf_unless_qmp(fmt, ap);
+    ret = error_vprintf_unless_qmp(fmt, ap);
     va_end(ap);
+    return ret;
 }
 
 static Location std_loc = {
-- 
2.17.2

Re: [Qemu-devel] [PATCH 11/15] monitor error: Make printf()-like functions return a value

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> printf() & friends return the number of characters written on success,
> negative value on error.
>
> monitor_printf(), monitor_vfprintf(), monitor_vprintf(),
> error_printf(), error_printf_unless_qmp(), error_vprintf(), and
> error_vprintf_unless_qmp() return void.  Some of them carry a TODO
> comment asking for int instead.
>
> Improve them to return int like printf() does.
>
> This makes our use of monitor_printf() as fprintf_function slightly
> less dirty: the function cast no longer adds a return value that isn't
> there.  It still changes a parameter's pointer type.  That will be
> addressed in a future commit.
>
> monitor_vfprintf() always returns zero.  Improve it to return the
> proper value.
>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
[...]
> diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> index 99c6406668..537298209d 100644
> --- a/stubs/error-printf.c
> +++ b/stubs/error-printf.c
> @@ -2,19 +2,19 @@
>  #include "qemu-common.h"
>  #include "qemu/error-report.h"
>  
> -void error_vprintf(const char *fmt, va_list ap)
> +int error_vprintf(const char *fmt, va_list ap)
>  {
>      if (g_test_initialized() && !g_test_subprocess() &&
>          getenv("QTEST_SILENT_ERRORS")) {
>          char *msg = g_strdup_vprintf(fmt, ap);
>          g_test_message("%s", msg);
>          g_free(msg);
> -    } else {
> -        vfprintf(stderr, fmt, ap);
> +        return strlen(msg);

Use after free.  Obvious fixup appended.

>      }
> +    return vfprintf(stderr, fmt, ap);
>  }
>  
> -void error_vprintf_unless_qmp(const char *fmt, va_list ap)
> +int error_vprintf_unless_qmp(const char *fmt, va_list ap)
>  {
> -    error_vprintf(fmt, ap);
> +    return error_vprintf(fmt, ap);
>  }
[...]

diff --git a/stubs/error-printf.c b/stubs/error-printf.c
index 537298209d..1f9d3b3714 100644
--- a/stubs/error-printf.c
+++ b/stubs/error-printf.c
@@ -4,12 +4,15 @@
 
 int error_vprintf(const char *fmt, va_list ap)
 {
+    int ret;
+
     if (g_test_initialized() && !g_test_subprocess() &&
         getenv("QTEST_SILENT_ERRORS")) {
         char *msg = g_strdup_vprintf(fmt, ap);
         g_test_message("%s", msg);
+        ret = strlen(msg);
         g_free(msg);
-        return strlen(msg);
+        return ret;
     }
     return vfprintf(stderr, fmt, ap);
 }

Re: [Qemu-devel] [PATCH 11/15] monitor error: Make printf()-like functions return a value

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
> > printf() & friends return the number of characters written on success,
> > negative value on error.
> >
> > monitor_printf(), monitor_vfprintf(), monitor_vprintf(),
> > error_printf(), error_printf_unless_qmp(), error_vprintf(), and
> > error_vprintf_unless_qmp() return void.  Some of them carry a TODO
> > comment asking for int instead.
> >
> > Improve them to return int like printf() does.
> >
> > This makes our use of monitor_printf() as fprintf_function slightly
> > less dirty: the function cast no longer adds a return value that isn't
> > there.  It still changes a parameter's pointer type.  That will be
> > addressed in a future commit.
> >
> > monitor_vfprintf() always returns zero.  Improve it to return the
> > proper value.
> >
> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
> [...]
> > diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> > index 99c6406668..537298209d 100644
> > --- a/stubs/error-printf.c
> > +++ b/stubs/error-printf.c
> > @@ -2,19 +2,19 @@
> >  #include "qemu-common.h"
> >  #include "qemu/error-report.h"
> >  
> > -void error_vprintf(const char *fmt, va_list ap)
> > +int error_vprintf(const char *fmt, va_list ap)
> >  {
> >      if (g_test_initialized() && !g_test_subprocess() &&
> >          getenv("QTEST_SILENT_ERRORS")) {
> >          char *msg = g_strdup_vprintf(fmt, ap);
> >          g_test_message("%s", msg);
> >          g_free(msg);
> > -    } else {
> > -        vfprintf(stderr, fmt, ap);
> > +        return strlen(msg);
> 
> Use after free.  Obvious fixup appended.
> 
> >      }
> > +    return vfprintf(stderr, fmt, ap);
> >  }
> >  
> > -void error_vprintf_unless_qmp(const char *fmt, va_list ap)
> > +int error_vprintf_unless_qmp(const char *fmt, va_list ap)
> >  {
> > -    error_vprintf(fmt, ap);
> > +    return error_vprintf(fmt, ap);
> >  }
> [...]
> 
> diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> index 537298209d..1f9d3b3714 100644
> --- a/stubs/error-printf.c
> +++ b/stubs/error-printf.c
> @@ -4,12 +4,15 @@
>  
>  int error_vprintf(const char *fmt, va_list ap)
>  {
> +    int ret;
> +
>      if (g_test_initialized() && !g_test_subprocess() &&
>          getenv("QTEST_SILENT_ERRORS")) {
>          char *msg = g_strdup_vprintf(fmt, ap);
>          g_test_message("%s", msg);
> +        ret = strlen(msg);
>          g_free(msg);
> -        return strlen(msg);
> +        return ret;
>      }
>      return vfprintf(stderr, fmt, ap);
>  }

Yeh, with the fixup:

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH 12/15] qemu-print: New qemu_printf(), qemu_vprintf() etc.

[Markus Armbruster]

We commonly want to print to the current monitor if we have one, else
to stdout/stderr.  For stderr, have error_printf().  For stdout, all
we have is monitor_vfprintf(), which is rather unwieldy.  We often
print to stderr just because error_printf() is easier.

New qemu_printf() and qemu_vprintf() do exactly what's needed.  The
next commits will put them to use.

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 MAINTAINERS               |  2 ++
 include/qemu/qemu-print.h | 19 ++++++++++++++++++
 stubs/monitor.c           |  5 +++++
 util/Makefile.objs        |  1 +
 util/qemu-print.c         | 42 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 69 insertions(+)
 create mode 100644 include/qemu/qemu-print.h
 create mode 100644 util/qemu-print.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 56139ac8ab..1aa19dc4ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1895,6 +1895,8 @@ F: hmp.[ch]
 F: hmp-commands*.hx
 F: include/monitor/hmp-target.h
 F: tests/test-hmp.c
+F: include/qemu/qemu-print.h
+F: util/qemu-print.c
 
 Network device backends
 M: Jason Wang <jasowang@redhat.com>
diff --git a/include/qemu/qemu-print.h b/include/qemu/qemu-print.h
new file mode 100644
index 0000000000..8fed32bf42
--- /dev/null
+++ b/include/qemu/qemu-print.h
@@ -0,0 +1,19 @@
+/*
+ * Print to stream or current monitor
+ *
+ * Copyright (C) 2019 Red Hat Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_PRINT_H
+#define QEMU_PRINT_H
+
+int qemu_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
+int qemu_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
+
+#endif
diff --git a/stubs/monitor.c b/stubs/monitor.c
index b57fe6c32f..b2ea975e40 100644
--- a/stubs/monitor.c
+++ b/stubs/monitor.c
@@ -6,6 +6,11 @@
 
 __thread Monitor *cur_mon;
 
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+{
+    abort();
+}
+
 int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
 {
     error_setg(errp, "only QEMU supports file descriptor passing");
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 835fcd69e2..9206878dec 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -23,6 +23,7 @@ util-obj-y += bitmap.o bitops.o hbitmap.o
 util-obj-y += fifo8.o
 util-obj-y += cacheinfo.o
 util-obj-y += error.o qemu-error.o
+util-obj-y += qemu-print.o
 util-obj-y += id.o
 util-obj-y += iov.o qemu-config.o qemu-sockets.o uri.o notify.o
 util-obj-y += qemu-option.o qemu-progress.o
diff --git a/util/qemu-print.c b/util/qemu-print.c
new file mode 100644
index 0000000000..86f9417af8
--- /dev/null
+++ b/util/qemu-print.c
@@ -0,0 +1,42 @@
+/*
+ * Print to stream or current monitor
+ *
+ * Copyright (C) 2019 Red Hat Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "monitor/monitor.h"
+#include "qemu/qemu-print.h"
+
+/*
+ * Print like vprintf().
+ * Print to current monitor if we have one, else to stdout.
+ */
+int qemu_vprintf(const char *fmt, va_list ap)
+{
+    if (cur_mon) {
+        return monitor_vprintf(cur_mon, fmt, ap);
+    }
+    return vprintf(fmt, ap);
+}
+
+/*
+ * Print like printf().
+ * Print to current monitor if we have one, else to stdout.
+ */
+int qemu_printf(const char *fmt, ...)
+{
+    va_list ap;
+    int ret;
+
+    va_start(ap, fmt);
+    ret = qemu_vprintf(fmt, ap);
+    va_end(ap);
+    return ret;
+}
-- 
2.17.2

Re: [Qemu-devel] [PATCH 12/15] qemu-print: New qemu_printf(), qemu_vprintf() etc.

[Philippe Mathieu-Daudé]

On 4/8/19 10:36 AM, Markus Armbruster wrote:
> We commonly want to print to the current monitor if we have one, else
> to stdout/stderr.  For stderr, have error_printf().  For stdout, all
> we have is monitor_vfprintf(), which is rather unwieldy.  We often
> print to stderr just because error_printf() is easier.
> 
> New qemu_printf() and qemu_vprintf() do exactly what's needed.  The
> next commits will put them to use.
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  MAINTAINERS               |  2 ++
>  include/qemu/qemu-print.h | 19 ++++++++++++++++++
>  stubs/monitor.c           |  5 +++++
>  util/Makefile.objs        |  1 +
>  util/qemu-print.c         | 42 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 69 insertions(+)
>  create mode 100644 include/qemu/qemu-print.h
>  create mode 100644 util/qemu-print.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 56139ac8ab..1aa19dc4ef 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1895,6 +1895,8 @@ F: hmp.[ch]
>  F: hmp-commands*.hx
>  F: include/monitor/hmp-target.h
>  F: tests/test-hmp.c
> +F: include/qemu/qemu-print.h
> +F: util/qemu-print.c
>  
>  Network device backends
>  M: Jason Wang <jasowang@redhat.com>
> diff --git a/include/qemu/qemu-print.h b/include/qemu/qemu-print.h
> new file mode 100644
> index 0000000000..8fed32bf42
> --- /dev/null
> +++ b/include/qemu/qemu-print.h
> @@ -0,0 +1,19 @@
> +/*
> + * Print to stream or current monitor
> + *
> + * Copyright (C) 2019 Red Hat Inc.
> + *
> + * Authors:
> + *  Markus Armbruster <armbru@redhat.com>,
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_PRINT_H
> +#define QEMU_PRINT_H
> +
> +int qemu_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
> +int qemu_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +
> +#endif
> diff --git a/stubs/monitor.c b/stubs/monitor.c
> index b57fe6c32f..b2ea975e40 100644
> --- a/stubs/monitor.c
> +++ b/stubs/monitor.c
> @@ -6,6 +6,11 @@
>  
>  __thread Monitor *cur_mon;
>  
> +int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
> +{
> +    abort();
> +}
> +
>  int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
>  {
>      error_setg(errp, "only QEMU supports file descriptor passing");
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 835fcd69e2..9206878dec 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -23,6 +23,7 @@ util-obj-y += bitmap.o bitops.o hbitmap.o
>  util-obj-y += fifo8.o
>  util-obj-y += cacheinfo.o
>  util-obj-y += error.o qemu-error.o
> +util-obj-y += qemu-print.o
>  util-obj-y += id.o
>  util-obj-y += iov.o qemu-config.o qemu-sockets.o uri.o notify.o
>  util-obj-y += qemu-option.o qemu-progress.o
> diff --git a/util/qemu-print.c b/util/qemu-print.c
> new file mode 100644
> index 0000000000..86f9417af8
> --- /dev/null
> +++ b/util/qemu-print.c
> @@ -0,0 +1,42 @@
> +/*
> + * Print to stream or current monitor
> + *
> + * Copyright (C) 2019 Red Hat Inc.
> + *
> + * Authors:
> + *  Markus Armbruster <armbru@redhat.com>,
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "monitor/monitor.h"
> +#include "qemu/qemu-print.h"
> +
> +/*
> + * Print like vprintf().
> + * Print to current monitor if we have one, else to stdout.
> + */
> +int qemu_vprintf(const char *fmt, va_list ap)
> +{
> +    if (cur_mon) {
> +        return monitor_vprintf(cur_mon, fmt, ap);
> +    }
> +    return vprintf(fmt, ap);
> +}
> +
> +/*
> + * Print like printf().
> + * Print to current monitor if we have one, else to stdout.
> + */
> +int qemu_printf(const char *fmt, ...)
> +{
> +    va_list ap;
> +    int ret;
> +
> +    va_start(ap, fmt);
> +    ret = qemu_vprintf(fmt, ap);
> +    va_end(ap);
> +    return ret;
> +}
> 

Re: [Qemu-devel] [PATCH 12/15] qemu-print: New qemu_printf(), qemu_vprintf() etc.

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> We commonly want to print to the current monitor if we have one, else
> to stdout/stderr.  For stderr, have error_printf().  For stdout, all
> we have is monitor_vfprintf(), which is rather unwieldy.  We often
> print to stderr just because error_printf() is easier.
> 
> New qemu_printf() and qemu_vprintf() do exactly what's needed.  The
> next commits will put them to use.
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

OK, it seems a little odd not to have the fprintf as well, but OK.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  MAINTAINERS               |  2 ++
>  include/qemu/qemu-print.h | 19 ++++++++++++++++++
>  stubs/monitor.c           |  5 +++++
>  util/Makefile.objs        |  1 +
>  util/qemu-print.c         | 42 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 69 insertions(+)
>  create mode 100644 include/qemu/qemu-print.h
>  create mode 100644 util/qemu-print.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 56139ac8ab..1aa19dc4ef 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1895,6 +1895,8 @@ F: hmp.[ch]
>  F: hmp-commands*.hx
>  F: include/monitor/hmp-target.h
>  F: tests/test-hmp.c
> +F: include/qemu/qemu-print.h
> +F: util/qemu-print.c
>  
>  Network device backends
>  M: Jason Wang <jasowang@redhat.com>
> diff --git a/include/qemu/qemu-print.h b/include/qemu/qemu-print.h
> new file mode 100644
> index 0000000000..8fed32bf42
> --- /dev/null
> +++ b/include/qemu/qemu-print.h
> @@ -0,0 +1,19 @@
> +/*
> + * Print to stream or current monitor
> + *
> + * Copyright (C) 2019 Red Hat Inc.
> + *
> + * Authors:
> + *  Markus Armbruster <armbru@redhat.com>,
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_PRINT_H
> +#define QEMU_PRINT_H
> +
> +int qemu_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
> +int qemu_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> +
> +#endif
> diff --git a/stubs/monitor.c b/stubs/monitor.c
> index b57fe6c32f..b2ea975e40 100644
> --- a/stubs/monitor.c
> +++ b/stubs/monitor.c
> @@ -6,6 +6,11 @@
>  
>  __thread Monitor *cur_mon;
>  
> +int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
> +{
> +    abort();
> +}
> +
>  int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
>  {
>      error_setg(errp, "only QEMU supports file descriptor passing");
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 835fcd69e2..9206878dec 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -23,6 +23,7 @@ util-obj-y += bitmap.o bitops.o hbitmap.o
>  util-obj-y += fifo8.o
>  util-obj-y += cacheinfo.o
>  util-obj-y += error.o qemu-error.o
> +util-obj-y += qemu-print.o
>  util-obj-y += id.o
>  util-obj-y += iov.o qemu-config.o qemu-sockets.o uri.o notify.o
>  util-obj-y += qemu-option.o qemu-progress.o
> diff --git a/util/qemu-print.c b/util/qemu-print.c
> new file mode 100644
> index 0000000000..86f9417af8
> --- /dev/null
> +++ b/util/qemu-print.c
> @@ -0,0 +1,42 @@
> +/*
> + * Print to stream or current monitor
> + *
> + * Copyright (C) 2019 Red Hat Inc.
> + *
> + * Authors:
> + *  Markus Armbruster <armbru@redhat.com>,
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "monitor/monitor.h"
> +#include "qemu/qemu-print.h"
> +
> +/*
> + * Print like vprintf().
> + * Print to current monitor if we have one, else to stdout.
> + */
> +int qemu_vprintf(const char *fmt, va_list ap)
> +{
> +    if (cur_mon) {
> +        return monitor_vprintf(cur_mon, fmt, ap);
> +    }
> +    return vprintf(fmt, ap);
> +}
> +
> +/*
> + * Print like printf().
> + * Print to current monitor if we have one, else to stdout.
> + */
> +int qemu_printf(const char *fmt, ...)
> +{
> +    va_list ap;
> +    int ret;
> +
> +    va_start(ap, fmt);
> +    ret = qemu_vprintf(fmt, ap);
> +    va_end(ap);
> +    return ret;
> +}
> -- 
> 2.17.2
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 12/15] qemu-print: New qemu_printf(), qemu_vprintf() etc.

[Markus Armbruster]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> We commonly want to print to the current monitor if we have one, else
>> to stdout/stderr.  For stderr, have error_printf().  For stdout, all
>> we have is monitor_vfprintf(), which is rather unwieldy.  We often
>> print to stderr just because error_printf() is easier.
>> 
>> New qemu_printf() and qemu_vprintf() do exactly what's needed.  The
>> next commits will put them to use.
>> 
>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> OK, it seems a little odd not to have the fprintf as well, but OK.

Wait for my next series touching this :)

> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks!

[Qemu-devel] [PATCH 13/15] blockdev: Make -drive format=help print to stdout

[Markus Armbruster]

Command line help explicitly requested by the user should be printed
to stdout, not stderr.  We do elsewhere.  Adjust -drive to match: use
qemu_printf() instead of error_printf().  Plain printf() would be
wrong because we need to print to the current monitor for "drive_add
... format=help".

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4775a07d93..79fbac8450 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -40,6 +40,7 @@
 #include "monitor/monitor.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
+#include "qemu/qemu-print.h"
 #include "qemu/config-file.h"
 #include "qapi/qapi-commands-block.h"
 #include "qapi/qapi-commands-transaction.h"
@@ -301,7 +302,7 @@ DriveInfo *drive_get_next(BlockInterfaceType type)
 
 static void bdrv_format_print(void *opaque, const char *name)
 {
-    error_printf(" %s", name);
+    qemu_printf(" %s", name);
 }
 
 typedef struct {
@@ -530,11 +531,11 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
 
     if ((buf = qemu_opt_get(opts, "format")) != NULL) {
         if (is_help_option(buf)) {
-            error_printf("Supported formats:");
+            qemu_printf("Supported formats:");
             bdrv_iterate_format(bdrv_format_print, NULL, false);
-            error_printf("\nSupported formats (read-only):");
+            qemu_printf("\nSupported formats (read-only):");
             bdrv_iterate_format(bdrv_format_print, NULL, true);
-            error_printf("\n");
+            qemu_printf("\n");
             goto early_err;
         }
 
-- 
2.17.2

[Qemu-devel] [PATCH 13/15] blockdev: Make -drive format=help print to stdout

[Markus Armbruster]

Command line help explicitly requested by the user should be printed
to stdout, not stderr.  We do elsewhere.  Adjust -drive to match: use
qemu_printf() instead of error_printf().  Plain printf() would be
wrong because we need to print to the current monitor for "drive_add
... format=help".

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4775a07d93..79fbac8450 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -40,6 +40,7 @@
 #include "monitor/monitor.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
+#include "qemu/qemu-print.h"
 #include "qemu/config-file.h"
 #include "qapi/qapi-commands-block.h"
 #include "qapi/qapi-commands-transaction.h"
@@ -301,7 +302,7 @@ DriveInfo *drive_get_next(BlockInterfaceType type)
 
 static void bdrv_format_print(void *opaque, const char *name)
 {
-    error_printf(" %s", name);
+    qemu_printf(" %s", name);
 }
 
 typedef struct {
@@ -530,11 +531,11 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
 
     if ((buf = qemu_opt_get(opts, "format")) != NULL) {
         if (is_help_option(buf)) {
-            error_printf("Supported formats:");
+            qemu_printf("Supported formats:");
             bdrv_iterate_format(bdrv_format_print, NULL, false);
-            error_printf("\nSupported formats (read-only):");
+            qemu_printf("\nSupported formats (read-only):");
             bdrv_iterate_format(bdrv_format_print, NULL, true);
-            error_printf("\n");
+            qemu_printf("\n");
             goto early_err;
         }
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH 13/15] blockdev: Make -drive format=help print to stdout

[Philippe Mathieu-Daudé]

On 4/8/19 10:36 AM, Markus Armbruster wrote:
> Command line help explicitly requested by the user should be printed
> to stdout, not stderr.  We do elsewhere.  Adjust -drive to match: use
> qemu_printf() instead of error_printf().  Plain printf() would be
> wrong because we need to print to the current monitor for "drive_add
> ... format=help".
> 
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: qemu-block@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  blockdev.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 4775a07d93..79fbac8450 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -40,6 +40,7 @@
>  #include "monitor/monitor.h"
>  #include "qemu/error-report.h"
>  #include "qemu/option.h"
> +#include "qemu/qemu-print.h"
>  #include "qemu/config-file.h"
>  #include "qapi/qapi-commands-block.h"
>  #include "qapi/qapi-commands-transaction.h"
> @@ -301,7 +302,7 @@ DriveInfo *drive_get_next(BlockInterfaceType type)
>  
>  static void bdrv_format_print(void *opaque, const char *name)
>  {
> -    error_printf(" %s", name);
> +    qemu_printf(" %s", name);
>  }
>  
>  typedef struct {
> @@ -530,11 +531,11 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
>  
>      if ((buf = qemu_opt_get(opts, "format")) != NULL) {
>          if (is_help_option(buf)) {
> -            error_printf("Supported formats:");
> +            qemu_printf("Supported formats:");
>              bdrv_iterate_format(bdrv_format_print, NULL, false);
> -            error_printf("\nSupported formats (read-only):");
> +            qemu_printf("\nSupported formats (read-only):");
>              bdrv_iterate_format(bdrv_format_print, NULL, true);
> -            error_printf("\n");
> +            qemu_printf("\n");
>              goto early_err;
>          }
>  
> 

[Qemu-devel] [PATCH 14/15] char: Make -chardev help print to stdout

[Markus Armbruster]

Command line help explicitly requested by the user should be printed
to stdout, not stderr.  We do elsewhere.  Adjust -chardev to match:
use qemu_printf() instead of error_printf().  Plain printf() would be
wrong because we need to print to the current monitor for chardev-add
help".

Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 chardev/char.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/chardev/char.c b/chardev/char.c
index 514cd6b0c3..54724a56b1 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -28,6 +28,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "qemu/qemu-print.h"
 #include "chardev/char.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-char.h"
@@ -651,7 +652,7 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
 
         chardev_name_foreach(help_string_append, str);
 
-        error_printf("Available chardev backend types: %s\n", str->str);
+        qemu_printf("Available chardev backend types: %s\n", str->str);
         g_string_free(str, true);
         return NULL;
     }
-- 
2.17.2

Re: [Qemu-devel] [PATCH 14/15] char: Make -chardev help print to stdout

[Marc-André Lureau]

On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Command line help explicitly requested by the user should be printed
> to stdout, not stderr.  We do elsewhere.  Adjust -chardev to match:
> use qemu_printf() instead of error_printf().  Plain printf() would be
> wrong because we need to print to the current monitor for chardev-add
> help".
>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  chardev/char.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 514cd6b0c3..54724a56b1 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -28,6 +28,7 @@
>  #include "sysemu/sysemu.h"
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
> +#include "qemu/qemu-print.h"
>  #include "chardev/char.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-char.h"
> @@ -651,7 +652,7 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
>
>          chardev_name_foreach(help_string_append, str);
>
> -        error_printf("Available chardev backend types: %s\n", str->str);
> +        qemu_printf("Available chardev backend types: %s\n", str->str);
>          g_string_free(str, true);
>          return NULL;
>      }
> --
> 2.17.2
>

Re: [Qemu-devel] [PATCH 14/15] char: Make -chardev help print to stdout

[Marc-André Lureau]

On Mon, Apr 8, 2019 at 10:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Command line help explicitly requested by the user should be printed
> to stdout, not stderr.  We do elsewhere.  Adjust -chardev to match:
> use qemu_printf() instead of error_printf().  Plain printf() would be
> wrong because we need to print to the current monitor for chardev-add
> help".
>
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  chardev/char.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 514cd6b0c3..54724a56b1 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -28,6 +28,7 @@
>  #include "sysemu/sysemu.h"
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
> +#include "qemu/qemu-print.h"
>  #include "chardev/char.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-char.h"
> @@ -651,7 +652,7 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
>
>          chardev_name_foreach(help_string_append, str);
>
> -        error_printf("Available chardev backend types: %s\n", str->str);
> +        qemu_printf("Available chardev backend types: %s\n", str->str);
>          g_string_free(str, true);
>          return NULL;
>      }
> --
> 2.17.2
>

Re: [Qemu-devel] [PATCH 14/15] char: Make -chardev help print to stdout

[Philippe Mathieu-Daudé]

On 4/8/19 10:36 AM, Markus Armbruster wrote:
> Command line help explicitly requested by the user should be printed
> to stdout, not stderr.  We do elsewhere.  Adjust -chardev to match:
> use qemu_printf() instead of error_printf().  Plain printf() would be
> wrong because we need to print to the current monitor for chardev-add
> help".
> 
> Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  chardev/char.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/chardev/char.c b/chardev/char.c
> index 514cd6b0c3..54724a56b1 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -28,6 +28,7 @@
>  #include "sysemu/sysemu.h"
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
> +#include "qemu/qemu-print.h"
>  #include "chardev/char.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-char.h"
> @@ -651,7 +652,7 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, GMainContext *context,
>  
>          chardev_name_foreach(help_string_append, str);
>  
> -        error_printf("Available chardev backend types: %s\n", str->str);
> +        qemu_printf("Available chardev backend types: %s\n", str->str);
>          g_string_free(str, true);
>          return NULL;
>      }
> 

Re: [Qemu-devel] [PATCH 14/15] char: Make -chardev help print to stdout

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]

On 4/8/19 3:36 AM, Markus Armbruster wrote:
> Command line help explicitly requested by the user should be printed
> to stdout, not stderr.  We do elsewhere.  Adjust -chardev to match:
> use qemu_printf() instead of error_printf().  Plain printf() would be
> wrong because we need to print to the current monitor for chardev-add
> help".

Mismatched "

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 14/15] char: Make -chardev help print to stdout

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 4/8/19 3:36 AM, Markus Armbruster wrote:
>> Command line help explicitly requested by the user should be printed
>> to stdout, not stderr.  We do elsewhere.  Adjust -chardev to match:
>> use qemu_printf() instead of error_printf().  Plain printf() would be
>> wrong because we need to print to the current monitor for chardev-add
>> help".
>
> Mismatched "

Eagle eyes.  Will fix, thanks!

Re: [Qemu-devel] [PATCH 14/15] char: Make -chardev help print to stdout

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 4/8/19 3:36 AM, Markus Armbruster wrote:
>> Command line help explicitly requested by the user should be printed
>> to stdout, not stderr.  We do elsewhere.  Adjust -chardev to match:
>> use qemu_printf() instead of error_printf().  Plain printf() would be
>> wrong because we need to print to the current monitor for chardev-add
>> help".
>
> Mismatched "

Eagle eyes.  Will fix, thanks!

[Qemu-devel] [PATCH 15/15] monitor: Simplify how -device/device_add print help

[Markus Armbruster]

Commit a95db58f210 added monitor_vfprintf() as an error_printf()
generalized from stderr to arbitrary streams, then used it wrapped in
helper out_printf() to print -device/device_add help to stdout.  Use
qemu_printf() instead, and delete monitor_vfprintf() and out_printf().

Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/monitor/monitor.h |  3 ---
 monitor.c                 | 16 ++++------------
 qdev-monitor.c            | 36 ++++++++++++++----------------------
 3 files changed, 18 insertions(+), 37 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index e4c3717454..316a168c41 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -48,7 +48,4 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
 void monitor_fdset_dup_fd_remove(int dup_fd);
 int monitor_fdset_dup_fd_find(int dup_fd);
 
-int monitor_vfprintf(FILE *stream,
-                     const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
-
 #endif /* MONITOR_H */
diff --git a/monitor.c b/monitor.c
index 7b4a78d798..10be8bdb86 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4541,23 +4541,15 @@ static void monitor_readline_flush(void *opaque)
     monitor_flush(opaque);
 }
 
-/*
- * Print to current monitor if we have one, else to stream.
- */
-int monitor_vfprintf(FILE *stream, const char *fmt, va_list ap)
-{
-    if (cur_mon && !monitor_cur_is_qmp()) {
-        return monitor_vprintf(cur_mon, fmt, ap);
-    }
-    return vfprintf(stream, fmt, ap);
-}
-
 /*
  * Print to current monitor if we have one, else to stderr.
  */
 int error_vprintf(const char *fmt, va_list ap)
 {
-    return monitor_vfprintf(stderr, fmt, ap);
+    if (cur_mon && !monitor_cur_is_qmp()) {
+        return monitor_vprintf(cur_mon, fmt, ap);
+    }
+    return vfprintf(stderr, fmt, ap);
 }
 
 int error_vprintf_unless_qmp(const char *fmt, va_list ap)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index d4320986a2..373b9ad445 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -31,6 +31,7 @@
 #include "qemu/error-report.h"
 #include "qemu/help_option.h"
 #include "qemu/option.h"
+#include "qemu/qemu-print.h"
 #include "sysemu/block-backend.h"
 #include "migration/misc.h"
 
@@ -104,31 +105,22 @@ static bool qdev_class_has_alias(DeviceClass *dc)
     return (qdev_class_get_alias(dc) != NULL);
 }
 
-static void out_printf(const char *fmt, ...)
-{
-    va_list ap;
-
-    va_start(ap, fmt);
-    monitor_vfprintf(stdout, fmt, ap);
-    va_end(ap);
-}
-
 static void qdev_print_devinfo(DeviceClass *dc)
 {
-    out_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
+    qemu_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
     if (dc->bus_type) {
-        out_printf(", bus %s", dc->bus_type);
+        qemu_printf(", bus %s", dc->bus_type);
     }
     if (qdev_class_has_alias(dc)) {
-        out_printf(", alias \"%s\"", qdev_class_get_alias(dc));
+        qemu_printf(", alias \"%s\"", qdev_class_get_alias(dc));
     }
     if (dc->desc) {
-        out_printf(", desc \"%s\"", dc->desc);
+        qemu_printf(", desc \"%s\"", dc->desc);
     }
     if (!dc->user_creatable) {
-        out_printf(", no-user");
+        qemu_printf(", no-user");
     }
-    out_printf("\n");
+    qemu_printf("\n");
 }
 
 static void qdev_print_devinfos(bool show_no_user)
@@ -164,7 +156,7 @@ static void qdev_print_devinfos(bool show_no_user)
                 continue;
             }
             if (!cat_printed) {
-                out_printf("%s%s devices:\n", i ? "\n" : "", cat_name[i]);
+                qemu_printf("%s%s devices:\n", i ? "\n" : "", cat_name[i]);
                 cat_printed = true;
             }
             qdev_print_devinfo(dc);
@@ -286,20 +278,20 @@ int qdev_device_help(QemuOpts *opts)
     }
 
     if (prop_list) {
-        out_printf("%s options:\n", driver);
+        qemu_printf("%s options:\n", driver);
     } else {
-        out_printf("There are no options for %s.\n", driver);
+        qemu_printf("There are no options for %s.\n", driver);
     }
     for (prop = prop_list; prop; prop = prop->next) {
         int len;
-        out_printf("  %s=<%s>%n", prop->value->name, prop->value->type, &len);
+        qemu_printf("  %s=<%s>%n", prop->value->name, prop->value->type, &len);
         if (prop->value->has_description) {
             if (len < 24) {
-                out_printf("%*s", 24 - len, "");
+                qemu_printf("%*s", 24 - len, "");
             }
-            out_printf(" - %s\n", prop->value->description);
+            qemu_printf(" - %s\n", prop->value->description);
         } else {
-            out_printf("\n");
+            qemu_printf("\n");
         }
     }
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH 15/15] monitor: Simplify how -device/device_add print help

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Commit a95db58f210 added monitor_vfprintf() as an error_printf()
> generalized from stderr to arbitrary streams, then used it wrapped in
> helper out_printf() to print -device/device_add help to stdout.  Use
> qemu_printf() instead, and delete monitor_vfprintf() and out_printf().
> 
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

(the 'monitor_vprintf vs monitor_vfprintf took a minute to notice)

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  include/monitor/monitor.h |  3 ---
>  monitor.c                 | 16 ++++------------
>  qdev-monitor.c            | 36 ++++++++++++++----------------------
>  3 files changed, 18 insertions(+), 37 deletions(-)
> 
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index e4c3717454..316a168c41 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -48,7 +48,4 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
>  void monitor_fdset_dup_fd_remove(int dup_fd);
>  int monitor_fdset_dup_fd_find(int dup_fd);
>  
> -int monitor_vfprintf(FILE *stream,
> -                     const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
> -
>  #endif /* MONITOR_H */
> diff --git a/monitor.c b/monitor.c
> index 7b4a78d798..10be8bdb86 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4541,23 +4541,15 @@ static void monitor_readline_flush(void *opaque)
>      monitor_flush(opaque);
>  }
>  
> -/*
> - * Print to current monitor if we have one, else to stream.
> - */
> -int monitor_vfprintf(FILE *stream, const char *fmt, va_list ap)
> -{
> -    if (cur_mon && !monitor_cur_is_qmp()) {
> -        return monitor_vprintf(cur_mon, fmt, ap);
> -    }
> -    return vfprintf(stream, fmt, ap);
> -}
> -
>  /*
>   * Print to current monitor if we have one, else to stderr.
>   */
>  int error_vprintf(const char *fmt, va_list ap)
>  {
> -    return monitor_vfprintf(stderr, fmt, ap);
> +    if (cur_mon && !monitor_cur_is_qmp()) {
> +        return monitor_vprintf(cur_mon, fmt, ap);
> +    }
> +    return vfprintf(stderr, fmt, ap);
>  }
>  
>  int error_vprintf_unless_qmp(const char *fmt, va_list ap)
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index d4320986a2..373b9ad445 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -31,6 +31,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/help_option.h"
>  #include "qemu/option.h"
> +#include "qemu/qemu-print.h"
>  #include "sysemu/block-backend.h"
>  #include "migration/misc.h"
>  
> @@ -104,31 +105,22 @@ static bool qdev_class_has_alias(DeviceClass *dc)
>      return (qdev_class_get_alias(dc) != NULL);
>  }
>  
> -static void out_printf(const char *fmt, ...)
> -{
> -    va_list ap;
> -
> -    va_start(ap, fmt);
> -    monitor_vfprintf(stdout, fmt, ap);
> -    va_end(ap);
> -}
> -
>  static void qdev_print_devinfo(DeviceClass *dc)
>  {
> -    out_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
> +    qemu_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
>      if (dc->bus_type) {
> -        out_printf(", bus %s", dc->bus_type);
> +        qemu_printf(", bus %s", dc->bus_type);
>      }
>      if (qdev_class_has_alias(dc)) {
> -        out_printf(", alias \"%s\"", qdev_class_get_alias(dc));
> +        qemu_printf(", alias \"%s\"", qdev_class_get_alias(dc));
>      }
>      if (dc->desc) {
> -        out_printf(", desc \"%s\"", dc->desc);
> +        qemu_printf(", desc \"%s\"", dc->desc);
>      }
>      if (!dc->user_creatable) {
> -        out_printf(", no-user");
> +        qemu_printf(", no-user");
>      }
> -    out_printf("\n");
> +    qemu_printf("\n");
>  }
>  
>  static void qdev_print_devinfos(bool show_no_user)
> @@ -164,7 +156,7 @@ static void qdev_print_devinfos(bool show_no_user)
>                  continue;
>              }
>              if (!cat_printed) {
> -                out_printf("%s%s devices:\n", i ? "\n" : "", cat_name[i]);
> +                qemu_printf("%s%s devices:\n", i ? "\n" : "", cat_name[i]);
>                  cat_printed = true;
>              }
>              qdev_print_devinfo(dc);
> @@ -286,20 +278,20 @@ int qdev_device_help(QemuOpts *opts)
>      }
>  
>      if (prop_list) {
> -        out_printf("%s options:\n", driver);
> +        qemu_printf("%s options:\n", driver);
>      } else {
> -        out_printf("There are no options for %s.\n", driver);
> +        qemu_printf("There are no options for %s.\n", driver);
>      }
>      for (prop = prop_list; prop; prop = prop->next) {
>          int len;
> -        out_printf("  %s=<%s>%n", prop->value->name, prop->value->type, &len);
> +        qemu_printf("  %s=<%s>%n", prop->value->name, prop->value->type, &len);
>          if (prop->value->has_description) {
>              if (len < 24) {
> -                out_printf("%*s", 24 - len, "");
> +                qemu_printf("%*s", 24 - len, "");
>              }
> -            out_printf(" - %s\n", prop->value->description);
> +            qemu_printf(" - %s\n", prop->value->description);
>          } else {
> -            out_printf("\n");
> +            qemu_printf("\n");
>          }
>      }
>  
> -- 
> 2.17.2
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 00/15] Clean up use of error_printf()

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190408083627.7479-1-armbru@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 10 check-block-qdict /public/crumple/bad_inputs
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-char -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-char" 
=================================================================
==8716==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000007090 at pc 0x55fe2ec7f820 bp 0x7ffff9dd8460 sp 0x7ffff9dd7c10
READ of size 2 at 0x602000007090 thread T0
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/endianness-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="endianness-test" 
PASS 1 test-char /char/null
---
  Right alloca redzone:    cb
  Shadow gap:              cc
==8716==ABORTING
ERROR - too few tests run (expected 34, got 1)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:908: check-unit] Error 1
make: *** Waiting for unfinished jobs....
PASS 1 endianness-test /x86_64/endianness/pc
---
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==8742==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test" 
==8754==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
==8760==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==8766==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/setup
PASS 4 ide-test /x86_64/ide/bmdma/simple_rw
PASS 5 ide-test /x86_64/ide/bmdma/trim
PASS 6 ide-test /x86_64/ide/bmdma/short_prdt
PASS 7 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
PASS 8 ide-test /x86_64/ide/bmdma/long_prdt
==8766==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdb0f4f000; bottom 0x7fa1795f8000; size: 0x005c37957000 (396069531648)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 9 ide-test /x86_64/ide/bmdma/no_busmaster
PASS 10 ide-test /x86_64/ide/bmdma/teardown
PASS 11 ide-test /x86_64/ide/flush/nodev
==8777==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 ide-test /x86_64/ide/flush/empty_drive
==8782==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 ide-test /x86_64/ide/flush/retry_pci
==8788==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 ide-test /x86_64/ide/flush/retry_isa
==8794==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 ide-test /x86_64/ide/cdrom/pio
==8800==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 16 ide-test /x86_64/ide/cdrom/pio_large
==8806==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 17 ide-test /x86_64/ide/cdrom/dma
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ahci-test" 
==8820==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ahci-test /x86_64/ahci/sanity
==8827==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 ahci-test /x86_64/ahci/pci_spec
==8833==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 ahci-test /x86_64/ahci/pci_enable
==8839==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 ahci-test /x86_64/ahci/hba_spec
==8845==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 ahci-test /x86_64/ahci/hba_enable
==8851==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 ahci-test /x86_64/ahci/identify
==8857==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 ahci-test /x86_64/ahci/max
==8863==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 ahci-test /x86_64/ahci/reset
==8869==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8869==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdc3dc7000; bottom 0x7fc5dd3fe000; size: 0x0037e69c9000 (240092221440)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 9 ahci-test /x86_64/ahci/io/pio/lba28/simple/zero
==8875==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8875==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdbad49000; bottom 0x7fe0599fe000; size: 0x001d6134b000 (126184894464)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 10 ahci-test /x86_64/ahci/io/pio/lba28/simple/low
==8881==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8881==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffae5c7000; bottom 0x7effa6ffe000; size: 0x0100075c9000 (1099635134464)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 11 ahci-test /x86_64/ahci/io/pio/lba28/simple/high
==8887==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8887==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd375ba000; bottom 0x7faebfbfe000; size: 0x004e779bc000 (337014145024)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 12 ahci-test /x86_64/ahci/io/pio/lba28/double/zero
==8893==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8893==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe90f8c000; bottom 0x7f93bf1fe000; size: 0x006ad1d8e000 (458787184640)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 13 ahci-test /x86_64/ahci/io/pio/lba28/double/low
==8899==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8899==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffacadb000; bottom 0x7fa5a63fe000; size: 0x005a066dd000 (386654916608)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 14 ahci-test /x86_64/ahci/io/pio/lba28/double/high
==8905==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8905==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdf1193000; bottom 0x7f8b6cd24000; size: 0x00728446f000 (491845513216)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 15 ahci-test /x86_64/ahci/io/pio/lba28/long/zero
==8911==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8911==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd376b6000; bottom 0x7f5484d24000; size: 0x00a8b2992000 (724550885376)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 16 ahci-test /x86_64/ahci/io/pio/lba28/long/low
==8917==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8917==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff89d4b000; bottom 0x7f6e413fe000; size: 0x00914894d000 (623987970048)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 17 ahci-test /x86_64/ahci/io/pio/lba28/long/high
==8923==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 18 ahci-test /x86_64/ahci/io/pio/lba28/short/zero
==8929==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 19 ahci-test /x86_64/ahci/io/pio/lba28/short/low
==8935==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 20 ahci-test /x86_64/ahci/io/pio/lba28/short/high
==8941==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8941==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc03cb8000; bottom 0x7ff67ebfe000; size: 0x0005850ba000 (23706968064)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 21 ahci-test /x86_64/ahci/io/pio/lba48/simple/zero
==8947==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8947==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe6de57000; bottom 0x7fafc29fe000; size: 0x004eab459000 (337880911872)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 22 ahci-test /x86_64/ahci/io/pio/lba48/simple/low
==8953==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8953==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc69e61000; bottom 0x7f6cf81fe000; size: 0x008f71c63000 (616089137152)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 23 ahci-test /x86_64/ahci/io/pio/lba48/simple/high
==8959==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8959==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe13e5c000; bottom 0x7f2ecfffe000; size: 0x00cf43e5e000 (890197368832)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 24 ahci-test /x86_64/ahci/io/pio/lba48/double/zero
==8965==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8965==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd8c176000; bottom 0x7f5896bfe000; size: 0x00a4f5578000 (708490788864)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 25 ahci-test /x86_64/ahci/io/pio/lba48/double/low
==8971==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8971==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd0c8ce000; bottom 0x7f3b9fffe000; size: 0x00c16c8d0000 (830749868032)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 26 ahci-test /x86_64/ahci/io/pio/lba48/double/high
==8977==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8977==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe5e1dc000; bottom 0x7f5b46bfe000; size: 0x00a3175de000 (700471697408)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 27 ahci-test /x86_64/ahci/io/pio/lba48/long/zero
==8983==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8983==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd93690000; bottom 0x7f6dc4dfe000; size: 0x008fce892000 (617645416448)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 28 ahci-test /x86_64/ahci/io/pio/lba48/long/low
==8989==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8989==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc237be000; bottom 0x7f7d43f24000; size: 0x007edf89a000 (544916217856)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 29 ahci-test /x86_64/ahci/io/pio/lba48/long/high
==8995==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 30 ahci-test /x86_64/ahci/io/pio/lba48/short/zero
==9001==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 31 ahci-test /x86_64/ahci/io/pio/lba48/short/low
==9007==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 32 ahci-test /x86_64/ahci/io/pio/lba48/short/high
==9013==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 33 ahci-test /x86_64/ahci/io/dma/lba28/fragmented
==9019==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 34 ahci-test /x86_64/ahci/io/dma/lba28/retry
==9025==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 35 ahci-test /x86_64/ahci/io/dma/lba28/simple/zero
==9031==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 36 ahci-test /x86_64/ahci/io/dma/lba28/simple/low
==9037==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 37 ahci-test /x86_64/ahci/io/dma/lba28/simple/high
==9043==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 38 ahci-test /x86_64/ahci/io/dma/lba28/double/zero
==9049==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 39 ahci-test /x86_64/ahci/io/dma/lba28/double/low
==9055==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 40 ahci-test /x86_64/ahci/io/dma/lba28/double/high
==9061==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 41 ahci-test /x86_64/ahci/io/dma/lba28/long/zero
==9067==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 42 ahci-test /x86_64/ahci/io/dma/lba28/long/low
==9073==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 43 ahci-test /x86_64/ahci/io/dma/lba28/long/high
==9079==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 44 ahci-test /x86_64/ahci/io/dma/lba28/short/zero
==9085==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 45 ahci-test /x86_64/ahci/io/dma/lba28/short/low
==9091==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 46 ahci-test /x86_64/ahci/io/dma/lba28/short/high
==9097==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 47 ahci-test /x86_64/ahci/io/dma/lba48/simple/zero
==9103==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 48 ahci-test /x86_64/ahci/io/dma/lba48/simple/low
==9109==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 49 ahci-test /x86_64/ahci/io/dma/lba48/simple/high
==9115==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 50 ahci-test /x86_64/ahci/io/dma/lba48/double/zero
==9121==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 51 ahci-test /x86_64/ahci/io/dma/lba48/double/low
==9127==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 52 ahci-test /x86_64/ahci/io/dma/lba48/double/high
==9133==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 53 ahci-test /x86_64/ahci/io/dma/lba48/long/zero
==9139==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 54 ahci-test /x86_64/ahci/io/dma/lba48/long/low
==9145==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 55 ahci-test /x86_64/ahci/io/dma/lba48/long/high
==9151==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 56 ahci-test /x86_64/ahci/io/dma/lba48/short/zero
==9157==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 57 ahci-test /x86_64/ahci/io/dma/lba48/short/low
==9163==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 58 ahci-test /x86_64/ahci/io/dma/lba48/short/high
==9169==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 59 ahci-test /x86_64/ahci/io/ncq/simple
==9175==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 60 ahci-test /x86_64/ahci/io/ncq/retry
==9181==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 61 ahci-test /x86_64/ahci/flush/simple
==9188==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 62 ahci-test /x86_64/ahci/flush/retry
==9194==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9199==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 63 ahci-test /x86_64/ahci/flush/migrate
==9208==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9213==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 64 ahci-test /x86_64/ahci/migrate/sanity
==9222==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9227==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 65 ahci-test /x86_64/ahci/migrate/dma/simple
==9236==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9241==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 66 ahci-test /x86_64/ahci/migrate/dma/halted
==9251==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9256==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 67 ahci-test /x86_64/ahci/migrate/ncq/simple
==9265==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9270==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 68 ahci-test /x86_64/ahci/migrate/ncq/halted
==9279==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 69 ahci-test /x86_64/ahci/cdrom/eject
==9284==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 70 ahci-test /x86_64/ahci/cdrom/dma/single
==9290==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 71 ahci-test /x86_64/ahci/cdrom/dma/multi
==9296==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 72 ahci-test /x86_64/ahci/cdrom/pio/single
==9302==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9302==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe3f0a3000; bottom 0x7f97b6d54000; size: 0x00668834f000 (440371834880)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 73 ahci-test /x86_64/ahci/cdrom/pio/multi
==9308==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 74 ahci-test /x86_64/ahci/cdrom/pio/bcl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/hd-geo-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="hd-geo-test" 
PASS 1 hd-geo-test /x86_64/hd-geo/ide/none
==9322==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 hd-geo-test /x86_64/hd-geo/ide/drive/cd_0
==9328==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/blank
==9334==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/lba
==9340==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/chs
==9346==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 hd-geo-test /x86_64/hd-geo/ide/device/mbr/blank
==9352==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 hd-geo-test /x86_64/hd-geo/ide/device/mbr/lba
==9358==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 hd-geo-test /x86_64/hd-geo/ide/device/mbr/chs
==9364==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 hd-geo-test /x86_64/hd-geo/ide/device/user/chs
==9369==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 hd-geo-test /x86_64/hd-geo/ide/device/user/chst
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-order-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-order-test" 
PASS 1 boot-order-test /x86_64/boot-order/pc
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9437==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 bios-tables-test /x86_64/acpi/piix4
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9443==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 bios-tables-test /x86_64/acpi/q35
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9449==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 bios-tables-test /x86_64/acpi/piix4/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9455==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 bios-tables-test /x86_64/acpi/piix4/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9461==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 bios-tables-test /x86_64/acpi/piix4/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9468==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 bios-tables-test /x86_64/acpi/piix4/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9474==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 bios-tables-test /x86_64/acpi/piix4/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9480==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 bios-tables-test /x86_64/acpi/piix4/dimmpxm
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9489==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 bios-tables-test /x86_64/acpi/q35/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9495==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 bios-tables-test /x86_64/acpi/q35/mmio64
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9501==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 bios-tables-test /x86_64/acpi/q35/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9507==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 bios-tables-test /x86_64/acpi/q35/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9514==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 bios-tables-test /x86_64/acpi/q35/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9520==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 bios-tables-test /x86_64/acpi/q35/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9526==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 bios-tables-test /x86_64/acpi/q35/dimmpxm
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-serial-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-serial-test" 
PASS 1 boot-serial-test /x86_64/boot-serial/isapc
---
PASS 1 i440fx-test /x86_64/i440fx/defaults
PASS 2 i440fx-test /x86_64/i440fx/pam
PASS 3 i440fx-test /x86_64/i440fx/firmware/bios
==9610==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 i440fx-test /x86_64/i440fx/firmware/pflash
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/fw_cfg-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="fw_cfg-test" 
PASS 1 fw_cfg-test /x86_64/fw_cfg/signature
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/drive_del-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="drive_del-test" 
PASS 1 drive_del-test /x86_64/drive_del/without-dev
PASS 2 drive_del-test /x86_64/drive_del/after_failed_device_add
==9648==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 drive_del-test /x86_64/blockdev/drive_del_device_del
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/wdt_ib700-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="wdt_ib700-test" 
PASS 1 wdt_ib700-test /x86_64/wdt_ib700/pause
---
PASS 1 usb-hcd-uhci-test /x86_64/uhci/pci/init
PASS 2 usb-hcd-uhci-test /x86_64/uhci/pci/port1
PASS 3 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug
==9843==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug/usb-storage
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/usb-hcd-xhci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="usb-hcd-xhci-test" 
PASS 1 usb-hcd-xhci-test /x86_64/xhci/pci/init
PASS 2 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug
==9852==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-uas
PASS 4 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-ccid
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/cpu-plug-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="cpu-plug-test" 
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9958==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9964==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid-auto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9970==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 vmgenid-test /x86_64/vmgenid/vmgenid/query-monitor
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/tpm-crb-swtpm-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="tpm-crb-swtpm-test" 
PASS 1 tpm-crb-swtpm-test /x86_64/tpm/crb-swtpm/test
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10075==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10080==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 migration-test /x86_64/migration/postcopy/unix
PASS 4 migration-test /x86_64/migration/postcopy/recovery
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10110==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10115==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 migration-test /x86_64/migration/precopy/unix
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10124==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10129==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 migration-test /x86_64/migration/precopy/tcp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10138==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10143==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 migration-test /x86_64/migration/xbzrle/unix
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/test-x86-cpuid-compat -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid-compat" 
PASS 1 test-x86-cpuid-compat /x86/cpuid/parsing-plus-minus
---
PASS 6 numa-test /x86_64/numa/pc/dynamic/cpu
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qmp-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="qmp-test" 
PASS 1 qmp-test /x86_64/qmp/protocol
==10472==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 qmp-test /x86_64/qmp/oob
PASS 3 qmp-test /x86_64/qmp/preconfig
PASS 4 qmp-test /x86_64/qmp/missing-any-arg
---
PASS 6 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/sdhci-pci/sdhci/sdhci-tests/registers
PASS 7 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/tpci200/ipack/ipoctal232/ipoctal232-tests/nop
PASS 8 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/tpci200/pci-device/pci-device-tests/nop
==10881==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/pci-device/pci-device-tests/nop
PASS 10 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio/virtio-tests/nop
PASS 11 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/config
---
PASS 20 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/virtio-9p-tests/fs/flush/ignored
PASS 21 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-balloon-pci/pci-device/pci-device-tests/nop
PASS 22 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-balloon-pci/virtio/virtio-tests/nop
==10894==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 23 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk/virtio-blk-tests/indirect
==10901==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 24 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk/virtio-blk-tests/config
==10908==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 25 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk/virtio-blk-tests/basic
==10915==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 26 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk/virtio-blk-tests/resize
==10922==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 27 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk-pci-tests/msix
==10929==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 28 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk-pci-tests/idx
==10936==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 29 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk-pci-tests/nxvirtq
==10943==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 30 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-blk-pci/virtio-blk-pci-tests/hotplug
PASS 31 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/basic
PASS 32 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/rx_stop_cont
---
PASS 40 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-rng-pci/pci-device/pci-device-tests/nop
PASS 41 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-rng-pci/virtio/virtio-tests/nop
PASS 42 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-rng-pci/virtio-rng-pci-tests/hotplug
==11054==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 43 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-scsi-pci/pci-device/pci-device-tests/nop
==11060==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 44 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-scsi-pci/virtio-scsi/virtio-scsi-tests/hotplug
==11066==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 45 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-scsi-pci/virtio-scsi/virtio-scsi-tests/unaligned-write-same
PASS 46 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-serial-pci/pci-device/pci-device-tests/nop
PASS 47 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-serial-pci/virtio/virtio-tests/nop
---
PASS 66 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/i82562/pci-device/pci-device-tests/nop
PASS 67 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/i82801/pci-device/pci-device-tests/nop
PASS 68 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/ES1370/pci-device/pci-device-tests/nop
==11204==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 69 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/megasas/pci-device/pci-device-tests/nop
PASS 70 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/megasas/megasas-tests/dcmd/pd-get-info/fuzz
PASS 71 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/ne2k_pci/pci-device/pci-device-tests/nop
PASS 72 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/nvme/pci-device/pci-device-tests/nop
==11216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 73 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/nvme/nvme-tests/oob-cmb-access
==11222==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 74 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/pcnet/pci-device/pci-device-tests/nop
PASS 75 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/pci-ohci/pci-device/pci-device-tests/nop
PASS 76 qos-test /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/pci-ohci/pci-ohci-tests/ohci_pci-test-hotplug


The full log is available at
http://patchew.org/logs/20190408083627.7479-1-armbru@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com