[PATCH] stubs: Move qemu_fd_register stub to util/main-loop.c

[PATCH] stubs: Move qemu_fd_register stub to util/main-loop.c

[Thomas Huth]

The linker of MinGW sometimes runs into the following problem:

libqemuutil.a(util_main-loop.c.obj): In function `qemu_fd_register':
/builds/huth/qemu/build/../util/main-loop.c:331: multiple definition of
 `qemu_fd_register'
libqemuutil.a(stubs_fd-register.c.obj):/builds/huth/qemu/stubs/fd-register.c:5:
 first defined here
collect2: error: ld returned 1 exit status
/builds/huth/qemu/rules.mak:88: recipe for target 'tests/test-timed-average.exe'
 failed

qemu_fd_register() is defined in util/main-loop.c for WIN32, so let's simply
move the stub also there in the #else part of the corresponding #ifndef
to fix this problem.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 stubs/fd-register.c | 6 ------
 stubs/meson.build   | 1 -
 util/main-loop.c    | 4 ++++
 3 files changed, 4 insertions(+), 7 deletions(-)
 delete mode 100644 stubs/fd-register.c

diff --git a/stubs/fd-register.c b/stubs/fd-register.c
deleted file mode 100644
index 63a4abdb20..0000000000
--- a/stubs/fd-register.c
+++ /dev/null
@@ -1,6 +0,0 @@
-#include "qemu/osdep.h"
-#include "qemu/main-loop.h"
-
-void qemu_fd_register(int fd)
-{
-}
diff --git a/stubs/meson.build b/stubs/meson.build
index e2dfedc2a7..e0b322bc28 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -9,7 +9,6 @@ stub_ss.add(files('cpu-get-clock.c'))
 stub_ss.add(files('cpu-get-icount.c'))
 stub_ss.add(files('dump.c'))
 stub_ss.add(files('error-printf.c'))
-stub_ss.add(files('fd-register.c'))
 stub_ss.add(files('fdset.c'))
 stub_ss.add(files('fw_cfg.c'))
 stub_ss.add(files('gdbstub.c'))
diff --git a/util/main-loop.c b/util/main-loop.c
index f69f055013..217c8d6056 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -179,6 +179,10 @@ static int max_priority;
 static int glib_pollfds_idx;
 static int glib_n_poll_fds;
 
