[Qemu-devel] [PATCH v3 0/2] main_loop: Make main_loop_wait() return void

[Qemu-devel] [PATCH v3 0/2] main_loop: Make main_loop_wait() return void

[Peter Maydell]

This patchset removes the useless return value from
main_loop_wait().

Changes v2->v3:
 * add initial patch which removes the use of the return value
   from the version of main_loop in test-char.c -- it didn't
   really need it anyway.
 * removed unnecessary 'return;' at end of function

NB: I've just also noticed that there is exactly one use in the
tree of the argument to main_loop_wait() now which doesn't
pass 'false' -- that is in a function in hw/display/xenfb.c
that was added 8 years ago with a comment claiming it was
strictly temporary ;-)


Peter Maydell (2):
  tests/test-char.c: Don't use main_loop_wait()'s return value
  main_loop: Make main_loop_wait() return void

 include/qemu/main-loop.h | 2 +-
 tests/test-char.c        | 6 +-----
 util/main-loop.c         | 8 +-------
 3 files changed, 3 insertions(+), 13 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v3 1/2] tests/test-char.c: Don't use main_loop_wait()'s return value

[Peter Maydell]

In QEMU's main_loop() we used to check whether we should do
a nonblocking call to main_loop(); this was deleted in commit e330c118f2a5,
because now that vCPUs always drop the I/O thread lock it is an unnecessary
optimization.

The loop in test-char.c copied the old QEMU main_loop() code, but
here the nonblocking check has never been necessary because this
standalone test case doesn't hold the I/O lock anyway. Remove it,
so we can drop the main_loop_wait() return value.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/test-char.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tests/test-char.c b/tests/test-char.c
index 9e361c8..94ef708 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -20,13 +20,9 @@ typedef struct FeHandler {
 
 static void main_loop(void)
 {
-    bool nonblocking;
-    int last_io = 0;
-
     quit = false;
     do {
-        nonblocking = last_io > 0;
-        last_io = main_loop_wait(nonblocking);
+        main_loop_wait(false);
     } while (!quit);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 1/2] tests/test-char.c: Don't use main_loop_wait()'s return value

[Marc-André Lureau]

On Tue, Jun 27, 2017 at 7:57 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> In QEMU's main_loop() we used to check whether we should do
> a nonblocking call to main_loop(); this was deleted in commit e330c118f2a5,
> because now that vCPUs always drop the I/O thread lock it is an unnecessary
> optimization.
>
> The loop in test-char.c copied the old QEMU main_loop() code, but
> here the nonblocking check has never been necessary because this
> standalone test case doesn't hold the I/O lock anyway. Remove it,
> so we can drop the main_loop_wait() return value.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>

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



> ---
>  tests/test-char.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/tests/test-char.c b/tests/test-char.c
> index 9e361c8..94ef708 100644
> --- a/tests/test-char.c
> +++ b/tests/test-char.c
> @@ -20,13 +20,9 @@ typedef struct FeHandler {
>
>  static void main_loop(void)
>  {
> -    bool nonblocking;
> -    int last_io = 0;
> -
>      quit = false;
>      do {
> -        nonblocking = last_io > 0;
> -        last_io = main_loop_wait(nonblocking);
> +        main_loop_wait(false);
>      } while (!quit);
>  }
>
> --
> 2.7.4
>
>
> --
Marc-André Lureau

[Qemu-devel] [PATCH v3 2/2] main_loop: Make main_loop_wait() return void

[Peter Maydell]

The last users of main_loop_wait() that cared about the return value
have now been changed to no longer use it. Drop the now-useless return
value and make the function return void.

We avoid the awkwardness of ifdeffery to handle the 'ret'
variable in main_loop_wait() only being wanted if CONFIG_SLIRP
by simply dropping all the ifdefs. There are stub implementations
of slirp_pollfds_poll() and slirp_pollfds_fill() already in
stubs/slirp.c which do nothing, as required.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
This will coincidentally satisfy Coverity, which currently complains
in CID 1372464 that we call main_loop_wait() in vl.c and ignore the
return value which may be reporting a poll() syscall failure.
Essentially we don't expect poll() to fail, except perhaps with
a transient EINTR -- if it ever did we'd spin retrying endlessly
I think.
---
 include/qemu/main-loop.h | 2 +-
 util/main-loop.c         | 8 +-------
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index d7e24af..6b4b60b 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -79,7 +79,7 @@ int qemu_init_main_loop(Error **errp);
  *
  * @nonblocking: Whether the caller should block until an event occurs.
  */
-int main_loop_wait(int nonblocking);
+void main_loop_wait(int nonblocking);
 
 /**
  * qemu_get_aio_context: Return the main loop's AioContext
diff --git a/util/main-loop.c b/util/main-loop.c
index 19cad6b..2f48f41 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -487,7 +487,7 @@ static int os_host_main_loop_wait(int64_t timeout)
 }
 #endif
 
-int main_loop_wait(int nonblocking)
+void main_loop_wait(int nonblocking)
 {
     int ret;
     uint32_t timeout = UINT32_MAX;
@@ -500,9 +500,7 @@ int main_loop_wait(int nonblocking)
     /* poll any events */
     g_array_set_size(gpollfds, 0); /* reset for new iteration */
     /* XXX: separate device handlers from system ones */
-#ifdef CONFIG_SLIRP
     slirp_pollfds_fill(gpollfds, &timeout);
-#endif
 
     if (timeout == UINT32_MAX) {
         timeout_ns = -1;
@@ -515,16 +513,12 @@ int main_loop_wait(int nonblocking)
                                           &main_loop_tlg));
 
     ret = os_host_main_loop_wait(timeout_ns);
-#ifdef CONFIG_SLIRP
     slirp_pollfds_poll(gpollfds, (ret < 0));
-#endif
 
     /* CPU thread can infinitely wait for event after
        missing the warp */
     qemu_start_warp_timer();
     qemu_clock_run_all_timers();
-
-    return ret;
 }
 
 /* Functions to operate on the main QEMU AioContext.  */
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 0/2] main_loop: Make main_loop_wait() return void

