On 31/03/2015 17:14, 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 <jus...@quarantainenet.nl> > --- > qga/commands-posix.c | 19 +++++++++++-------- > qga/qapi-schema.json | 5 +++-- > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index ba8de62..5b11ecf 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -1325,18 +1325,15 @@ static void guest_fsfreeze_cleanup(void) > /* > * Walk list of mounted file systems in the guest, and trim them. > */ > -void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) > +int64_t qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) > { > int ret = 0; > FsMountList mounts; > struct FsMount *mount; > int fd; > Error *local_err = NULL; > - struct fstrim_range r = { > - .start = 0, > - .len = -1, > - .minlen = has_minimum ? minimum : 0, > - }; > + struct fstrim_range r; > + int64_t trimmed = 0; > > slog("guest-fstrim called"); > > @@ -1344,7 +1341,7 @@ void qmp_guest_fstrim(bool has_minimum, int64_t > minimum, Error **errp) > build_fs_mount_list(&mounts, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return; > + return 0; > } > > QTAILQ_FOREACH(mount, &mounts, next) { > @@ -1360,6 +1357,9 @@ void qmp_guest_fstrim(bool has_minimum, int64_t > minimum, Error **errp) > * error means an unexpected error, so return it in those cases. In > * some other cases ENOTTY will be reported (e.g. CD-ROMs). > */ > + r.start = 0; > + r.len = -1; > + r.minlen = has_minimum ? minimum : 0; > ret = ioctl(fd, FITRIM, &r); > if (ret == -1) { > if (errno != ENOTTY && errno != EOPNOTSUPP) { > @@ -1370,10 +1370,12 @@ void qmp_guest_fstrim(bool has_minimum, int64_t > minimum, Error **errp) > } > } > close(fd); > + trimmed += r.len; > } > > error: > free_fs_mount_list(&mounts); > + return trimmed; > } > #endif /* CONFIG_FSTRIM */ > > @@ -2402,9 +2404,10 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp) > #endif /* CONFIG_FSFREEZE */ > > #if !defined(CONFIG_FSTRIM) > -void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) > +int64_t qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) > { > error_set(errp, QERR_UNSUPPORTED); > + return 0; > } > #endif > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index 95f49e3..48953f4 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -437,12 +437,13 @@ > # fragmented free space, although not all blocks will be discarded. > # The default value is zero, meaning "discard every free block". > # > -# Returns: Nothing. > +# Returns: Number of bytes trimmed by this call.
It's better to add "(since 2.4)" here. Apart from this, looks good. I'm CCing the qemu-ga maintainer. Reviewed-by: Paolo Bonzini <pbonz...@redhat.com> Paolo > # > # Since: 1.2 > ## > { 'command': 'guest-fstrim', > - 'data': { '*minimum': 'int' } } > + 'data': { '*minimum': 'int' }, > + 'returns': 'int' } > > ## > # @guest-suspend-disk >