Re: [Qemu-devel] [PATCH v3 42/46] util: Replace fprintf(stderr, "*\n" with error_report()

[Stefan Hajnoczi <stefanha@redhat.com>]

On Fri, Oct 20, 2017 at 08:27:41AM +0200, Thomas Huth wrote:
> On 19.10.2017 21:47, Stefan Weil wrote:
> > Am 19.10.2017 um 19:53 schrieb Thomas Huth:
> >> On 19.10.2017 18:18, Alistair Francis wrote:
> >>> Replace a large number of the fprintf(stderr, "*\n" calls with
> >>> error_report(). The functions were renamed with these commands and then
> >>> compiler issues where manually fixed.
> >> [...]
> >>> diff --git a/util/aio-posix.c b/util/aio-posix.c
> >>> index 5946ac09f0..29fff51fcf 100644
> >>> --- a/util/aio-posix.c
> >>> +++ b/util/aio-posix.c
> >>> @@ -15,6 +15,7 @@
> >>>  
> >>>  #include "qemu/osdep.h"
> >>>  #include "qemu-common.h"
> >>> +#include "qemu/error-report.h"
> >>>  #include "block/block.h"
> >>>  #include "qemu/rcu_queue.h"
> >>>  #include "qemu/sockets.h"
> >>> @@ -703,8 +704,8 @@ void aio_context_setup(AioContext *ctx)
> >>>  {
> >>>      /* TODO remove this in final patch submission */
> >>>      if (getenv("QEMU_AIO_POLL_MAX_NS")) {
> >>> -        fprintf(stderr, "The QEMU_AIO_POLL_MAX_NS environment variable has "
> >>> -                "been replaced with -object iothread,poll-max-ns=NUM\n");
> >>> +        error_report("The QEMU_AIO_POLL_MAX_NS environment variable has "
> >>> +                "been replaced with -object iothread,poll-max-ns=NUM");
> >>>          exit(1);
> >>>      }
> >>
> >> The comment in front of this code block indicates that this should
> >> rather be removed completely. Stefan, do you agree?
> > 
> > I assume you asked the other Stefan, but I think he'll agree as I do,
> 
> Right, the code has been introduced by this commit:
> 
> commit 4a1cba3802554a3b077d436002519ff1fb0c18bf
> Author: Stefan Hajnoczi <stefanha@redhat.com>
> Date:   Thu Dec 1 19:26:42 2016 +0000
> 
>     aio: add polling mode to AioContext
> 
> and apparently the environment variable has never been used in the
> committed code base, so removing this code block sounds like the right
> way to go.

Yes, feel free to remove it.

Initially the RFC patches used the QEMU_AIO_POLL_MAX_NS environment
variable.  When I wrote a proper command-line interface for this setting
I left the error to help any of the people testing this optimization
migrate their scripts to the new interface.

...Then I forgot to remove it for the final non-RFC patch which got
merged :).

Stefan