From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51150) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brmbn-0001Ir-JH for qemu-devel@nongnu.org; Wed, 05 Oct 2016 09:51:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brmbj-0002dZ-W6 for qemu-devel@nongnu.org; Wed, 05 Oct 2016 09:51:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60214) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brmbj-0002d7-Mg for qemu-devel@nongnu.org; Wed, 05 Oct 2016 09:51:39 -0400 Date: Wed, 5 Oct 2016 14:51:36 +0100 From: "Richard W.M. Jones" Message-ID: <20161005135135.GB27578@redhat.com> References: <1475503285-9021-1-git-send-email-den@openvz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1475503285-9021-1-git-send-email-den@openvz.org> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 1/1] qga: minimal support for fstrim for Windows guests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: qemu-devel@nongnu.org, Denis Plotnikov , =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , Michael Roth , Stefan Weil On Mon, Oct 03, 2016 at 05:01:25PM +0300, Denis V. Lunev wrote: > Unfortunately, there is no public Windows API to start trimming the > filesystem. The only viable way here is to call 'defrag.exe /L' for > each volume. It's good to know that at least in one way, ntfs-3g is more featureful than Windows :-) > This is working since Win8 and Win2k12. > > Signed-off-by: Denis V. Lunev > Signed-off-by: Denis Plotnikov > CC: Michael Roth > CC: Stefan Weil > CC: Marc-Andr=E9 Lureau > --- > qga/commands-win32.c | 97 ++++++++++++++++++++++++++++++++++++++++++++= ++++++-- > 1 file changed, 94 insertions(+), 3 deletions(-) >=20 > Changes from v3: > - fixed memory leak on error path for FindFirstVolumeW > - replaced g_malloc0 with g_malloc for uc_path. g_malloc is better as w= e are > allocating string, not an object >=20 > Changes from v1, v2: > - next attempt to fix error handling on error in FindFirstVolumeW >=20 > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 9c9be12..cebf4cc 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -840,8 +840,99 @@ static void guest_fsfreeze_cleanup(void) > GuestFilesystemTrimResponse * > qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) > { > - error_setg(errp, QERR_UNSUPPORTED); > - return NULL; > + GuestFilesystemTrimResponse *resp; > + HANDLE handle; > + WCHAR guid[MAX_PATH] =3D L""; > + > + handle =3D FindFirstVolumeW(guid, ARRAYSIZE(guid)); > + if (handle =3D=3D INVALID_HANDLE_VALUE) { > + error_setg_win32(errp, GetLastError(), "failed to find any vol= ume"); > + return NULL; > + } > + > + resp =3D g_new0(GuestFilesystemTrimResponse, 1); > + > + do { > + GuestFilesystemTrimResult *res; > + GuestFilesystemTrimResultList *list; > + PWCHAR uc_path; > + DWORD char_count =3D 0; > + char *path, *out; > + GError *gerr =3D NULL; > + gchar * argv[4]; > + > + GetVolumePathNamesForVolumeNameW(guid, NULL, 0, &char_count); > + > + if (GetLastError() !=3D ERROR_MORE_DATA) { > + continue; > + } > + if (GetDriveTypeW(guid) !=3D DRIVE_FIXED) { > + continue; > + } > + > + uc_path =3D g_malloc(sizeof(WCHAR) * char_count); > + if (!GetVolumePathNamesForVolumeNameW(guid, uc_path, char_coun= t, > + &char_count) || !*uc_pat= h) { > + /* strange, but this condition could be faced even with si= ze =3D=3D 2 */ > + g_free(uc_path); > + continue; > + } > + > + res =3D g_new0(GuestFilesystemTrimResult, 1); > + > + path =3D g_utf16_to_utf8(uc_path, char_count, NULL, NULL, &ger= r); > + > + g_free(uc_path); > + > + if (gerr !=3D NULL && gerr->code) { > + res->has_error =3D true; > + res->error =3D g_strdup(gerr->message); > + g_error_free(gerr); > + break; > + } > + > + res->path =3D path; > + > + list =3D g_new0(GuestFilesystemTrimResultList, 1); > + list->value =3D res; > + list->next =3D resp->paths; > + > + resp->paths =3D list; > + > + memset(argv, 0, sizeof(argv)); > + argv[0] =3D (gchar *)"defrag.exe"; > + argv[1] =3D (gchar *)"/L"; > + argv[2] =3D path; > + > + if (!g_spawn_sync(NULL, argv, NULL, G_SPAWN_SEARCH_PATH, NULL,= NULL, > + &out /* stdout */, NULL /* stdin */, > + NULL, &gerr)) { > + res->has_error =3D true; > + res->error =3D g_strdup(gerr->message); > + g_error_free(gerr); > + } else { > + /* defrag.exe is UGLY. Exit code is ALWAYS zero. > + Error is reported in the output with something like > + (x89000020) etc code in the stdout */ > + > + int i; > + gchar **lines =3D g_strsplit(out, "\r\n", 0); > + g_free(out); > + > + for (i =3D 0; lines[i] !=3D NULL; i++) { > + if (g_strstr_len(lines[i], -1, "(0x") =3D=3D NULL) { > + continue; > + } > + res->has_error =3D true; > + res->error =3D g_strdup(lines[i]); > + break; > + } > + g_strfreev(lines); > + } > + } while (FindNextVolumeW(handle, guid, ARRAYSIZE(guid))); > + > + FindVolumeClose(handle); > + return resp; > } > =20 > typedef enum { > @@ -1416,7 +1507,7 @@ GList *ga_command_blacklist_init(GList *blacklist= ) > "guest-get-memory-blocks", "guest-set-memory-blocks", > "guest-get-memory-block-size", > "guest-fsfreeze-freeze-list", > - "guest-fstrim", NULL}; > + NULL}; > char **p =3D (char **)list_unsupported; > =20 > while (*p) { The patch looks good to me. It's a bit of a shame that we have to grep the output for "(0x" and hope that that is the only way that error can be reported, but not much else we can do. Reviewed-by: Richard W.M. Jones Rich. --=20 Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rj= ones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW