Re: [Qemu-devel] [PATCH] qga/commands-posix: Fix bug in guest-fstrim

[Eric Blake]

On 03/31/2015 09:14 AM, Justin Ossevoort wrote:
> The FITRIM ioctl updates the fstrim_range structure it receives. This
> way the caller can determine how many bytes were trimmed. The
> guest-fstrim logic reuses the same fstrim_range for each filesystem,
> effectively limiting each filesystem to trim at most as much as the
> previous was able to trim.
> 
> If a previous filesystem would have trimmed 0 bytes, than the next
> filesystem would report an error 'Invalid argument' because a FITRIM
> request with length 0 is not valid.
> 
> This change resets the fstrim_range structure for each filesystem. It
> also returns all bytes trimmed for all filesystems, providing a hint to
> the caller about how effective the guest-fstrim request was.
> 
> Signed-off-by: Justin Ossevoort <address@hidden>
> ---

> -# Returns: Nothing.
> +# Returns: Number of bytes trimmed by this call.
>  #
>  # Since: 1.2
>  ##
>  { 'command': 'guest-fstrim',
> -  'data': { '*minimum': 'int' } }
> +  'data': { '*minimum': 'int' },
> +  'returns': 'int' }

No. Please don't add any new non-dictionary returns (see my pending
patch that would flag this as not being on the whitelist:
https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg05046.html
).  This is particularly true for an already existing command (clients
might not be prepared for older guest agent returning an empty
dictionary in place of an int; but SHOULD be prepared to see if the
returned dictionary is empty).

Better would be to add a key to the returned dictionary that reports
either total amount trimmed, or even better, an array reporting stats
for each file system trimmed; something like:

{ "return": { "trimmed":
   [ { "name":"abc", "mountpoint":"xyz", "trimmed":123 },
     { "name":"def", "mountpoint":"pqr", "trimmed":456 } ] } }

where the 'name' and 'mountpoint' keys of each array element correspond
to the data given by the 'GuestFilesystemInfo' type in
'guest-get-fsinfo' (so that the two commands have correlated information).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature