[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH] qga/commands-posix: Fix bug in guest-fstrim
Date: Tue, 31 Mar 2015 11:01:55 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

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:
).  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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]