Re: [PATCH v2 0/2] modules: Improve modinfo.c support
Hello, Gerd!
On Tue, Sep 28, 2021 at 07:06:28AM +0200, Gerd Hoffmann wrote:
> On Mon, Sep 27, 2021 at 11:11:58AM -0300, Jose R. Ziviani wrote:
> > This patchset introduces the modinfo_need and changes
> > modinfo-generate.py/meson.build to generate/link one modinfo per target.
> >
> > modinfo-generate.py will know, thanks to modinfo_need, which modules are
> > currently enabled for a given target before adding it in the array of
> > modules. It will give a hint about why some modules failed, so
> > developers can have a clue about it:
>
> The approach looks good to me.
Awesome, I'll apply your review and send a new version.
Thank you!
>
> > /* hw-display-qxl.modinfo */
> > /* module QXL is missing. */
>
> You are using kconfig symbols here, so the comment should say so ;)
>
> Renaming modinfo_need to modinfo_kconfig will probably also help
> to make that clear.
>
> > /* hw-display-virtio-gpu.modinfo */
> > .name = "hw-display-virtio-gpu",
> > .objs = ((const char*[]){ "virtio-gpu-base", "virtio-gpu-device",
> > "vhost-user-gpu", NULL }),
>
> Hmm? Leftover from an older version of the series?
>
> > - accelerators can be filtered as well (this only covers the device
> >part), then the field QemuModinfo.arch can be removed.
>
> It's target-specific modules. Although accelerators are the only
> in-tree users right this is not limited to accelerators.
>
> take care,
> Gerd
>
signature.asc
Description: Digital signature
Re: [PATCH v2 0/2] modules: Improve modinfo.c support
On Mon, Sep 27, 2021 at 11:11:58AM -0300, Jose R. Ziviani wrote:
> This patchset introduces the modinfo_need and changes
> modinfo-generate.py/meson.build to generate/link one modinfo per target.
>
> modinfo-generate.py will know, thanks to modinfo_need, which modules are
> currently enabled for a given target before adding it in the array of
> modules. It will give a hint about why some modules failed, so
> developers can have a clue about it:
The approach looks good to me.
> /* hw-display-qxl.modinfo */
> /* module QXL is missing. */
You are using kconfig symbols here, so the comment should say so ;)
Renaming modinfo_need to modinfo_kconfig will probably also help
to make that clear.
> /* hw-display-virtio-gpu.modinfo */
> .name = "hw-display-virtio-gpu",
> .objs = ((const char*[]){ "virtio-gpu-base", "virtio-gpu-device",
> "vhost-user-gpu", NULL }),
Hmm? Leftover from an older version of the series?
> - accelerators can be filtered as well (this only covers the device
>part), then the field QemuModinfo.arch can be removed.
It's target-specific modules. Although accelerators are the only
in-tree users right this is not limited to accelerators.
take care,
Gerd
[PATCH v2 0/2] modules: Improve modinfo.c support
This patchset introduces the modinfo_need and changes
modinfo-generate.py/meson.build to generate/link one modinfo per target.
modinfo-generate.py will know, thanks to modinfo_need, which modules are
currently enabled for a given target before adding it in the array of
modules. It will give a hint about why some modules failed, so
developers can have a clue about it:
},{
/* hw-display-qxl.modinfo */
/* module QXL is missing. */
/* hw-display-virtio-gpu.modinfo */
.name = "hw-display-virtio-gpu",
.objs = ((const char*[]){ "virtio-gpu-base", "virtio-gpu-device",
"vhost-user-gpu", NULL }),
},{
The main reason of this change is to fix problems like:
$ ./qemu-system-s390x -nodefaults -display none -accel tcg -M none -device help
| head
Failed to open module: /.../hw-display-qxl.so: undefined symbol: vga_ioport_read
Failed to open module: /.../hw-display-virtio-vga.so: undefined symbol:
vmstate_vga_common
Failed to open module: /.../hw-display-virtio-vga.so: undefined symbol:
vmstate_vga_common
Failed to open module: /.../hw-display-virtio-vga-gl.so: undefined symbol:
have_vga
Failed to open module: /.../hw-usb-smartcard.so: undefined symbol:
ccid_card_ccid_attach
Failed to open module: /.../hw-usb-redirect.so: undefined symbol:
vmstate_usb_device
Failed to open module: /.../hw-usb-host.so: undefined symbol: vmstate_usb_device
With this patch, I run this small script successfuly:
#!/bin/bash
pushd ~/suse/virtualization/qemu/build
for qemu in qemu-system-*
do
[[ -f "$qemu" ]] || continue
res=$(./$qemu -nodefaults -display none -accel tcg -M none -device help
2>&1 | grep "Failed to" > /dev/null; echo $?)
[[ $res -eq 0 ]] && echo "Error: $qemu"
done
popd
Also run make check and check-acceptance without any failures.
Todo:
- accelerators can be filtered as well (this only covers the device
part), then the field QemuModinfo.arch can be removed.
v1 -> v2:
- Changed the approach to this problem after suggestions made by Paolo and
Gerd.
Thank you!
Jose R. Ziviani (2):
modules: introduces module_needs directive
modules: generates per-target modinfo
hw/display/qxl.c| 1 +
hw/display/vhost-user-gpu-pci.c | 1 +
hw/display/vhost-user-gpu.c | 1 +
hw/display/vhost-user-vga.c | 1 +
hw/display/virtio-gpu-base.c| 1 +
hw/display/virtio-gpu-gl.c | 1 +
hw/display/virtio-gpu-pci-gl.c | 1 +
hw/display/virtio-gpu-pci.c | 1 +
hw/display/virtio-gpu.c | 1 +
hw/display/virtio-vga-gl.c | 1 +
hw/display/virtio-vga.c | 1 +
hw/s390x/virtio-ccw-gpu.c | 1 +
hw/usb/ccid-card-emulated.c | 1 +
hw/usb/ccid-card-passthru.c | 1 +
hw/usb/host-libusb.c| 1 +
hw/usb/redirect.c | 1 +
include/qemu/module.h | 10 +
meson.build | 25 ++---
scripts/modinfo-generate.py | 40 -
19 files changed, 67 insertions(+), 24 deletions(-)
--
2.33.0
[PATCH v2 0/2] modules: Improve modinfo.c support
This patchset introduces the modinfo_need and changes
modinfo-generate.py/meson.build to generate/link one modinfo per target.
modinfo-generate.py will know, thanks to modinfo_need, which modules are
currently enabled for a given target before adding it in the array of
modules. It will give a hint about why some modules failed, so
developers can have a clue about it:
},{
/* hw-display-qxl.modinfo */
/* module QXL is missing. */
/* hw-display-virtio-gpu.modinfo */
.name = "hw-display-virtio-gpu",
.objs = ((const char*[]){ "virtio-gpu-base", "virtio-gpu-device",
"vhost-user-gpu", NULL }),
},{
The main reason of this change is to fix problems like:
$ ./qemu-system-s390x -nodefaults -display none -accel tcg -M none -device help
| head
Failed to open module: /.../hw-display-qxl.so: undefined symbol: vga_ioport_read
Failed to open module: /.../hw-display-virtio-vga.so: undefined symbol:
vmstate_vga_common
Failed to open module: /.../hw-display-virtio-vga.so: undefined symbol:
vmstate_vga_common
Failed to open module: /.../hw-display-virtio-vga-gl.so: undefined symbol:
have_vga
Failed to open module: /.../hw-usb-smartcard.so: undefined symbol:
ccid_card_ccid_attach
Failed to open module: /.../hw-usb-redirect.so: undefined symbol:
vmstate_usb_device
Failed to open module: /.../hw-usb-host.so: undefined symbol: vmstate_usb_device
With this patch, I run this small script successfuly:
#!/bin/bash
pushd ~/suse/virtualization/qemu/build
for qemu in qemu-system-*
do
[[ -f "$qemu" ]] || continue
res=$(./$qemu -nodefaults -display none -accel tcg -M none -device help
2>&1 | grep "Failed to" > /dev/null; echo $?)
[[ $res -eq 0 ]] && echo "Error: $qemu"
done
popd
Also run make check and check-acceptance without any failures.
Todo:
- accelerators can be filtered as well (this only covers the device
part), then the field QemuModinfo.arch can be removed.
v1 -> v2:
- Changed the approach to this problem after suggestions made by Paolo and
Gerd.
Thank you!
Jose R. Ziviani (2):
modules: introduces module_needs directive
modules: generates per-target modinfo
hw/display/qxl.c| 1 +
hw/display/vhost-user-gpu-pci.c | 1 +
hw/display/vhost-user-gpu.c | 1 +
hw/display/vhost-user-vga.c | 1 +
hw/display/virtio-gpu-base.c| 1 +
hw/display/virtio-gpu-gl.c | 1 +
hw/display/virtio-gpu-pci-gl.c | 1 +
hw/display/virtio-gpu-pci.c | 1 +
hw/display/virtio-gpu.c | 1 +
hw/display/virtio-vga-gl.c | 1 +
hw/display/virtio-vga.c | 1 +
hw/s390x/virtio-ccw-gpu.c | 1 +
hw/usb/ccid-card-emulated.c | 1 +
hw/usb/ccid-card-passthru.c | 1 +
hw/usb/host-libusb.c| 1 +
hw/usb/redirect.c | 1 +
include/qemu/module.h | 10 +
meson.build | 25 ++---
scripts/modinfo-generate.py | 40 -
19 files changed, 67 insertions(+), 24 deletions(-)
--
2.33.0
