Change in vdsm[ovirt-3.3]: BindingXMLRPC bugfix: Return lastClientIface for current con...

amuller at redhat.com amuller at redhat.com
Sun Jan 5 15:33:06 UTC 2014


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/22967

to review the following change.

Change subject: BindingXMLRPC bugfix: Return lastClientIface for current connection
......................................................................

BindingXMLRPC bugfix: Return lastClientIface for current connection

The patch does two things:
1) Fixes a bug where lastClientIface was for the previous
   connection, and not the current one. The method to retrieve
   the last client was through a log_request LoggingMixIn
   which is invoked at the end of a response, not at the beginning.
   The patch removes the usage of that MixIn, and instead uses
   values that are updated at the beginning of the connection
2) Simplifies the 'LoggingMixin' out of the picture as its no
   longer needed

Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca
Signed-off-by: Assaf Muller <amuller at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/22837
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/BindingXMLRPC.py
1 file changed, 16 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/22967/1

diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py
index 5ee7126..666cc95 100644
--- a/vdsm/BindingXMLRPC.py
+++ b/vdsm/BindingXMLRPC.py
@@ -80,6 +80,15 @@
         self._thread.join()
         return {'status': doneCode}
 
+    def updateTimestamp(self):
+        # FIXME: The setup+editNetwork API uses this log file to
+        # determine if this host is still accessible.  We use a
+        # file (rather than an event) because setup+editNetwork is
+        # performed by a separate, root process.  To clean this
+        # up we need to move this to an API wrapper that is only
+        # run for real clients (not vdsm internal API calls).
+        file(constants.P_VDSM_CLIENT_LOG, 'w')
+
     def _getKeyCertFilenames(self):
         """
         Get the locations of key and certificate files.
@@ -97,29 +106,16 @@
 
         threadLocal = self.cif.threadLocal
 
-        class LoggingMixIn:
-
-            def log_request(self, code='-', size='-'):
-                """Track from where client connections are coming."""
-                self.server.lastClient = self.client_address[0]
-                self.server.lastServerIP = self.request.getsockname()[0]
-                # FIXME: The editNetwork API uses this log file to
-                # determine if this host is still accessible.  We use a
-                # file (rather than an event) because editNetwork is
-                # performed by a separate, root process.  To clean this
-                # up we need to move this to an API wrapper that is only
-                # run for real clients (not vdsm internal API calls).
-                file(constants.P_VDSM_CLIENT_LOG, 'w')
-
         server_address = (self.serverIP, int(self.serverPort))
         if self.enableSSL:
             basehandler = SecureXMLRPCServer.SecureXMLRPCRequestHandler
         else:
             basehandler = SimpleXMLRPCServer.SimpleXMLRPCRequestHandler
 
-        class LoggingHandler(LoggingMixIn, basehandler):
+        class LoggingHandler(basehandler):
             def setup(self):
                 threadLocal.client = self.client_address[0]
+                threadLocal.server = self.request.getsockname()[0]
                 return basehandler.setup(self)
 
             def parse_request(self):
@@ -149,6 +145,7 @@
     def _registerFunctions(self):
         def wrapIrsMethod(f):
             def wrapper(*args, **kwargs):
+                self.updateTimestamp()
                 fmt = ""
                 logargs = []
 
@@ -301,8 +298,9 @@
         api = API.Global()
         ret = api.getCapabilities()
         ret['info']['management_ip'] = self.serverIP
-        ret['info']['lastClient'] = self.server.lastClient
-        ret['info']['lastClientIface'] = getIfaceByIP(self.server.lastServerIP)
+        ret['info']['lastClient'] = self.cif.threadLocal.client
+        ret['info']['lastClientIface'] = getIfaceByIP(
+            self.cif.threadLocal.server)
         return ret
 
     def getHardwareInfo(self):
@@ -940,6 +938,7 @@
 def wrapApiMethod(f):
     def wrapper(*args, **kwargs):
         try:
+            f.im_self.updateTimestamp()
             logLevel = logging.DEBUG
             if f.__name__ in ('getVMList', 'getAllVmStats', 'getStats',
                               'fenceNode'):


-- 
To view, visit http://gerrit.ovirt.org/22967
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I36e48cc07a27f44a2b413f0e9159110404f1b0ca
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list