| Message ID | 20200722062902.24509-6-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | candidate fixes for 5.1-rc1 (testing, semihosting, OOM tcg, x86 fpu) | expand |
On 7/22/20 8:28 AM, Alex Bennée wrote: > It seems GetPhysicallyInstalledSystemMemory isn't available in the > MinGW headers so we have to declare it ourselves. Compile tested only. > > Cc: Stefan Weil <sw@weilnetz.de> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > util/oslib-win32.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 31030463cc9..f0f94833197 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -43,6 +43,8 @@ > /* this must come after including "trace.h" */ > #include <shlobj.h> > > +WINBASEAPI BOOL WINAPI GetPhysicallyInstalledSystemMemory (PULONGLONG); > + > void *qemu_oom_check(void *ptr) > { > if (ptr == NULL) { > @@ -831,6 +833,15 @@ char *qemu_get_host_name(Error **errp) > > size_t qemu_get_host_physmem(void) > { > - /* currently unimplemented */ > - return 0; > + ULONGLONG mem; > + > + if (GetPhysicallyInstalledSystemMemory(&mem)) { > + if (mem > SIZE_MAX) { > + return SIZE_MAX; > + } else { > + return mem; > + } > + } else { > + return 0; > + } > } >
Am 22.07.20 um 08:28 schrieb Alex Bennée: > It seems GetPhysicallyInstalledSystemMemory isn't available in the > MinGW headers so we have to declare it ourselves. Compile tested only. It is available, at least for Mingw-w64 which I also use for cross builds on Debian, but is only included with _WIN32_WINNT >= 0x0601. Currently we set _WIN32_WINNT to 0x0600. Regards, Stefan
On Wed, Jul 22, 2020 at 12:03:27PM +0200, Stefan Weil wrote: > Am 22.07.20 um 08:28 schrieb Alex Bennée: > > > It seems GetPhysicallyInstalledSystemMemory isn't available in the > > MinGW headers so we have to declare it ourselves. Compile tested only. > > > It is available, at least for Mingw-w64 which I also use for cross > builds on Debian, but is only included with _WIN32_WINNT >= 0x0601. This would be equiv to requiring Windows 7 or newer > Currently we set _WIN32_WINNT to 0x0600. This is equiv to Windows 6 / Vista / Server 2008 So if we blindly declare GetPhysicallyInstalledSystemMemory ourselves, then we're likely going to fail at runtime when QEMU is used on Windows prior to Windows 7. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
I would suggest use loadLibrary to retrieve the function, if not available, then return 0 (For Win Xp and Vista); On Wed, Jul 22, 2020 at 2:34 PM Alex Bennée <alex.bennee@linaro.org> wrote: > It seems GetPhysicallyInstalledSystemMemory isn't available in the > MinGW headers so we have to declare it ourselves. Compile tested only. > > Cc: Stefan Weil <sw@weilnetz.de> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > util/oslib-win32.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 31030463cc9..f0f94833197 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -43,6 +43,8 @@ > /* this must come after including "trace.h" */ > #include <shlobj.h> > > +WINBASEAPI BOOL WINAPI GetPhysicallyInstalledSystemMemory (PULONGLONG); > + > void *qemu_oom_check(void *ptr) > { > if (ptr == NULL) { > @@ -831,6 +833,15 @@ char *qemu_get_host_name(Error **errp) > > size_t qemu_get_host_physmem(void) > { > - /* currently unimplemented */ > - return 0; > + ULONGLONG mem; > + > + if (GetPhysicallyInstalledSystemMemory(&mem)) { > + if (mem > SIZE_MAX) { > + return SIZE_MAX; > + } else { > + return mem; > + } > + } else { > + return 0; > + } > } > -- > 2.20.1 > > > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo <div dir="ltr">I would suggest use loadLibrary to retrieve the function, if not available, then return 0 (For Win Xp and Vista);</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 22, 2020 at 2:34 PM Alex Bennée <<a href="mailto:alex.bennee@linaro.org">alex.bennee@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">It seems GetPhysicallyInstalledSystemMemory isn't available in the<br> MinGW headers so we have to declare it ourselves. Compile tested only.<br> <br> Cc: Stefan Weil <<a href="mailto:sw@weilnetz.de" target="_blank">sw@weilnetz.de</a>><br> Signed-off-by: Alex Bennée <<a href="mailto:alex.bennee@linaro.org" target="_blank">alex.bennee@linaro.org</a>><br> ---<br> util/oslib-win32.c | 15 +++++++++++++--<br> 1 file changed, 13 insertions(+), 2 deletions(-)<br> <br> diff --git a/util/oslib-win32.c b/util/oslib-win32.c<br> index 31030463cc9..f0f94833197 100644<br> --- a/util/oslib-win32.c<br> +++ b/util/oslib-win32.c<br> @@ -43,6 +43,8 @@<br> /* this must come after including "trace.h" */<br> #include <shlobj.h><br> <br> +WINBASEAPI BOOL WINAPI GetPhysicallyInstalledSystemMemory (PULONGLONG);<br> +<br> void *qemu_oom_check(void *ptr)<br> {<br> if (ptr == NULL) {<br> @@ -831,6 +833,15 @@ char *qemu_get_host_name(Error **errp)<br> <br> size_t qemu_get_host_physmem(void)<br> {<br> - /* currently unimplemented */<br> - return 0;<br> + ULONGLONG mem;<br> +<br> + if (GetPhysicallyInstalledSystemMemory(&mem)) {<br> + if (mem > SIZE_MAX) {<br> + return SIZE_MAX;<br> + } else {<br> + return mem;<br> + }<br> + } else {<br> + return 0;<br> + }<br> }<br> -- <br> 2.20.1<br> <br> <br> </blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"> 此致<br>礼<br>罗勇刚<br>Yours<br> sincerely,<br>Yonggang Luo<br></div>
Am 22.07.20 um 12:32 schrieb 罗勇刚(Yonggang Luo): > I would suggest use loadLibrary to retrieve the function, if not > available, then return 0 (For Win Xp and Vista); Maybe using GlobalMemoryStatusEx is a better alternative. It is available since XP. Regards, Stefan ||
Stefan Weil <sw@weilnetz.de> writes: > Am 22.07.20 um 12:32 schrieb 罗勇刚(Yonggang Luo): > >> I would suggest use loadLibrary to retrieve the function, if not >> available, then return 0 (For Win Xp and Vista); > > > Maybe using GlobalMemoryStatusEx is a better alternative. It is > available since XP. I would welcome an alternative patch. I have no way to test if it works anyway. -- Alex Bennée
Stefan Weil <sw@weilnetz.de> writes: > Am 22.07.20 um 08:28 schrieb Alex Bennée: > >> It seems GetPhysicallyInstalledSystemMemory isn't available in the >> MinGW headers so we have to declare it ourselves. Compile tested only. > > > It is available, at least for Mingw-w64 which I also use for cross > builds on Debian, but is only included with _WIN32_WINNT >= 0x0601. > > Currently we set _WIN32_WINNT to 0x0600. That would explain why some people see things working if they build with visual studio (which I presume has a higher setting). We could just wrap the body of the function in: #if (_WIN32_WINNT >= 0x0601) much like in commands-win32.c? Of course it wouldn't even be compile tested (I used the fedora docker image). We should probably clean up the test-mingw code to work with both the fedora and debian-w[32|64] images. > > Regards, > > Stefan -- Alex Bennée
On Wed, Jul 22, 2020 at 12:33:47PM +0100, Alex Bennée wrote: > > Stefan Weil <sw@weilnetz.de> writes: > > > Am 22.07.20 um 08:28 schrieb Alex Bennée: > > > >> It seems GetPhysicallyInstalledSystemMemory isn't available in the > >> MinGW headers so we have to declare it ourselves. Compile tested only. > > > > > > It is available, at least for Mingw-w64 which I also use for cross > > builds on Debian, but is only included with _WIN32_WINNT >= 0x0601. > > > > Currently we set _WIN32_WINNT to 0x0600. > > That would explain why some people see things working if they build with > visual studio (which I presume has a higher setting). We could just wrap > the body of the function in: No, that's not how it works. We define _WIN32_WINNT in qemu/osdep.h, and this causes the Windows headers to hide any functions that post-date that version. This is similar to how you might set _POSIX_C_SOURCE / _XOPEN_SOURCE to control UNIX header visibility. IOW, the use of visual studio shouldn't affect it. > #if (_WIN32_WINNT >= 0x0601) > > much like in commands-win32.c? > > Of course it wouldn't even be compile tested (I used the fedora docker > image). We should probably clean up the test-mingw code to work with > both the fedora and debian-w[32|64] images. I'd just go for GlobalMemoryStatusEx like Stefan suggests. We use this in libvirt and GNULIB already and it does the job. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Jul 22, 2020 at 12:31:06PM +0100, Alex Bennée wrote: > > Stefan Weil <sw@weilnetz.de> writes: > > > Am 22.07.20 um 12:32 schrieb 罗勇刚(Yonggang Luo): > > > >> I would suggest use loadLibrary to retrieve the function, if not > >> available, then return 0 (For Win Xp and Vista); > > > > > > Maybe using GlobalMemoryStatusEx is a better alternative. It is > > available since XP. > > I would welcome an alternative patch. I have no way to test if it works > anyway. Something like this should work: LPMEMORYSTATUSEX info = { .dwLength = sizeof(LPMEMORYSTATUSEX), }; if (!GlobalMemoryStatusEx(&info)) { ...error report... } return info.ullTotalPhys; (or ullAvailPhys for current free memory) Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 31030463cc9..f0f94833197 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -43,6 +43,8 @@ /* this must come after including "trace.h" */ #include <shlobj.h> +WINBASEAPI BOOL WINAPI GetPhysicallyInstalledSystemMemory (PULONGLONG); + void *qemu_oom_check(void *ptr) { if (ptr == NULL) { @@ -831,6 +833,15 @@ char *qemu_get_host_name(Error **errp) size_t qemu_get_host_physmem(void) { - /* currently unimplemented */ - return 0; + ULONGLONG mem; + + if (GetPhysicallyInstalledSystemMemory(&mem)) { + if (mem > SIZE_MAX) { + return SIZE_MAX; + } else { + return mem; + } + } else { + return 0; + } }
It seems GetPhysicallyInstalledSystemMemory isn't available in the MinGW headers so we have to declare it ourselves. Compile tested only. Cc: Stefan Weil <sw@weilnetz.de> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- util/oslib-win32.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) -- 2.20.1