+void qemu_fd_register(int fd)
+{
+}
+
 static void glib_pollfds_fill(int64_t *cur_timeout)
 {
     GMainContext *context = g_main_context_default();
-- 
2.18.2

Re: [PATCH] stubs: Move qemu_fd_register stub to util/main-loop.c

[Paolo Bonzini]

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

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Il gio 3 set 2020, 07:45 Thomas Huth <thuth@redhat.com> ha scritto:

> The linker of MinGW sometimes runs into the following problem:
>
> libqemuutil.a(util_main-loop.c.obj): In function `qemu_fd_register':
> /builds/huth/qemu/build/../util/main-loop.c:331: multiple definition of
>  `qemu_fd_register'
>
> libqemuutil.a(stubs_fd-register.c.obj):/builds/huth/qemu/stubs/fd-register.c:5:
>  first defined here
> collect2: error: ld returned 1 exit status
> /builds/huth/qemu/rules.mak:88: recipe for target
> 'tests/test-timed-average.exe'
>  failed
>
> qemu_fd_register() is defined in util/main-loop.c for WIN32, so let's
> simply
> move the stub also there in the #else part of the corresponding #ifndef
> to fix this problem.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  stubs/fd-register.c | 6 ------
>  stubs/meson.build   | 1 -
>  util/main-loop.c    | 4 ++++
>  3 files changed, 4 insertions(+), 7 deletions(-)
>  delete mode 100644 stubs/fd-register.c
>
> diff --git a/stubs/fd-register.c b/stubs/fd-register.c
> deleted file mode 100644
> index 63a4abdb20..0000000000
> --- a/stubs/fd-register.c
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#include "qemu/osdep.h"
> -#include "qemu/main-loop.h"
> -
> -void qemu_fd_register(int fd)
> -{
> -}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index e2dfedc2a7..e0b322bc28 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -9,7 +9,6 @@ stub_ss.add(files('cpu-get-clock.c'))
>  stub_ss.add(files('cpu-get-icount.c'))
>  stub_ss.add(files('dump.c'))
>  stub_ss.add(files('error-printf.c'))
> -stub_ss.add(files('fd-register.c'))
>  stub_ss.add(files('fdset.c'))
>  stub_ss.add(files('fw_cfg.c'))
>  stub_ss.add(files('gdbstub.c'))
> diff --git a/util/main-loop.c b/util/main-loop.c
> index f69f055013..217c8d6056 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -179,6 +179,10 @@ static int max_priority;
>  static int glib_pollfds_idx;
>  static int glib_n_poll_fds;
>
> +void qemu_fd_register(int fd)
> +{
> +}
> +
>  static void glib_pollfds_fill(int64_t *cur_timeout)
>  {
>      GMainContext *context = g_main_context_default();
> --
> 2.18.2
>
>

[-- Attachment #2: Type: text/html, Size: 2898 bytes --]

Re: [PATCH] stubs: Move qemu_fd_register stub to util/main-loop.c

[罗勇刚(Yonggang Luo)]

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

I am also facing some problem alike:

  LINK    tests/test-qdev-global-props.exe
  LINK    tests/test-timed-average.exe
C:/CI-Tools/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
libqemuutil.a(util_main-loop.c.obj): in function `qemu_notify_event':
C:\work\xemu\qemu-build/../qemu/util/main-loop.c:139: multiple definition
of `qemu_notify_event';
libqemuutil.a(stubs_notify-event.c.obj):C:\work\xemu\qemu-build/../qemu/stubs/notify-event.c:6:
first defined here
collect2.exe: error: ld returned 1 exit status
make: *** [C:/work/xemu/qemu/rules.mak:88：tests/test-timed-average.exe] 错误 1

On Thu, Sep 3, 2020 at 1:46 PM Thomas Huth <thuth@redhat.com> wrote:

> The linker of MinGW sometimes runs into the following problem:
>
> libqemuutil.a(util_main-loop.c.obj): In function `qemu_fd_register':
> /builds/huth/qemu/build/../util/main-loop.c:331: multiple definition of
>  `qemu_fd_register'
>
> libqemuutil.a(stubs_fd-register.c.obj):/builds/huth/qemu/stubs/fd-register.c:5:
>  first defined here
> collect2: error: ld returned 1 exit status
> /builds/huth/qemu/rules.mak:88: recipe for target
> 'tests/test-timed-average.exe'
>  failed
>
> qemu_fd_register() is defined in util/main-loop.c for WIN32, so let's
> simply
> move the stub also there in the #else part of the corresponding #ifndef
> to fix this problem.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  stubs/fd-register.c | 6 ------
>  stubs/meson.build   | 1 -
>  util/main-loop.c    | 4 ++++
>  3 files changed, 4 insertions(+), 7 deletions(-)
>  delete mode 100644 stubs/fd-register.c
>
> diff --git a/stubs/fd-register.c b/stubs/fd-register.c
> deleted file mode 100644
> index 63a4abdb20..0000000000
> --- a/stubs/fd-register.c
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#include "qemu/osdep.h"
> -#include "qemu/main-loop.h"
> -
> -void qemu_fd_register(int fd)
> -{
> -}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index e2dfedc2a7..e0b322bc28 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -9,7 +9,6 @@ stub_ss.add(files('cpu-get-clock.c'))
>  stub_ss.add(files('cpu-get-icount.c'))
>  stub_ss.add(files('dump.c'))
>  stub_ss.add(files('error-printf.c'))
> -stub_ss.add(files('fd-register.c'))
>  stub_ss.add(files('fdset.c'))
>  stub_ss.add(files('fw_cfg.c'))
>  stub_ss.add(files('gdbstub.c'))
> diff --git a/util/main-loop.c b/util/main-loop.c
> index f69f055013..217c8d6056 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -179,6 +179,10 @@ static int max_priority;
>  static int glib_pollfds_idx;
>  static int glib_n_poll_fds;
>
> +void qemu_fd_register(int fd)
> +{
> +}
> +
>  static void glib_pollfds_fill(int64_t *cur_timeout)
>  {
>      GMainContext *context = g_main_context_default();
> --
> 2.18.2
>
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[-- Attachment #2: Type: text/html, Size: 3697 bytes --]

Re: [PATCH] stubs: Move qemu_fd_register stub to util/main-loop.c

[Thomas Huth]

On 03/09/2020 09.08, 罗勇刚(Yonggang Luo) wrote:
> I am also facing some problem alike:
> 
>   LINK    tests/test-qdev-global-props.exe
>   LINK    tests/test-timed-average.exe
> C:/CI-Tools/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
> libqemuutil.a(util_main-loop.c.obj): in function `qemu_notify_event':
> C:\work\xemu\qemu-build/../qemu/util/main-loop.c:139: multiple
> definition of `qemu_notify_event';
> libqemuutil.a(stubs_notify-event.c.obj):C:\work\xemu\qemu-build/../qemu/stubs/notify-event.c:6:
> first defined here

For the qemu_notify_event, I also got a patch here:

 https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00724.html

I just saw that you also posted a patch ... but I think the whole file
can be removed, not only the function, so maybe you could replace your
patch with mine in your series?

Thanks,
  Thomas

Re: [PATCH] stubs: Move qemu_fd_register stub to util/main-loop.c

[Daniel P. Berrangé]

On Thu, Sep 03, 2020 at 07:45:03AM +0200, Thomas Huth wrote:
> The linker of MinGW sometimes runs into the following problem:
> 
> libqemuutil.a(util_main-loop.c.obj): In function `qemu_fd_register':
> /builds/huth/qemu/build/../util/main-loop.c:331: multiple definition of
>  `qemu_fd_register'
> libqemuutil.a(stubs_fd-register.c.obj):/builds/huth/qemu/stubs/fd-register.c:5:
>  first defined here
> collect2: error: ld returned 1 exit status
> /builds/huth/qemu/rules.mak:88: recipe for target 'tests/test-timed-average.exe'
>  failed
> 
> qemu_fd_register() is defined in util/main-loop.c for WIN32, so let's simply
> move the stub also there in the #else part of the corresponding #ifndef
> to fix this problem.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  stubs/fd-register.c | 6 ------
>  stubs/meson.build   | 1 -
>  util/main-loop.c    | 4 ++++

>  3 files changed, 4 insertions(+), 7 deletions(-)
>  delete mode 100644 stubs/fd-register.c

The util/meson.build only adds main-loop.c under 'if have_block'.

Since you didn't remove that conditional, I assume that nothing
built in a "if not have_block" scenario was relying on the existing
stub ?

Assuming the answer is yes and/or CI passes 

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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: [PATCH] stubs: Move qemu_fd_register stub to util/main-loop.c

[Thomas Huth]

On 03/09/2020 10.24, Daniel P. Berrangé wrote:
> On Thu, Sep 03, 2020 at 07:45:03AM +0200, Thomas Huth wrote:
>> The linker of MinGW sometimes runs into the following problem:
>>
>> libqemuutil.a(util_main-loop.c.obj): In function `qemu_fd_register':
>> /builds/huth/qemu/build/../util/main-loop.c:331: multiple definition of
>>  `qemu_fd_register'
>> libqemuutil.a(stubs_fd-register.c.obj):/builds/huth/qemu/stubs/fd-register.c:5:
>>  first defined here
>> collect2: error: ld returned 1 exit status
>> /builds/huth/qemu/rules.mak:88: recipe for target 'tests/test-timed-average.exe'
>>  failed
>>
>> qemu_fd_register() is defined in util/main-loop.c for WIN32, so let's simply
>> move the stub also there in the #else part of the corresponding #ifndef
>> to fix this problem.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  stubs/fd-register.c | 6 ------
>>  stubs/meson.build   | 1 -
>>  util/main-loop.c    | 4 ++++
> 
>>  3 files changed, 4 insertions(+), 7 deletions(-)
>>  delete mode 100644 stubs/fd-register.c
> 
> The util/meson.build only adds main-loop.c under 'if have_block'.
> 
> Since you didn't remove that conditional, I assume that nothing
> built in a "if not have_block" scenario was relying on the existing
> stub ?

Right, as far as I can see, this is not used by the linux-user or
bsd-user builds, and since

 have_block = have_system or have_tools

we should be fine without the separate stub.

> Assuming the answer is yes and/or CI passes 

CI compilation succeeded here:

 https://gitlab.com/huth/qemu/-/pipelines/185094808
 (the failed acceptance test is something different)

and:

 https://cirrus-ci.com/build/4756242964938752

> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Thanks!

 Thomas

Re: [PATCH] stubs: Move qemu_fd_register stub to util/main-loop.c

[Paolo Bonzini]

On 03/09/20 12:22, Thomas Huth wrote:
>> Since you didn't remove that conditional, I assume that nothing
>> built in a "if not have_block" scenario was relying on the existing
>> stub ?

Technically there would be a user:

scripts/tracetool/backend/log.py
  -> (qemu_get_thread_id) util/oslib-win32.c

which brings in qemu_try_set_nonblock and that calls qemu_fd_register.

Likewise,

util/qht.c
  -> (qemu_vfree) util/oslib-win32.c

Windows builds actually don't build anything on a --disable-system
--disable-tools build so it happens to work.

However, as a follow-up qemu_set_block/qemu_try_set_nonblock and
qemu_set_nonblock probably should be moved from util/oslib-* to
util/main-loop.c.

Paolo