[Paolo Bonzini]

On 27/06/2017 19:32, Peter Maydell wrote:
> This patchset removes the useless return value from
> main_loop_wait().
> 
> Changes v2->v3:
>  * add initial patch which removes the use of the return value
>    from the version of main_loop in test-char.c -- it didn't
>    really need it anyway.
>  * removed unnecessary 'return;' at end of function
> 
> NB: I've just also noticed that there is exactly one use in the
> tree of the argument to main_loop_wait() now which doesn't
> pass 'false' -- that is in a function in hw/display/xenfb.c
> that was added 8 years ago with a comment claiming it was
> strictly temporary ;-)
> 
> 
> Peter Maydell (2):
>   tests/test-char.c: Don't use main_loop_wait()'s return value
>   main_loop: Make main_loop_wait() return void
> 
>  include/qemu/main-loop.h | 2 +-
>  tests/test-char.c        | 6 +-----
>  util/main-loop.c         | 8 +-------
>  3 files changed, 3 insertions(+), 13 deletions(-)
> 

Queued, thanks.

Paolo

Re: [Qemu-devel] [PATCH v3 0/2] main_loop: Make main_loop_wait() return void

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH v3 0/2] main_loop: Make main_loop_wait() return void
Message-id: 1498584769-12439-1-git-send-email-peter.maydell@linaro.org
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
fatal: Cannot update paths and switch to branch 'test' at the same time.
Did you intend to checkout 'origin/patchew/1498584769-12439-1-git-send-email-peter.maydell@linaro.org' which can not be resolved as commit?
Traceback (most recent call last):
  File "/home/fam/bin/patchew", line 440, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf)
  File "/home/fam/bin/patchew", line 53, in git_clone_repo
    cwd=clone)
  File "/usr/lib64/python3.5/subprocess.py", line 271, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'checkout', 'origin/patchew/1498584769-12439-1-git-send-email-peter.maydell@linaro.org', '-b', 'test']' returned non-zero exit status 128



---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH v3 0/2] main_loop: Make main_loop_wait() return void

[Fam Zheng]

On Thu, 07/06 16:48, no-reply@patchew.org wrote:
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> fatal: Cannot update paths and switch to branch 'test' at the same time.
> Did you intend to checkout 'origin/patchew/1498584769-12439-1-git-send-email-peter.maydell@linaro.org' which can not be resolved as commit?
> Traceback (most recent call last):
>   File "/home/fam/bin/patchew", line 440, in test_one
>     git_clone_repo(clone, r["repo"], r["head"], logf)
>   File "/home/fam/bin/patchew", line 53, in git_clone_repo
>     cwd=clone)
>   File "/usr/lib64/python3.5/subprocess.py", line 271, in check_call
>     raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['git', 'checkout', 'origin/patchew/1498584769-12439-1-git-send-email-peter.maydell@linaro.org', '-b', 'test']' returned non-zero exit status 128

Ignore this please, patchew is recovering from a bad state.