* [Qemu-devel] [PATCH v3 1/1] qga: minimal support for fstrim for Windows guests
@ 2016-10-03 12:08 Denis V. Lunev
2016-10-03 12:22 ` Marc-André Lureau
2016-10-03 12:52 ` Stefan Weil
0 siblings, 2 replies; 4+ messages in thread
From: Denis V. Lunev @ 2016-10-03 12:08 UTC (permalink / raw)
To: qemu-devel
Cc: den, Denis Plotnikov, Michael Roth, Stefan Weil,
Marc-André Lureau
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.
This is working since Win8 and Win2k12.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
CC: Stefan Weil <sw@weilnetz.de>
CC: Marc-André Lureau <marcandre.lureau@gmail.com>
---
qga/commands-win32.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 92 insertions(+), 3 deletions(-)
Changes from v1, v2:
- next attempt to fix error handling on error in FindFirstVolumeW
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 9c9be12..57436b9 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -840,8 +840,97 @@ 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 = g_new0(GuestFilesystemTrimResponse, 1);
+ HANDLE handle;
+ WCHAR guid[MAX_PATH] = L"";
+
+ handle = FindFirstVolumeW(guid, ARRAYSIZE(guid));
+ if (handle == INVALID_HANDLE_VALUE) {
+ error_setg_win32(errp, GetLastError(), "failed to find any volume");
+ return NULL;
+ }
+
+ do {
+ GuestFilesystemTrimResult *res;
+ GuestFilesystemTrimResultList *list;
+ PWCHAR uc_path;
+ DWORD char_count = 0;
+ char *path, *out;
+ GError *gerr = NULL;
+ gchar * argv[4];
+
+ GetVolumePathNamesForVolumeNameW(guid, NULL, 0, &char_count);
+
+ if (GetLastError() != ERROR_MORE_DATA) {
+ continue;
+ }
+ if (GetDriveTypeW(guid) != DRIVE_FIXED) {
+ continue;
+ }
+
+ uc_path = g_malloc0(sizeof(WCHAR) * char_count);
+ if (!GetVolumePathNamesForVolumeNameW(guid, uc_path, char_count,
+ &char_count) || !*uc_path) {
+ /* strange, but this condition could be faced even with size == 2 */
+ g_free(uc_path);
+ continue;
+ }
+
+ res = g_new0(GuestFilesystemTrimResult, 1);
+
+ path = g_utf16_to_utf8(uc_path, char_count, NULL, NULL, &gerr);
+
+ g_free(uc_path);
+
+ if (gerr != NULL && gerr->code) {
+ res->has_error = true;
+ res->error = g_strdup(gerr->message);
+ g_error_free(gerr);
+ break;
+ }
+
+ res->path = path;
+
+ list = g_new0(GuestFilesystemTrimResultList, 1);
+ list->value = res;
+ list->next = resp->paths;
+
+ resp->paths = list;
+
+ memset(argv, 0, sizeof(argv));
+ argv[0] = (gchar *)"defrag.exe";
+ argv[1] = (gchar *)"/L";
+ argv[2] = path;
+
+ if (!g_spawn_sync(NULL, argv, NULL, G_SPAWN_SEARCH_PATH, NULL, NULL,
+ &out /* stdout */, NULL /* stdin */,
+ NULL, &gerr)) {
+ res->has_error = true;
+ res->error = 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 = g_strsplit(out, "\r\n", 0);
+ g_free(out);
+
+ for (i = 0; lines[i] != NULL; i++) {
+ if (g_strstr_len(lines[i], -1, "(0x") == NULL) {
+ continue;
+ }
+ res->has_error = true;
+ res->error = g_strdup(lines[i]);
+ break;
+ }
+ g_strfreev(lines);
+ }
+ } while (FindNextVolumeW(handle, guid, ARRAYSIZE(guid)));
+
+ FindVolumeClose(handle);
+ return resp;
}
typedef enum {
@@ -1416,7 +1505,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 = (char **)list_unsupported;
while (*p) {
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/1] qga: minimal support for fstrim for Windows guests
2016-10-03 12:08 [Qemu-devel] [PATCH v3 1/1] qga: minimal support for fstrim for Windows guests Denis V. Lunev
@ 2016-10-03 12:22 ` Marc-André Lureau
2016-10-03 12:39 ` Denis V. Lunev
2016-10-03 12:52 ` Stefan Weil
1 sibling, 1 reply; 4+ messages in thread
From: Marc-André Lureau @ 2016-10-03 12:22 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel; +Cc: Denis Plotnikov, Michael Roth, Stefan Weil
Hi Denis
On Mon, Oct 3, 2016 at 4:08 PM Denis V. Lunev <den@openvz.org> 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.
>
> This is working since Win8 and Win2k12.
>
Could you describe the changes the made between the versions and why? (btw
I didn't receive reply to my previous review comments)
thanks
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
> CC: Stefan Weil <sw@weilnetz.de>
> CC: Marc-André Lureau <marcandre.lureau@gmail.com>
> ---
> qga/commands-win32.c | 95
> ++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 92 insertions(+), 3 deletions(-)
>
> Changes from v1, v2:
> - next attempt to fix error handling on error in FindFirstVolumeW
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 9c9be12..57436b9 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -840,8 +840,97 @@ 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 =
> g_new0(GuestFilesystemTrimResponse, 1);
> + HANDLE handle;
> + WCHAR guid[MAX_PATH] = L"";
> +
> + handle = FindFirstVolumeW(guid, ARRAYSIZE(guid));
> + if (handle == INVALID_HANDLE_VALUE) {
> + error_setg_win32(errp, GetLastError(), "failed to find any
> volume");
> + return NULL;
> + }
> +
> + do {
> + GuestFilesystemTrimResult *res;
> + GuestFilesystemTrimResultList *list;
> + PWCHAR uc_path;
> + DWORD char_count = 0;
> + char *path, *out;
> + GError *gerr = NULL;
> + gchar * argv[4];
> +
> + GetVolumePathNamesForVolumeNameW(guid, NULL, 0, &char_count);
> +
> + if (GetLastError() != ERROR_MORE_DATA) {
> + continue;
> + }
> + if (GetDriveTypeW(guid) != DRIVE_FIXED) {
> + continue;
> + }
> +
> + uc_path = g_malloc0(sizeof(WCHAR) * char_count);
> + if (!GetVolumePathNamesForVolumeNameW(guid, uc_path, char_count,
> + &char_count) || !*uc_path) {
> + /* strange, but this condition could be faced even with size
> == 2 */
> + g_free(uc_path);
> + continue;
> + }
> +
> + res = g_new0(GuestFilesystemTrimResult, 1);
> +
> + path = g_utf16_to_utf8(uc_path, char_count, NULL, NULL, &gerr);
> +
> + g_free(uc_path);
> +
> + if (gerr != NULL && gerr->code) {
> + res->has_error = true;
> + res->error = g_strdup(gerr->message);
> + g_error_free(gerr);
> + break;
> + }
> +
> + res->path = path;
> +
> + list = g_new0(GuestFilesystemTrimResultList, 1);
> + list->value = res;
> + list->next = resp->paths;
> +
> + resp->paths = list;
> +
> + memset(argv, 0, sizeof(argv));
> + argv[0] = (gchar *)"defrag.exe";
> + argv[1] = (gchar *)"/L";
> + argv[2] = path;
> +
> + if (!g_spawn_sync(NULL, argv, NULL, G_SPAWN_SEARCH_PATH, NULL,
> NULL,
> + &out /* stdout */, NULL /* stdin */,
> + NULL, &gerr)) {
> + res->has_error = true;
> + res->error = 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 = g_strsplit(out, "\r\n", 0);
> + g_free(out);
> +
> + for (i = 0; lines[i] != NULL; i++) {
> + if (g_strstr_len(lines[i], -1, "(0x") == NULL) {
> + continue;
> + }
> + res->has_error = true;
> + res->error = g_strdup(lines[i]);
> + break;
> + }
> + g_strfreev(lines);
> + }
> + } while (FindNextVolumeW(handle, guid, ARRAYSIZE(guid)));
> +
> + FindVolumeClose(handle);
> + return resp;
> }
>
> typedef enum {
> @@ -1416,7 +1505,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 = (char **)list_unsupported;
>
> while (*p) {
> --
> 2.7.4
>
> --
Marc-André Lureau
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/1] qga: minimal support for fstrim for Windows guests
2016-10-03 12:22 ` Marc-André Lureau
@ 2016-10-03 12:39 ` Denis V. Lunev
0 siblings, 0 replies; 4+ messages in thread
From: Denis V. Lunev @ 2016-10-03 12:39 UTC (permalink / raw)
To: Marc-André Lureau, Denis V. Lunev, qemu-devel
Cc: Stefan Weil, Denis Plotnikov, Michael Roth
On 10/03/2016 03:22 PM, Marc-André Lureau wrote:
> Hi Denis
>
> On Mon, Oct 3, 2016 at 4:08 PM Denis V. Lunev <den@openvz.org> 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.
>>
>> This is working since Win8 and Win2k12.
>>
> Could you describe the changes the made between the versions and why? (btw
> I didn't receive reply to my previous review comments)
>
> thanks
>
>
The only change with this patch is error code for FindFirstVolumeW,
which is now generated exactly like in the case inside
qmp_guest_get_fsinfo()
I do not want to reuse this code as this will result in 2 line helper.
I do not see much benefit with that. On the other hand,
qmp_guest_get_fsinfo() should be rewritten to use unicode. I have
started doing that but this work is not completed. These
patches have revealed serious problems inside get_fsinfo(),
which is completely broken now.
But I have missed the fact, that this exact patch is rotten,
I have missed de-allocation on the error path, pls disregard.
Thank you for a prompt reply.
Den
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
>> CC: Stefan Weil <sw@weilnetz.de>
>> CC: Marc-André Lureau <marcandre.lureau@gmail.com>
>> ---
>> qga/commands-win32.c | 95
>> ++++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 92 insertions(+), 3 deletions(-)
>>
>> Changes from v1, v2:
>> - next attempt to fix error handling on error in FindFirstVolumeW
>>
>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>> index 9c9be12..57436b9 100644
>> --- a/qga/commands-win32.c
>> +++ b/qga/commands-win32.c
>> @@ -840,8 +840,97 @@ 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 =
>> g_new0(GuestFilesystemTrimResponse, 1);
>> + HANDLE handle;
>> + WCHAR guid[MAX_PATH] = L"";
>> +
>> + handle = FindFirstVolumeW(guid, ARRAYSIZE(guid));
>> + if (handle == INVALID_HANDLE_VALUE) {
>> + error_setg_win32(errp, GetLastError(), "failed to find any
>> volume");
>> + return NULL;
>> + }
>> +
>> + do {
>> + GuestFilesystemTrimResult *res;
>> + GuestFilesystemTrimResultList *list;
>> + PWCHAR uc_path;
>> + DWORD char_count = 0;
>> + char *path, *out;
>> + GError *gerr = NULL;
>> + gchar * argv[4];
>> +
>> + GetVolumePathNamesForVolumeNameW(guid, NULL, 0, &char_count);
>> +
>> + if (GetLastError() != ERROR_MORE_DATA) {
>> + continue;
>> + }
>> + if (GetDriveTypeW(guid) != DRIVE_FIXED) {
>> + continue;
>> + }
>> +
>> + uc_path = g_malloc0(sizeof(WCHAR) * char_count);
>> + if (!GetVolumePathNamesForVolumeNameW(guid, uc_path, char_count,
>> + &char_count) || !*uc_path) {
>> + /* strange, but this condition could be faced even with size
>> == 2 */
>> + g_free(uc_path);
>> + continue;
>> + }
>> +
>> + res = g_new0(GuestFilesystemTrimResult, 1);
>> +
>> + path = g_utf16_to_utf8(uc_path, char_count, NULL, NULL, &gerr);
>> +
>> + g_free(uc_path);
>> +
>> + if (gerr != NULL && gerr->code) {
>> + res->has_error = true;
>> + res->error = g_strdup(gerr->message);
>> + g_error_free(gerr);
>> + break;
>> + }
>> +
>> + res->path = path;
>> +
>> + list = g_new0(GuestFilesystemTrimResultList, 1);
>> + list->value = res;
>> + list->next = resp->paths;
>> +
>> + resp->paths = list;
>> +
>> + memset(argv, 0, sizeof(argv));
>> + argv[0] = (gchar *)"defrag.exe";
>> + argv[1] = (gchar *)"/L";
>> + argv[2] = path;
>> +
>> + if (!g_spawn_sync(NULL, argv, NULL, G_SPAWN_SEARCH_PATH, NULL,
>> NULL,
>> + &out /* stdout */, NULL /* stdin */,
>> + NULL, &gerr)) {
>> + res->has_error = true;
>> + res->error = 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 = g_strsplit(out, "\r\n", 0);
>> + g_free(out);
>> +
>> + for (i = 0; lines[i] != NULL; i++) {
>> + if (g_strstr_len(lines[i], -1, "(0x") == NULL) {
>> + continue;
>> + }
>> + res->has_error = true;
>> + res->error = g_strdup(lines[i]);
>> + break;
>> + }
>> + g_strfreev(lines);
>> + }
>> + } while (FindNextVolumeW(handle, guid, ARRAYSIZE(guid)));
>> +
>> + FindVolumeClose(handle);
>> + return resp;
>> }
>>
>> typedef enum {
>> @@ -1416,7 +1505,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 = (char **)list_unsupported;
>>
>> while (*p) {
>> --
>> 2.7.4
>>
>> --
> Marc-André Lureau
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/1] qga: minimal support for fstrim for Windows guests
2016-10-03 12:08 [Qemu-devel] [PATCH v3 1/1] qga: minimal support for fstrim for Windows guests Denis V. Lunev
2016-10-03 12:22 ` Marc-André Lureau
@ 2016-10-03 12:52 ` Stefan Weil
1 sibling, 0 replies; 4+ messages in thread
From: Stefan Weil @ 2016-10-03 12:52 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel
Cc: Denis Plotnikov, Marc-André Lureau, Michael Roth
See two comments below.
On 10/03/16 14:08, 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.
>
> This is working since Win8 and Win2k12.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
> CC: Stefan Weil <sw@weilnetz.de>
> CC: Marc-André Lureau <marcandre.lureau@gmail.com>
> ---
> qga/commands-win32.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 92 insertions(+), 3 deletions(-)
>
> Changes from v1, v2:
> - next attempt to fix error handling on error in FindFirstVolumeW
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 9c9be12..57436b9 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -840,8 +840,97 @@ 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 = g_new0(GuestFilesystemTrimResponse, 1);
> + HANDLE handle;
> + WCHAR guid[MAX_PATH] = L"";
> +
> + handle = FindFirstVolumeW(guid, ARRAYSIZE(guid));
> + if (handle == INVALID_HANDLE_VALUE) {
> + error_setg_win32(errp, GetLastError(), "failed to find any volume");
This is leaking memory allocated for resp.
> + return NULL;
> + }
> +
> + do {
> + GuestFilesystemTrimResult *res;
> + GuestFilesystemTrimResultList *list;
> + PWCHAR uc_path;
> + DWORD char_count = 0;
> + char *path, *out;
> + GError *gerr = NULL;
> + gchar * argv[4];
> +
> + GetVolumePathNamesForVolumeNameW(guid, NULL, 0, &char_count);
> +
> + if (GetLastError() != ERROR_MORE_DATA) {
> + continue;
> + }
> + if (GetDriveTypeW(guid) != DRIVE_FIXED) {
> + continue;
> + }
> +
> + uc_path = g_malloc0(sizeof(WCHAR) * char_count);
I suggest to use g_new here (no need to fill the memory with 0).
Regards
Stefan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-03 12:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-03 12:08 [Qemu-devel] [PATCH v3 1/1] qga: minimal support for fstrim for Windows guests Denis V. Lunev
2016-10-03 12:22 ` Marc-André Lureau
2016-10-03 12:39 ` Denis V. Lunev
2016-10-03 12:52 ` Stefan Weil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).