Re: [Qemu-devel] [PATCH v3 1/1] qga: minimal support for fstrim for Windows guests

[Stefan Weil]

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 <address@hidden>
> Signed-off-by: Denis Plotnikov <address@hidden>
> CC: Michael Roth <address@hidden>
> CC: Stefan Weil <address@hidden>
> CC: Marc-André Lureau <address@hidden>
> ---
>  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