io: invert the return semantics of qio_channel_flush

With the kernel's zerocopy notification mechanism, the caller can
determine whether
 * All syscalls successfully used zero copy
 * At least one syscall failed to use zero copy

But, as of now QEMU's IO channel flush function semantics are like
 * 1 => all syscalls failed to use zero copy
 * 0 => at least one syscall successfully used zero copy

This is not aligned with what the kernel reports, and ends up reporting
false negatives for cases like when there's just a single successful
zerocopy amongst a collection of deferred zero-copies during a flush.

Fix this by inverting the return semantics of the IO flush function.

Suggested-by: Peter Xu <peterx@redhat.com>
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Tejus GK <tejus.gk@nutanix.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
index a1ef3136ea22f679dbb6a254a235e401cd469be3..b07cd614776f4e12ef75eb263b5510f95c2f6a5f 100644 (file)
--- a/include/io/channel-socket.h
+++ b/include/io/channel-socket.h
@@ -50,11 +50,7 @@ struct QIOChannelSocket {
     ssize_t zero_copy_queued;
     ssize_t zero_copy_sent;
     bool blocking;
-    /**
-     * This flag indicates whether any new data was successfully sent with
-     * zerocopy since the last qio_channel_socket_flush() call.
-     */
-    bool new_zero_copy_sent_success;
+    bool zero_copy_fallback;
 };
 
 

diff --git a/include/io/channel.h b/include/io/channel.h
index 287d10cd6faa46c8d900b2584405105aca02bc70..98485c9280a67068d21c3509d699088d255d2106 100644 (file)
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -1147,8 +1147,8 @@ int coroutine_mixed_fn qio_channel_writev_full_all(QIOChannel *ioc,
  * If not implemented, acts as a no-op, and returns 0.
  *
  * Returns -1 if any error is found,
- *          1 if every send failed to use zero copy.
- *          0 otherwise.
+ *          1 if at least one send failed to use zero copy.
+ *          0 if every send successfully used zero copy.
  */
 
 int qio_channel_flush(QIOChannel *ioc,

diff --git a/io/channel-socket.c b/io/channel-socket.c
index 3053b35ad858647674741e6f3caeb5430b3d4194..ea2ec84108223837d89e8a40b098ea6f8f3e3720 100644 (file)
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -72,7 +72,7 @@ qio_channel_socket_new(void)
     sioc->zero_copy_queued = 0;
     sioc->zero_copy_sent = 0;
     sioc->blocking = false;
-    sioc->new_zero_copy_sent_success = false;
+    sioc->zero_copy_fallback = false;
 
     ioc = QIO_CHANNEL(sioc);
     qio_channel_set_feature(ioc, QIO_CHANNEL_FEATURE_SHUTDOWN);
@@ -880,9 +880,9 @@ static int qio_channel_socket_flush_internal(QIOChannel *ioc,
         /* No errors, count successfully finished sendmsg()*/
         sioc->zero_copy_sent += serr->ee_data - serr->ee_info + 1;
 
-        /* If any sendmsg() succeeded using zero copy, mark zerocopy success */
-        if (serr->ee_code != SO_EE_CODE_ZEROCOPY_COPIED) {
-            sioc->new_zero_copy_sent_success = true;
+        if (serr->ee_code == SO_EE_CODE_ZEROCOPY_COPIED) {
+            /* If any sendmsg() fell back to a copy, mark fallback as true */
+            sioc->zero_copy_fallback = true;
         }
     }
 
@@ -900,12 +900,12 @@ static int qio_channel_socket_flush(QIOChannel *ioc,
         return ret;
     }
 
-    if (sioc->new_zero_copy_sent_success) {
-        sioc->new_zero_copy_sent_success = false;
-        return 0;
+    if (sioc->zero_copy_fallback) {
+        sioc->zero_copy_fallback = false;
+        return 1;
     }
 
-    return 1;
+    return 0;
 }
 
 #endif /* QEMU_MSG_ZEROCOPY */