Change in vdsm[master]: xmlrpc: retry when flushing socket

[Piotr Kliczewski]

Piotr Kliczewski has uploaded a new change for review.

Change subject: xmlrpc: retry when flushing socket
......................................................................

xmlrpc: retry when flushing socket

m2crypto requires retry of write and we need to make sure that we do it once
flushing data before closing a socket. In order to do so we need to move
original implementation to our code base.


Bug-Url: https://bugzilla.redhat.com/1261255
Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com&gt;
---
M vdsm/rpc/bindingxmlrpc.py
1 file changed, 19 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/47637/2

diff --git a/vdsm/rpc/bindingxmlrpc.py b/vdsm/rpc/bindingxmlrpc.py
index c521cd8..9f77457 100644
--- a/vdsm/rpc/bindingxmlrpc.py
+++ b/vdsm/rpc/bindingxmlrpc.py
@@ -25,6 +25,7 @@
 import libvirt
 import threading
 import re
+import socket
 import sys
 
 from vdsm.password import (ProtectedPassword,
@@ -33,6 +34,7 @@
 from vdsm import utils
 from vdsm import xmlrpc
 from vdsm.define import doneCode, errCode
+from vdsm.m2cutils import SSL
 from vdsm.netinfo import getDeviceByIP
 import API
 from vdsm.exception import VdsmException
@@ -288,7 +290,23 @@
                 return r
 
             def finish(self):
-                xmlrpc.IPXMLRPCRequestHandler.finish(self)
+                if not self.wfile.closed:
+                    while True:
+                        try:
+                            self.wfile.flush()
+                            break
+                        except socket.error:
+                            # An final socket error may have occurred here,
+                            # such as the local error ECONNABORTED.
+                            break
+                        except SSL.SSLError as e:
+                            if e.message == 'bad write retry':
+                                continue
+                            else:
+                                break
+
+                self.wfile.close()
+                self.rfile.close()
                 threadLocal.client = None
                 threadLocal.server = None
                 threadLocal.flowID = None


-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org

automation(a)ovirt.org has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 2:

* Update tracker::#1261255::OK
* Check Bug-Url::OK
* Check Public Bug::#1261255::OK, public bug
* Check Product::#1261255::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 2: Verified+1

Verified on broken host when this error occurs.

-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/47637/2/vdsm/rpc/bindingxmlrpc.py
File vdsm/rpc/bindingxmlrpc.py:

Line 299:                             # An final socket error may have occurred here,
Line 300:                             # such as the local error ECONNABORTED.
Line 301:                             break
Line 302:                         except SSL.SSLError as e:
Line 303:                             if e.message == 'bad write retry':
we don't have any other mean to do this check? an error code, maybe?
Line 304:                                 continue
Line 305:                             else:
Line 306:                                 break
Line 307: 


-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/47637/2/vdsm/rpc/bindingxmlrpc.py
File vdsm/rpc/bindingxmlrpc.py:

Line 299:                             # An final socket error may have occurred here,
Line 300:                             # such as the local error ECONNABORTED.
Line 301:                             break
Line 302:                         except SSL.SSLError as e:
Line 303:                             if e.message == 'bad write retry':
>  we don't have any other mean to do this check? an error code,
> maybe? 
As you see m2crypto is very good at giving us error code. (everything is in
message).
Line 304:                                 continue
Line 305:                             else:
Line 306:                                 break
Line 307: 


-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

Yaniv Bronhaim has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/47637/2//COMMIT_MSG
Commit Message:

Line 7: xmlrpc: retry when flushing socket
Line 8: 
Line 9: m2crypto requires retry of write and we need to make sure that we do it once
Line 10: flushing data before closing a socket. In order to do so we need to move
Line 11: original implementation to our code base.
can you specify where is the original code exactly ?
Line 12: 
Line 13: 
Line 14: Bug-Url: https://bugzilla.redhat.com/1261255
Line 15: Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055


https://gerrit.ovirt.org/#/c/47637/2/vdsm/rpc/bindingxmlrpc.py
File vdsm/rpc/bindingxmlrpc.py:

Line 287
Line 288
Line 289
Line 290
Line 291
what did this call do? we don't need it anymore?


-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/47637/2//COMMIT_MSG
Commit Message:

Line 7: xmlrpc: retry when flushing socket
Line 8: 
Line 9: m2crypto requires retry of write and we need to make sure that we do it once
Line 10: flushing data before closing a socket. In order to do so we need to move
Line 11: original implementation to our code base.
>  can you specify where is the original code exactly ? 
Will add.
Line 12: 
Line 13: 
Line 14: Bug-Url: https://bugzilla.redhat.com/1261255
Line 15: Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055


https://gerrit.ovirt.org/#/c/47637/2/vdsm/rpc/bindingxmlrpc.py
File vdsm/rpc/bindingxmlrpc.py:

Line 287
Line 288
Line 289
Line 290
Line 291
>  what did this call do? we don't need it anymore? 
We moved
code from this method here to be able to handle ssl retry issue.


-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

automation(a)ovirt.org has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 3:

* Update tracker::#1261255::OK
* Check Bug-Url::OK
* Check Public Bug::#1261255::OK, public bug
* Check Product::#1261255::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TM::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Piotr Kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: xmlrpc: retry when flushing socket
......................................................................


Patch Set 8:

Let's keep it for a bit longer.

-- 
To view, visit https://gerrit.ovirt.org/47637
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No