qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] whpx: fix compilation


From: Marc-André Lureau
Subject: Re: [PATCH] whpx: fix compilation
Date: Fri, 18 Dec 2020 15:07:23 +0400

Hi

On Fri, Dec 18, 2020 at 2:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
On 18/12/20 11:08, Marc-André Lureau wrote:
>      >   #ifdef CONFIG_WHPX
>      >
>      > -#include "whp-dispatch.h"
>      > +#include <windows.h>
>      > +#include <WinHvPlatform.h>
>      > +#include <WinHvEmulation.h>
>      >
>      >   struct whpx_state {
>      >       uint64_t mem_quota;
>      >
>
>     This is wrong, just "git mv target/i386/whpx/whp-dispatch.h
>     include/sysemu" instead (and possibly change the #include line to
>     sysemu/whp-dispatch.h).
>
>
> Wrong? It fixes the build. So whatever is in whp-dispatch, it's not
> exactly necessary. Why should it be included?

Did you read the code that initializes whp-dispatch, or even easier try
to find the patch the introduced it ("git log --follow
target/i386/whpx/whp-dispatch.h")?  Marc-André, I expect you to know
better than "it fixes the build, ergo it is correct"...


I did try to follow. And it ends up with:
commit 93d1499c8119989e3eb9a6936c5a18aaaaca6330
Author: Paolo Bonzini <pbonzini@redhat.com>
Date:   Wed Jun 6 15:41:58 2018 +0200

    whpx: commit missing file
   
    Not included by mistake in commit 327fccb288976f95808efa968082fc9d4a9ced84.
   
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

then that commit didn't exactly help me to understand why this header should be global in include/...

 
> Furthermore, I tried your suggestion first. But it fails with other
> include issues. I can probably solve them differently, but why should
> whp-dispatch be in include/sysemu?

It shouldn't indeed, hence the second suggestion:

>     But I wonder if whp-dispatch.h is needed at all in this file.  If we
>     can just include it in whpx-all.c and whpx-apic.c, that would be much
>     better.
>
> May be, but how does this solve the issue with include/sysemu/whpx.h ?

Because "just" including it in those two .c files implies removing it
from include/sysemu/whpx.h.  I was a bit concise, I admit.


Perhaps, but whpx.h still is around with at least  WHV_PARTITION_HANDLE (if not more from indirect includes with types from the windows headers)

So I still stand behind my suggestion. It merely reduces the amount of includes to the windows system headers. We can try to further cleanup and reduce the amount of includes or header later on. This is not my goal (why do I always end up in side-track tasks that take days..)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]