[PULL,17/63] stubs: include stubs only if needed

Commit Message

Paolo Bonzini April 23, 2024, 3:09 p.m. UTC

Currently it is not documented anywhere why some functions need to
be stubbed.

Group the files in stubs/meson.build according to who needs them, both
to reduce the size of the compilation and to clarify the use of stubs.

Signed-off-by: Paolo Bonzini <[e-mail]>
Message-ID: <[e-mail]>
Signed-off-by: Paolo Bonzini <[e-mail]>
---
 stubs/{monitor.c => monitor-internal.c} |   0
 stubs/meson.build                       | 122 +++++++++++++++---------
 2 files changed, 75 insertions(+), 47 deletions(-)
 rename stubs/{monitor.c => monitor-internal.c} (100%)

Comments

Daniel P. Berrangé June 4, 2024, 10:07 a.m. UTC | #1

On Tue, Apr 23, 2024 at 05:09:05PM +0200, Paolo Bonzini wrote:
> Currently it is not documented anywhere why some functions need to
> be stubbed.
> 
> Group the files in stubs/meson.build according to who needs them, both
> to reduce the size of the compilation and to clarify the use of stubs.
> 
> Signed-off-by: Paolo Bonzini <[e-mail]>
> Message-ID: <[e-mail]>
> Signed-off-by: Paolo Bonzini <[e-mail]>
> ---
>  stubs/{monitor.c => monitor-internal.c} |   0
>  stubs/meson.build                       | 122 +++++++++++++++---------
>  2 files changed, 75 insertions(+), 47 deletions(-)
>  rename stubs/{monitor.c => monitor-internal.c} (100%)

This change breaks the build of many tests in the following situation:

  ./configure --disable-system --disable-user --disable-tools

$ makej
changing dir to build for make ""...
make[1]: Entering directory '/var/home/berrange/src/virt/qemu/build'
[1/46] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
[2/46] Linking target tests/bench/qht-bench
FAILED: tests/bench/qht-bench 
cc -m64  -o tests/bench/qht-bench tests/bench/qht-bench.p/qht-bench.c.o -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--whole-archive libevent-loop-base.fa libqom.fa -Wl,--no-whole-archive -fstack-protector-strong -Wl,-z,relro -Wl,-z,now -Wl,--start-group libqemuutil.a libevent-loop-base.fa libqom.fa -lm -pthread /usr/lib64/libglib-2.0.so /usr/lib64/libgmodule-2.0.so -Wl,--end-group
/usr/bin/ld: libqemuutil.a.p/util_error-report.c.o: in function `error_printf':
/var/home/berrange/src/virt/qemu/build/../util/error-report.c:38:(.text+0x93): undefined reference to `error_vprintf'
/usr/bin/ld: libqemuutil.a.p/util_error-report.c.o: in function `vreport':
/var/home/berrange/src/virt/qemu/build/../util/error-report.c:225:(.text+0x195): undefined reference to `error_vprintf'
collect2: error: ld returned 1 exit status
[3/46] Linking target tests/unit/check-qdict
FAILED: tests/unit/check-qdict 
cc -m64  -o tests/unit/check-qdict tests/unit/check-qdict.p/check-qdict.c.o -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--whole-archive libevent-loop-base.fa libqom.fa -Wl,--no-whole-archive -fstack-protector-strong -Wl,-z,relro -Wl,-z,now -Wl,--start-group libqemuutil.a libevent-loop-base.fa libqom.fa -lm -pthread /usr/lib64/libglib-2.0.so /usr/lib64/libgmodule-2.0.so -Wl,--end-group
/usr/bin/ld: libqemuutil.a.p/util_error-report.c.o: in function `error_printf':
/var/home/berrange/src/virt/qemu/build/../util/error-report.c:38:(.text+0x93): undefined reference to `error_vprintf'
/usr/bin/ld: libqemuutil.a.p/util_error-report.c.o: in function `vreport':
/var/home/berrange/src/virt/qemu/build/../util/error-report.c:225:(.text+0x195): undefined reference to `error_vprintf'
collect2: error: ld returned 1 exit status
[4/46] Linking target tests/unit/check-qstring
FAILED: tests/unit/check-qstring 
...snip many more similar errors...


With regards,
Daniel

Zhao Liu June 5, 2024, 2:46 p.m. UTC | #2

