Re: [Qemu-devel] [PATCH v4 0/2] Fix guest-fstrim behaviour

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Quoting Justin Ossevoort (2015-05-11 01:58:43)
> The qemu-ga 'guest-fstrim' command is currently not working properly.
> 
> There are 2 issues:
> - The current implementation reuses a struct between ioctl() calls without
>   reinitialising it's fields. This struct however is updated to reflect
>   the result of the trim operation.
>   Therefor only the first filesystem is thoroughly trimmed, the rest is only
>   trimmed up to the amount that was trimmed by the previous filesystem.
> - The current implementation will return an error if some filesystem returned
>   an unexpected error. The first issue consistently causes this issue when
>   the 'guest-fstrim' is performed multiple times in a row when multiple
>   filesystems are being trimmed, as this causes a trim request for at most
>   0 bytes, which is an error.

Applied, thanks.

> 
> The first patch fixes the first issue by explicitly resetting the struct used
> to perform the trim ioctl for each path. This is a pretty mundane change and
> fixes most use-cases.
> 
> The second patch fixes the second issue by changing the returned value to
> return a per-path result. This way all paths are always trimmed and dependening
> on the outcome of the ioctl an error or some details about the trim are
> returned. The returned values for error, minimum and trimmed are filesystem,
> storage stack and kernel version dependant.
> 
> There was an earlier request to mirror the fields from the 'guest-fsinfo'
> operation. The trim operation however need not happen at the mountpoint level.
> A logical future improvement would be to allow the caller to supply an optional
> list of paths they want to trim, without needing to have intimate details about
> the filesystem layout of the guest.
> 
> [Changes since v3]
> - Patch 2: Change return type of qmp_guest_fstrim in qga/command-win32.c
> - Patch 2: Change commit message on patch 2 to indicate returned values are
>            filesystem, storage stack and kernel version dependant
> 
> Justin Ossevoort (2):
>   qga/commands-posix: Fix bug in guest-fstrim
>   qga/commands-posix: Return per path fstrim result
> 
>  qga/commands-posix.c | 63 ++++++++++++++++++++++++++++++++++++----------------
>  qga/commands-win32.c |  4 +++-
>  qga/qapi-schema.json | 32 +++++++++++++++++++++++---
>  3 files changed, 76 insertions(+), 23 deletions(-)
> 
> -- 
> 2.1.4
> 
>