From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brrLp-00059X-JB for qemu-devel@nongnu.org; Wed, 05 Oct 2016 14:55:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brrLl-0005PW-69 for qemu-devel@nongnu.org; Wed, 05 Oct 2016 14:55:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39324) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brrLk-0005PN-Of for qemu-devel@nongnu.org; Wed, 05 Oct 2016 14:55:29 -0400 References: <1475503285-9021-1-git-send-email-den@openvz.org> From: Laszlo Ersek Message-ID: Date: Wed, 5 Oct 2016 20:55:22 +0200 MIME-Version: 1.0 In-Reply-To: <1475503285-9021-1-git-send-email-den@openvz.org> Content-Type: text/plain; charset=utf-8 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" , qemu-devel@nongnu.org Cc: Denis Plotnikov , =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , Michael Roth , Stefan Weil On 10/03/16 16:01, 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. >=20 > This is working since Win8 and Win2k12. >=20 > Signed-off-by: Denis V. Lunev > Signed-off-by: Denis Plotnikov > CC: Michael Roth > CC: Stefan Weil > CC: Marc-Andr=C3=A9 Lureau > --- > qga/commands-win32.c | 97 ++++++++++++++++++++++++++++++++++++++++++++= ++++++-- > 1 file changed, 94 insertions(+), 3 deletions(-) Just to educate myself (really, you can ignore my question as "review comment", because it's not one): why is this necessary? In Windows 8 and Windows Server 2012 R2, simply putting your NTFS filesystem on a SCSI disk (such as virtio-scsi-pci / scsi-hd) or AHCI, and specifying discard=3Don for the -drive, will result in the guest automatically trimming the NTFS (with a little delay) after deleting files, and the host getting those blocks back. Thanks Laszlo > 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) { >=20