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

[Justin Ossevoort]

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>
---
 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.
 #
 # Since: 1.2
 ##
 { 'command': 'guest-fstrim',
-  'data': { '*minimum': 'int' } }
+  'data': { '*minimum': 'int' },
+  'returns': 'int' }
 
 ##
 # @guest-suspend-disk
-- 
2.1.0