[RESEND PATCH v2 1/1] rpc: Add the {repoll,retry} logic in virNetClientSetTLSSession

yong.huang@smartx.com posted 1 patch 7 months, 1 week ago
There is a newer version of this series
[RESEND PATCH v2 1/1] rpc: Add the {repoll,retry} logic in virNetClientSetTLSSession
Posted by yong.huang@smartx.com 7 months, 1 week ago
From: Hyman Huang <yong.huang@smartx.com>

As advised by the GNU TLS, the caller should attempt again
if the gnutls_record_{recv,send} return EAGAIN or EINTR;
check the following link to view the details:
https://www.gnutls.org/manual/html_node/Data-transfer-and-termination.html

virNetClientSetTLSSession failed to handle EINTR/EGAIN,
though EGAIN seems like it ought to be unlikely given that
the caller waited for G_IO_IN.

Add the {repoll, retry} logic to handle EINTR/EGAIN that
may happen theoretically. This may reduce the likelihood
that the upper application receives the following error
message utmostly when it calls the virConnectOpenAuth API:
Unable to read TLS confirmation: Resource temporarily unavailable

Note that in order to fully avoid the mentioned problem, the
upper application should retry virConnectOpenAuth.
---
 src/rpc/virnetclient.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 92933220e2..ee729d5e62 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -987,6 +987,7 @@ int virNetClientSetTLSSession(virNetClient *client,
      * etc.  If we make the grade, it will send us a '\1' byte.
      */
 
+ repoll:
     source = virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
                                         G_IO_IN,
                                         client->eventCtx,
@@ -1003,7 +1004,14 @@ int virNetClientSetTLSSession(virNetClient *client,
     ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
 #endif /* !WIN32 */
 
+ retry:
     len = virNetTLSSessionRead(client->tls, buf, 1);
+    if (len < 0 && errno == EINTR) {
+        goto retry;
+    }
+    if (len < 0 && errno == EAGAIN) {
+        goto repoll;
+    }
     if (len < 0 && errno != ENOMSG) {
         virReportSystemError(errno, "%s",
                              _("Unable to read TLS confirmation"));
-- 
2.27.0
Re: [RESEND PATCH v2 1/1] rpc: Add the {repoll,retry} logic in virNetClientSetTLSSession
Posted by Daniel P. Berrangé via Devel 7 months, 1 week ago
On Wed, May 14, 2025 at 11:24:22AM +0800, yong.huang@smartx.com wrote:
> From: Hyman Huang <yong.huang@smartx.com>
> 
> As advised by the GNU TLS, the caller should attempt again
> if the gnutls_record_{recv,send} return EAGAIN or EINTR;
> check the following link to view the details:
> https://www.gnutls.org/manual/html_node/Data-transfer-and-termination.html
> 
> virNetClientSetTLSSession failed to handle EINTR/EGAIN,
> though EGAIN seems like it ought to be unlikely given that
> the caller waited for G_IO_IN.
> 
> Add the {repoll, retry} logic to handle EINTR/EGAIN that
> may happen theoretically. This may reduce the likelihood
> that the upper application receives the following error
> message utmostly when it calls the virConnectOpenAuth API:
> Unable to read TLS confirmation: Resource temporarily unavailable
> 
> Note that in order to fully avoid the mentioned problem, the
> upper application should retry virConnectOpenAuth.

The patch is fine but this needs your Signed-off-by line
to state that you're contributing according to

   https://developercertificate.org/


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|