| Message ID | CAKPgXcF64Bq+xfFRojYwHS-s3HjNS8s=1Lix4C9Z16Y5Ez3yCg@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Sameeh Jubran (2018-07-17 00:46:27) > I'v successfully compiled the previous patch on Fedora 27, but it seems to be > failing on RHEL, You can apply this patch instead which avoids using the > versionhelpers header: Thanks, applied to qga tree: https://github.com/mdroth/qemu/commits/qga I went ahead with the GetVersionEx() version since it's also used elsewhere in qga code. I needed to fix up the version check to be (major > 6) || (major == 6 && minor >= 2) since that seems to align with what's documented here: https://docs.microsoft.com/en-us/windows/desktop/api/winnt/ns-winnt-_osversioninfoa > --- > qga/commands-win32.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index fb91f5d..e688e71 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -846,6 +846,17 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error > **errp) > GuestFilesystemTrimResponse *resp; > HANDLE handle; > WCHAR guid[MAX_PATH] = L""; > + OSVERSIONINFO osvi; > + BOOL bIsWindows8orLater; > + > + ZeroMemory(&osvi, sizeof(OSVERSIONINFO)); > + osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); > + GetVersionEx(&osvi); > + bIsWindows8orLater = ((osvi.dwMajorVersion == 6) && (osvi.dwMinorVersion > > = 2)); > + if (!bIsWindows8orLater) { > + error_setg(errp, "fstrim is only supported for Win8+"); > + return NULL; > + } > > handle = FindFirstVolumeW(guid, ARRAYSIZE(guid)); > if (handle == INVALID_HANDLE_VALUE) { > -- > 2.8.1.185.gdc0db2c > > > On Mon, Jul 16, 2018 at 11:10 PM, Michael Roth <mdroth@linux.vnet.ibm.com> > wrote: > > Quoting Sameeh Jubran (2018-06-24 07:45:40) > > From: Sameeh Jubran <sjubran@redhat.com> > > > > The defrag.exe tool which is used for executing the fstrim command > > on Windows doesn't support retrim for OSes lower than Win8. This > > commit handles this case and returns a suitable error. > > > > Output of fstrim before this commit: > > {"execute":"guest-fstrim"} > > {"return": {"paths": [{"path": "C:\\", "error": "An invalid command line > option > > was specified. (0x89000008)"}, {"path": "F:\\", "error": "An invalid > command > > line option was specified. (0x89000008)"}, {"path": "S:\\", "error": "An > > invalid command line option was specified. (0x89000008)"}]}} > > > > Reported on: > > https://bugzilla.redhat.com/show_bug.cgi?id=1594113 > > > > Signed-off-by: Sameeh Jubran <sjubran@redhat.com> > > --- > > qga/commands-win32.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > > index d79974f212..0bdcd9dd38 100644 > > --- a/qga/commands-win32.c > > +++ b/qga/commands-win32.c > > @@ -30,6 +30,7 @@ > > #include <lm.h> > > #include <wtsapi32.h> > > #include <wininet.h> > > +#include <versionhelpers.h> > > I have this queued locally but the mingw64 build environment I've been > using (fc20) doesn't support this header and I'm running into some odd > null pointer / 0xc0000005 / access violation issues with the binary > generated in newer tool chains. I'll send a pull for this next week when > I can get it tested. > > What build envionment are you using? > > > > > #include "qga/guest-agent-core.h" > > #include "qga/vss-win32.h" > > @@ -852,6 +853,11 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, > Error **errp) > > HANDLE handle; > > WCHAR guid[MAX_PATH] = L""; > > > > + if (!IsWindows8OrGreater()) { > > + error_setg(errp, "fstrim is only supported for Win8+"); > > + return NULL; > > + } > > + > > handle = FindFirstVolumeW(guid, ARRAYSIZE(guid)); > > if (handle == INVALID_HANDLE_VALUE) { > > error_setg_win32(errp, GetLastError(), "failed to find any > volume"); > > -- > > 2.13.6 > > > > > > > -- > Respectfully, > Sameeh Jubran > Linkedin > Software Engineer @ Daynix.
diff --git a/qga/commands-win32.c b/qga/commands-win32.c index fb91f5d..e688e71 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -846,6 +846,17 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) GuestFilesystemTrimResponse *resp; HANDLE handle; WCHAR guid[MAX_PATH] = L""; + OSVERSIONINFO osvi; + BOOL bIsWindows8orLater; + + ZeroMemory(&osvi, sizeof(OSVERSIONINFO)); + osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); + GetVersionEx(&osvi); + bIsWindows8orLater = ((osvi.dwMajorVersion == 6) && (osvi.dwMinorVersion >= 2)); + if (!bIsWindows8orLater) { + error_setg(errp, "fstrim is only supported for Win8+"); + return NULL; + }
I'v successfully compiled the previous patch on Fedora 27, but it seems to be failing on RHEL, You can apply this patch instead which avoids using the versionhelpers header: --- qga/commands-win32.c | 11 +++++++++++ 1 file changed, 11 insertions(+) handle = FindFirstVolumeW(guid, ARRAYSIZE(guid)); if (handle == INVALID_HANDLE_VALUE) { -- 2.8.1.185.gdc0db2c On Mon, Jul 16, 2018 at 11:10 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > Quoting Sameeh Jubran (2018-06-24 07:45:40) > > From: Sameeh Jubran <sjubran@redhat.com> > > > > The defrag.exe tool which is used for executing the fstrim command > > on Windows doesn't support retrim for OSes lower than Win8. This > > commit handles this case and returns a suitable error. > > > > Output of fstrim before this commit: > > {"execute":"guest-fstrim"} > > {"return": {"paths": [{"path": "C:\\", "error": "An invalid command line > option > > was specified. (0x89000008)"}, {"path": "F:\\", "error": "An invalid > command > > line option was specified. (0x89000008)"}, {"path": "S:\\", "error": "An > > invalid command line option was specified. (0x89000008)"}]}} > > > > Reported on: > > https://bugzilla.redhat.com/show_bug.cgi?id=1594113 > > > > Signed-off-by: Sameeh Jubran <sjubran@redhat.com> > > --- > > qga/commands-win32.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > > index d79974f212..0bdcd9dd38 100644 > > --- a/qga/commands-win32.c > > +++ b/qga/commands-win32.c > > @@ -30,6 +30,7 @@ > > #include <lm.h> > > #include <wtsapi32.h> > > #include <wininet.h> > > +#include <versionhelpers.h> > > I have this queued locally but the mingw64 build environment I've been > using (fc20) doesn't support this header and I'm running into some odd > null pointer / 0xc0000005 / access violation issues with the binary > generated in newer tool chains. I'll send a pull for this next week when > I can get it tested. > > What build envionment are you using? > > > > > #include "qga/guest-agent-core.h" > > #include "qga/vss-win32.h" > > @@ -852,6 +853,11 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, > Error **errp) > > HANDLE handle; > > WCHAR guid[MAX_PATH] = L""; > > > > + if (!IsWindows8OrGreater()) { > > + error_setg(errp, "fstrim is only supported for Win8+"); > > + return NULL; > > + } > > + > > handle = FindFirstVolumeW(guid, ARRAYSIZE(guid)); > > if (handle == INVALID_HANDLE_VALUE) { > > error_setg_win32(errp, GetLastError(), "failed to find any > volume"); > > -- > > 2.13.6 > > >