From a20e47682d1fa84b660c874eee4f86115485789c Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Thu, 22 Mar 2012 15:13:36 +0100
Subject: [PATCH] cmdlib: Stop forking in LUClusterQuery
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

While debugging another issue we realized that LUClusterQuery forks.
This turned out to be the β€œplatform.architecture” function from the
Python library. It uses the β€œfile” command to determine the architecture
of the Python binary.

This patch adds two new functions to the β€œruntime” module to get this
information once per process instead of doing it every single time
LUClusterQuery is used. Forking is a no-go in a multi-threaded
environment anyway.

A future change will also have to change the terminology in β€œgnt-cluster
info”: it reports the binary architecture simply as β€œarchitecture”, when
it's actually the binaries' architecture. Kernel and userland can be
different.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>
---
 lib/cmdlib.py                   |  4 ++--
 lib/runtime.py                  | 36 +++++++++++++++++++++++++++++++++
 lib/server/masterd.py           |  4 ++++
 test/ganeti.runtime_unittest.py | 33 ++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index c65da1d60..b8485e92f 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -32,7 +32,6 @@ import os
 import os.path
 import time
 import re
-import platform
 import logging
 import copy
 import OpenSSL
@@ -59,6 +58,7 @@ from ganeti import query
 from ganeti import qlang
 from ganeti import opcodes
 from ganeti import ht
+from ganeti import runtime
 
 import ganeti.masterd.instance # pylint: disable=W0611
 
@@ -5520,7 +5520,7 @@ class LUClusterQuery(NoHooksLU):
       "config_version": constants.CONFIG_VERSION,
       "os_api_version": max(constants.OS_API_VERSIONS),
       "export_version": constants.EXPORT_VERSION,
-      "architecture": (platform.architecture()[0], platform.machine()),
+      "architecture": runtime.GetArchInfo(),
       "name": cluster.cluster_name,
       "master": cluster.master_node,
       "default_hypervisor": cluster.enabled_hypervisors[0],
diff --git a/lib/runtime.py b/lib/runtime.py
index 9e478ec5e..8f6e51392 100644
--- a/lib/runtime.py
+++ b/lib/runtime.py
@@ -26,6 +26,7 @@
 import grp
 import pwd
 import threading
+import platform
 
 from ganeti import constants
 from ganeti import errors
@@ -35,6 +36,9 @@ from ganeti import utils
 _priv = None
 _priv_lock = threading.Lock()
 
+#: Architecture information
+_arch = None
+
 
 def GetUid(user, _getpwnam):
   """Retrieve the uid from the database.
@@ -187,3 +191,35 @@ def GetEnts(resolver=GetentResolver):
       _priv_lock.release()
 
   return _priv
+
+
+def InitArchInfo():
+  """Initialize architecture information.
+
+  We can assume this information never changes during the lifetime of a
+  process, therefore the information can easily be cached.
+
+  @note: This function uses C{platform.architecture} to retrieve the Python
+    binary architecture and does so by forking to run C{file} (see Python
+    documentation for more information). Therefore it must not be used in a
+    multi-threaded environment.
+
+  """
+  global _arch # pylint: disable=W0603
+
+  if _arch is not None:
+    raise errors.ProgrammerError("Architecture information can only be"
+                                 " initialized once")
+
+  _arch = (platform.architecture()[0], platform.machine())
+
+
+def GetArchInfo():
+  """Returns previsouly initialized architecture information.
+
+  """
+  if _arch is None:
+    raise errors.ProgrammerError("Architecture information hasn't been"
+                                 " initialized")
+
+  return _arch
diff --git a/lib/server/masterd.py b/lib/server/masterd.py
index 3ca50dfc0..7b2336fae 100644
--- a/lib/server/masterd.py
+++ b/lib/server/masterd.py
@@ -57,6 +57,7 @@ from ganeti import bootstrap
 from ganeti import netutils
 from ganeti import objects
 from ganeti import query
+from ganeti import runtime
 
 
 CLIENT_REQUEST_WORKERS = 16
@@ -548,6 +549,9 @@ def CheckMasterd(options, args):
                           (constants.MASTERD_USER, constants.DAEMONS_GROUP))
     sys.exit(constants.EXIT_FAILURE)
 
+  # Determine static runtime architecture information
+  runtime.InitArchInfo()
+
   # Check the configuration is sane before anything else
   try:
     config.ConfigWriter()
diff --git a/test/ganeti.runtime_unittest.py b/test/ganeti.runtime_unittest.py
index 79ede58bc..73f4143cc 100755
--- a/test/ganeti.runtime_unittest.py
+++ b/test/ganeti.runtime_unittest.py
@@ -23,6 +23,7 @@
 from ganeti import constants
 from ganeti import errors
 from ganeti import runtime
+from ganeti import ht
 
 import testutils
 import unittest
@@ -138,5 +139,37 @@ class TestErrors(unittest.TestCase):
                       self.resolver.LookupGroup, "does-not-exist-foo")
 
 
+class TestArchInfo(unittest.TestCase):
+  EXP_TYPES = \
+    ht.TAnd(ht.TIsLength(2),
+            ht.TItems([
+              ht.TNonEmptyString,
+              ht.TNonEmptyString,
+              ]))
+
+  def setUp(self):
+    self.assertTrue(runtime._arch is None)
+
+  def tearDown(self):
+    runtime._arch = None
+
+  def testNotInitialized(self):
+    self.assertRaises(errors.ProgrammerError, runtime.GetArchInfo)
+
+  def testInitializeMultiple(self):
+    runtime.InitArchInfo()
+
+    self.assertRaises(errors.ProgrammerError, runtime.InitArchInfo)
+
+  def testNormal(self):
+    runtime.InitArchInfo()
+
+    info = runtime.GetArchInfo()
+
+    self.assertTrue(self.EXP_TYPES(info),
+                    msg=("Doesn't match expected type description: %s" %
+                         self.EXP_TYPES))
+
+
 if __name__ == "__main__":
   testutils.GanetiTestProgram()
-- 
GitLab