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

[Alistair Francis]

On Fri, Oct 20, 2017 at 3:47 AM, Stefan Hajnoczi <address@hidden> wrote:
> 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 <address@hidden>
>> 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 :).

Ok, I will remove it in the next version.

Thanks,
Alistair

>
> Stefan