[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] qga: minimal support for fstrim for Windows
From: |
Denis V. Lunev |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] qga: minimal support for fstrim for Windows guests |
Date: |
Thu, 29 Sep 2016 19:08:41 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 09/29/2016 03:21 PM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Sep 29, 2016 at 2:30 PM Denis V. Lunev <address@hidden> 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 <address@hidden>
>> Signed-off-by: Denis Plotnikov <address@hidden>
>> CC: Michael Roth <address@hidden>
>> CC: Stefan Weil <address@hidden>
>> ---
>> qga/commands-win32.c | 94
>> ++++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 91 insertions(+), 3 deletions(-)
>>
>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>> index 9c9be12..7144bac 100644
>> --- a/qga/commands-win32.c
>> +++ b/qga/commands-win32.c
>> @@ -840,8 +840,96 @@ 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));
>>
> qga doesn't use unicode functions, I wonder if we shouldn't fix that first
> to be the default.
>
> I also wonder how it currently impacts qmp_guest_set_user_password() which
> seems to handle unicode but call the ansi variant.
ah, I think that this is separate matter completely.
In general, qmp_guest_get_fsinfo is rotten. We have
stub code, but its factorizing to normal way
takes a lot of time :(
>
>> + if (handle == INVALID_HANDLE_VALUE) {
>> + return resp;
>>
> no error returned?
>
> btw, there is similar call in qmp_guest_get_fsinfo(), perhaps it can be
> factorized
ok. reasonable.
> + }
>> +
>> + 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 +1504,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};
>>
> looks fine, if it fails on <win8/2k12, it will return an error
>
>
>> char **p = (char **)list_unsupported;
>>
>> while (*p) {
>> --
>> 2.7.4
>>
>>
>> --
> Marc-André Lureau