On Tue, Jun 04, 2024 at 11:07:30AM +0100, Daniel P. Berrangé wrote:
> Date: Tue, 4 Jun 2024 11:07:30 +0100
> From: "Daniel P. Berrangé" <[e-mail]>
> Subject: Re: [PULL 17/63] stubs: include stubs only if needed
> 
> On Tue, Apr 23, 2024 at 05:09:05PM +0200, Paolo Bonzini wrote:
> > Currently it is not documented anywhere why some functions need to
> > be stubbed.
> > 
> > Group the files in stubs/meson.build according to who needs them, both
> > to reduce the size of the compilation and to clarify the use of stubs.
> > 
> > Signed-off-by: Paolo Bonzini <[e-mail]>
> > Message-ID: <[e-mail]>
> > Signed-off-by: Paolo Bonzini <[e-mail]>
> > ---
> >  stubs/{monitor.c => monitor-internal.c} |   0
> >  stubs/meson.build                       | 122 +++++++++++++++---------
> >  2 files changed, 75 insertions(+), 47 deletions(-)
> >  rename stubs/{monitor.c => monitor-internal.c} (100%)
> 
> This change breaks the build of many tests in the following situation:
> 
>   ./configure --disable-system --disable-user --disable-tools
> 
> $ makej
> changing dir to build for make ""...
> make[1]: Entering directory '/var/home/berrange/src/virt/qemu/build'
> [1/46] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
> [2/46] Linking target tests/bench/qht-bench
> FAILED: tests/bench/qht-bench 
> cc -m64  -o tests/bench/qht-bench tests/bench/qht-bench.p/qht-bench.c.o -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--whole-archive libevent-loop-base.fa libqom.fa -Wl,--no-whole-archive -fstack-protector-strong -Wl,-z,relro -Wl,-z,now -Wl,--start-group libqemuutil.a libevent-loop-base.fa libqom.fa -lm -pthread /usr/lib64/libglib-2.0.so /usr/lib64/libgmodule-2.0.so -Wl,--end-group
> /usr/bin/ld: libqemuutil.a.p/util_error-report.c.o: in function `error_printf':
> /var/home/berrange/src/virt/qemu/build/../util/error-report.c:38:(.text+0x93): undefined reference to `error_vprintf'
> /usr/bin/ld: libqemuutil.a.p/util_error-report.c.o: in function `vreport':
> /var/home/berrange/src/virt/qemu/build/../util/error-report.c:225:(.text+0x195): undefined reference to `error_vprintf'
> collect2: error: ld returned 1 exit status
> [3/46] Linking target tests/unit/check-qdict
> FAILED: tests/unit/check-qdict 
> cc -m64  -o tests/unit/check-qdict tests/unit/check-qdict.p/check-qdict.c.o -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--whole-archive libevent-loop-base.fa libqom.fa -Wl,--no-whole-archive -fstack-protector-strong -Wl,-z,relro -Wl,-z,now -Wl,--start-group libqemuutil.a libevent-loop-base.fa libqom.fa -lm -pthread /usr/lib64/libglib-2.0.so /usr/lib64/libgmodule-2.0.so -Wl,--end-group
> /usr/bin/ld: libqemuutil.a.p/util_error-report.c.o: in function `error_printf':
> /var/home/berrange/src/virt/qemu/build/../util/error-report.c:38:(.text+0x93): undefined reference to `error_vprintf'
> /usr/bin/ld: libqemuutil.a.p/util_error-report.c.o: in function `vreport':
> /var/home/berrange/src/virt/qemu/build/../util/error-report.c:225:(.text+0x195): undefined reference to `error_vprintf'
> collect2: error: ld returned 1 exit status
> [4/46] Linking target tests/unit/check-qstring
> FAILED: tests/unit/check-qstring 
> ...snip many more similar errors...
>

error-printf.c should be a common file in stub_ss, since bench test and
unit test both need qemuutil.

