Re: [PATCH] hw/display: avoid creating empty loadable modules

[Paolo Bonzini]

On 12/19/22 13:58, Daniel P. Berrangé wrote:
> When using --disable-virglrenderer, QEMU still creates
>
>    hw-display-virtio-gpu-gl.so
>    hw-display-virtio-vga-gl.so
>    hw-display-virtio-gpu-pci-gl.so
>
> but when these are loaded, they provide no functionality as the code
> which registers types is not compiled in. Funtionally this is
> relatively harmless, because QEMU is fine loading a module with no
> types.
>
> This is rather confusing for users and OS distro maintainers though,
> as they think they have the GL functionality built, but in fact the
> module they are looking at provides nothing of value.
>
> The root cause is the use of 'when/if_true' rules when adding sources
> to the module source set. If all the rules evaluate to false, then we
> have declared the module, but not added anything to it.  We need to
> put declaration of the entire module inside a condition based on
> existance of the 3rd party library deps that are mandatory.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1352
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   hw/display/meson.build | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/hw/display/meson.build b/hw/display/meson.build
> index 7a725ed80e..20a53d24a8 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -64,7 +64,7 @@ softmmu_ss.add(when: 'CONFIG_ARTIST', if_true: 
> files('artist.c'))
>   softmmu_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c', 
> 'ati_2d.c', 'ati_dbg.c'))
>   
>   
> -if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
> +if config_all_devices.has_key('CONFIG_VIRTIO_GPU') and pixman.found()

pixman is mandatory if have_system.

Can you instead change hw/meson.build to skip system-only directories? 
I think this should do:

subdir('core')
if not have_system
  subdir_done()
endif

# everything else...

The rest is good, so I have applied it.

Paolo

>     virtio_gpu_ss = ss.source_set()
>     virtio_gpu_ss.add(when: 'CONFIG_VIRTIO_GPU',
>                       if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), 
> pixman])
> @@ -73,13 +73,15 @@ if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
>     virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: 
> files('vhost-user-gpu.c'))
>     hw_display_modules += {'virtio-gpu': virtio_gpu_ss}
>   
> -  virtio_gpu_gl_ss = ss.source_set()
> -  virtio_gpu_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', virgl, opengl],
> -                       if_true: [files('virtio-gpu-gl.c', 
> 'virtio-gpu-virgl.c'), pixman, virgl])
> -  hw_display_modules += {'virtio-gpu-gl': virtio_gpu_gl_ss}
> +  if virgl.found() and opengl.found()
> +    virtio_gpu_gl_ss = ss.source_set()
> +    virtio_gpu_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', virgl, opengl],
> +                         if_true: [files('virtio-gpu-gl.c', 
> 'virtio-gpu-virgl.c'), pixman, virgl])
> +    hw_display_modules += {'virtio-gpu-gl': virtio_gpu_gl_ss}
> +  endif
>   endif
>   
> -if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
> +if config_all_devices.has_key('CONFIG_VIRTIO_PCI') and pixman.found()
>     virtio_gpu_pci_ss = ss.source_set()
>     virtio_gpu_pci_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI'],
>                           if_true: [files('virtio-gpu-pci.c'), pixman])
> @@ -87,10 +89,12 @@ if config_all_devices.has_key('CONFIG_VIRTIO_PCI')
>                           if_true: files('vhost-user-gpu-pci.c'))
>     hw_display_modules += {'virtio-gpu-pci': virtio_gpu_pci_ss}
>   
> -  virtio_gpu_pci_gl_ss = ss.source_set()
> -  virtio_gpu_pci_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI', 
> virgl, opengl],
> -                           if_true: [files('virtio-gpu-pci-gl.c'), pixman])
> -  hw_display_modules += {'virtio-gpu-pci-gl': virtio_gpu_pci_gl_ss}
> +  if virgl.found() and opengl.found()
> +    virtio_gpu_pci_gl_ss = ss.source_set()
> +    virtio_gpu_pci_gl_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRTIO_PCI', 
> virgl, opengl],
> +                             if_true: [files('virtio-gpu-pci-gl.c'), pixman])
> +    hw_display_modules += {'virtio-gpu-pci-gl': virtio_gpu_pci_gl_ss}
> +  endif
>   endif
>   
>   if config_all_devices.has_key('CONFIG_VIRTIO_VGA')