Re: [PATCH 14/22] plugins: make test/example plugins work on windows

[Paolo Bonzini <pbonzini@redhat.com>]

One important remark below that Greg can answer; the others are nits.

On 11/6/23 19:51, Alex Bennée wrote:
> diff --git a/contrib/plugins/win32_linker.c b/contrib/plugins/win32_linker.c
> new file mode 100644
> index 0000000000..50797d616e
> --- /dev/null
> +++ b/contrib/plugins/win32_linker.c
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright (C) 2023, Greg Manning <gmanning@rapitasystems.com>
> + *
> + * This hook, __pfnDliFailureHook2, is documented in the microsoft documentation here:
> + * https://learn.microsoft.com/en-us/cpp/build/reference/error-handling-and-notification
> + * It gets called when a delay-loaded DLL encounters various errors.
> + * We handle the specific case of a DLL looking for a "qemu.exe",
> + * and give it the running executable (regardless of what it is named).
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#include <Windows.h>

Just a nit, but we generally use lowercase "windows.h".

> +#include <delayimp.h>
> +
> +FARPROC WINAPI dll_failure_hook(unsigned dliNotify, PDelayLoadInfo pdli);
> +
> +
> +PfnDliHook __pfnDliFailureHook2 = dll_failure_hook;
> +
> +FARPROC WINAPI dll_failure_hook(unsigned dliNotify, PDelayLoadInfo pdli) {
> +    if (dliNotify == dliFailLoadLib) {

I think this could also use the notification hook and 
dliNotePreLoadLibrary.  That's a little more tidy but it's okay either way.

A bit more important: would it make sense to include the hook *in the 
QEMU executable itself*, rather than in the DLL?  If it works, it would 
be much preferrable.  You still would have to add the .lib file to the 
compilation, but win32_linker.c could simply be placed in os-win32.c 
with fewer changes to meson.build and the makefiles.

> +    if targetos == 'windows'
> +      t += shared_module(i, files(i + '.c') + '../../contrib/plugins/win32_linker.c',
> +                        include_directories: '../../include/qemu',
> +                        objects: [win32_qemu_plugin_api_lib],
> +                        dependencies: glib)
> +
> +    else
> +      t += shared_module(i, files(i + '.c'),
> +                        include_directories: '../../include/qemu',
> +                        dependencies: glib)
> +    endif

If the win32_linker.c file can be removed (by moving the hook into the 
emulator), I'd rather have this where win32_qemu_plugin_api_lib is created:

if targetos == 'windows'
   ...
else
   win32_qemu_plugin_api_lib = []
endif

and then here you can just use "objects: [win32_qemu_plugin_api_lib]" 
unconditionally, saving an "if" and some duplication.

Paolo

>     endforeach
>   endif
>   if t.length() > 0