It seems the related previous fix 109f1a437f99 ("stubs: Add missing qga
stubs") is not a complete fix.

Patch

diff --git a/stubs/monitor.c b/stubs/monitor-internal.c
similarity index 100%
rename from stubs/monitor.c
rename to stubs/monitor-internal.c
diff --git a/stubs/meson.build b/stubs/meson.build
index 4a524f5816b..8ee1fd57530 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -1,58 +1,86 @@ 
-stub_ss.add(files('bdrv-next-monitor-owned.c'))
-stub_ss.add(files('blk-commit-all.c'))
-stub_ss.add(files('blk-exp-close-all.c'))
-stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
-stub_ss.add(files('change-state-handler.c'))
-stub_ss.add(files('cmos.c'))
+# If possible, add new files to other directories, by using "if_false".
+# If you need them here, try to add them under one of the if statements
+# below, so that it is clear who needs the stubbed functionality.
+
 stub_ss.add(files('cpu-get-clock.c'))
-stub_ss.add(files('cpus-get-virtual-clock.c'))
-stub_ss.add(files('qemu-timer-notify-cb.c'))
-stub_ss.add(files('icount.c'))
-stub_ss.add(files('dump.c'))
-stub_ss.add(files('error-printf.c'))
 stub_ss.add(files('fdset.c'))
-stub_ss.add(files('gdbstub.c'))
-stub_ss.add(files('get-vm-name.c'))
-stub_ss.add(files('graph-lock.c'))
-stub_ss.add(files('hotplug-stubs.c'))
-if linux_io_uring.found()
-  stub_ss.add(files('io_uring.c'))
-endif
 stub_ss.add(files('iothread-lock.c'))
-if have_block
-  stub_ss.add(files('iothread-lock-block.c'))
-endif
 stub_ss.add(files('is-daemonized.c'))
-if libaio.found()
-  stub_ss.add(files('linux-aio.c'))
-endif
-stub_ss.add(files('migr-blocker.c'))
-stub_ss.add(files('monitor.c'))
 stub_ss.add(files('monitor-core.c'))
-stub_ss.add(files('physmem.c'))
-stub_ss.add(files('qemu-timer-notify-cb.c'))
-stub_ss.add(files('qmp-command-available.c'))
-stub_ss.add(files('qmp-quit.c'))
-stub_ss.add(files('qtest.c'))
-stub_ss.add(files('ram-block.c'))
-stub_ss.add(files('replay.c'))
 stub_ss.add(files('replay-mode.c'))
-stub_ss.add(files('runstate-check.c'))
-stub_ss.add(files('sysbus.c'))
-stub_ss.add(files('target-get-monitor-def.c'))
-stub_ss.add(files('target-monitor-defs.c'))
 stub_ss.add(files('trace-control.c'))
-stub_ss.add(files('uuid.c'))
-stub_ss.add(files('vmstate.c'))
-stub_ss.add(files('vm-stop.c'))
-stub_ss.add(files('win32-kbd-hook.c'))
-stub_ss.add(files('cpu-synchronize-state.c'))
-if have_block or have_ga
+
+if have_block
+  stub_ss.add(files('bdrv-next-monitor-owned.c'))
+  stub_ss.add(files('blk-commit-all.c'))
+  stub_ss.add(files('blk-exp-close-all.c'))
+  stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
+  stub_ss.add(files('change-state-handler.c'))
+  stub_ss.add(files('get-vm-name.c'))
+  stub_ss.add(files('iothread-lock-block.c'))
+  stub_ss.add(files('migr-blocker.c'))
+  stub_ss.add(files('physmem.c'))
+  stub_ss.add(files('ram-block.c'))
   stub_ss.add(files('replay-tools.c'))
+  stub_ss.add(files('runstate-check.c'))
+  stub_ss.add(files('uuid.c'))
 endif
-if have_system
-  stub_ss.add(files('fw_cfg.c'))
-  stub_ss.add(files('xen-hw-stub.c'))
-else
+
+if have_block or have_ga
+  # stubs for hooks in util/main-loop.c, util/async.c etc.
+  stub_ss.add(files('cpus-get-virtual-clock.c'))
+  stub_ss.add(files('icount.c'))
+  stub_ss.add(files('graph-lock.c'))
+  if linux_io_uring.found()
+    stub_ss.add(files('io_uring.c'))
+  endif
+  if libaio.found()
+    stub_ss.add(files('linux-aio.c'))
+  endif
+  stub_ss.add(files('qemu-timer-notify-cb.c'))
+
+  # stubs for monitor
+  stub_ss.add(files('monitor-internal.c'))
+  stub_ss.add(files('qmp-command-available.c'))
+  stub_ss.add(files('qmp-quit.c'))
+endif
+
+if have_block or have_user
+  stub_ss.add(files('qtest.c'))
+  stub_ss.add(files('vm-stop.c'))
+  stub_ss.add(files('vmstate.c'))
+
+  # more symbols provided by the monitor
+  stub_ss.add(files('error-printf.c'))
+endif
+
+if have_user
+  # Symbols that are used by hw/core.
+  stub_ss.add(files('cpu-synchronize-state.c'))
   stub_ss.add(files('qdev.c'))
 endif
+
+if have_system
+  # Symbols that are only needed in some configurations.  Try not
+  # adding more of these.  If the symbol is used in specific_ss,
+  # in particular, consider defining a preprocessor macro via
+  # Kconfig or configs/targets/.
+  stub_ss.add(files('dump.c'))
+  stub_ss.add(files('cmos.c'))
+  stub_ss.add(files('fw_cfg.c'))
+  stub_ss.add(files('target-get-monitor-def.c'))
+  stub_ss.add(files('target-monitor-defs.c'))
+  stub_ss.add(files('win32-kbd-hook.c'))
+  stub_ss.add(files('xen-hw-stub.c'))
+endif
+
+if have_system or have_user
+  stub_ss.add(files('gdbstub.c'))
+
+  # Also included in have_system for --disable-tcg builds
+  stub_ss.add(files('replay.c'))
+
+  # Also included in have_system for tests/unit/test-qdev-global-props
+  stub_ss.add(files('hotplug-stubs.c'))
+  stub_ss.add(files('sysbus.c'))
+endif