Change in vdsm[master]: netinfo: reduce netinfo instantiation to be linear
by asegurap＠redhat.com
Antoni Segura Puimedon has uploaded a new change for review.
Change subject: netinfo: reduce netinfo instantiation to be linear
......................................................................
netinfo: reduce netinfo instantiation to be linear
Change-Id: I424e0ba9a99951432dea0365e12a02b7fb7a5bf7
Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com>
---
M lib/vdsm/netinfo.py
M vdsm/netconf/ifcfg.py
M vdsm/netconf/iproute2.py
M vdsm/netmodels.py
4 files changed, 38 insertions(+), 24 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/77/23577/1
diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index 094c089..5d18ab0 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -43,7 +43,7 @@
from .ipwrapper import linkShowDev
from .utils import anyFnmatch
from .netconfpersistence import RunningConfig
-from .netlink import iter_addrs
+from .netlink import iter_addrs, iter_links
NET_CONF_DIR = '/etc/sysconfig/network-scripts/'
@@ -813,3 +813,23 @@
if iface == vdict['iface']:
users.add(v)
return users
+
+
+def anyIfaceUsers(iface):
+ if os.path.exists(os.path.join(NET_PATH, iface, 'brport')): # Is it a port
+ return True
+ for linkDict in iter_links():
+ if linkDict['name'] == iface and 'master' in linkDict: # Is it a slave
+ return True
+ if linkDict.get('device') == iface: # Does it back a vlan
+ return True
+ for name, info in networks().iteritems():
+ if info.get('iface') == iface:
+ return True
+ return False
+
+
+def vlanDevsForIface(iface):
+ for linkDict in iter_links():
+ if linkDict.get('device') == iface:
+ yield linkDict['name']
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index c5e6074..0d8a0e1 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -166,16 +166,15 @@
self.configApplier.removeVlan(vlan.name)
vlan.device.remove()
- def _ifaceDownAndCleanup(self, iface, _netinfo):
+ def _ifaceDownAndCleanup(self, iface):
"""Returns True iff the iface is to be removed."""
DynamicSourceRoute.addInterfaceTracking(iface)
ifdown(iface.name)
self._removeSourceRoute(iface)
- return not _netinfo.ifaceUsers(iface.name)
+ return not netinfo.anyIfaceUsers(iface.name)
def removeBond(self, bonding):
- _netinfo = netinfo.NetInfo()
- to_be_removed = self._ifaceDownAndCleanup(bonding, _netinfo)
+ to_be_removed = self._ifaceDownAndCleanup(bonding)
if to_be_removed:
if bonding.destroyOnMasterRemoval:
self.configApplier.removeBonding(bonding.name)
@@ -189,16 +188,15 @@
ifup(bonding.name)
else:
self._setNewMtu(bonding,
- _netinfo.getVlanDevsForIface(bonding.name))
+ netinfo.vlanDevsForIface(bonding.name))
ifup(bonding.name)
def removeNic(self, nic):
- _netinfo = netinfo.NetInfo()
- to_be_removed = self._ifaceDownAndCleanup(nic, _netinfo)
+ to_be_removed = self._ifaceDownAndCleanup(nic)
if to_be_removed:
self.configApplier.removeNic(nic.name)
else:
- self._setNewMtu(nic, _netinfo.getVlanDevsForIface(nic.name))
+ self._setNewMtu(nic, netinfo.vlanDevsForIface(nic.name))
ifup(nic.name)
def _getFilePath(self, fileType, device):
@@ -604,7 +602,7 @@
if bond.bridge:
conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name)
- ipconfig, mtu = self._getIfaceConfValues(bond, netinfo.NetInfo())
+ ipconfig, mtu = self._getIfaceConfValues(bond)
self._createConfFile(conf, bond.name, ipconfig, mtu, **opts)
# create the bonding device to avoid initscripts noise
@@ -614,9 +612,9 @@
def addNic(self, nic, **opts):
""" Create ifcfg-* file with proper fields for NIC """
- _netinfo = netinfo.NetInfo()
conf = ''
if _hwaddr_required():
+ _netinfo = netinfo.NetInfo()
hwaddr = (_netinfo.nics[nic.name].get('permhwaddr') or
_netinfo.nics[nic.name]['hwaddr'])
@@ -626,17 +624,17 @@
if nic.bond:
conf += 'MASTER=%s\nSLAVE=yes\n' % pipes.quote(nic.bond.name)
- ipconfig, mtu = self._getIfaceConfValues(nic, _netinfo)
+ ipconfig, mtu = self._getIfaceConfValues(nic)
self._createConfFile(conf, nic.name, ipconfig, mtu, **opts)
@staticmethod
- def _getIfaceConfValues(iface, _netinfo):
+ def _getIfaceConfValues(iface):
ipaddr, netmask, gateway, defaultRoute, ipv6addr, ipv6gateway, \
ipv6defaultRoute, bootproto, async, ipv6autoconf, dhcpv6 = \
iface.ipConfig
defaultRoute = ConfigWriter._toIfcfgFormat(defaultRoute)
mtu = iface.mtu
- if _netinfo.ifaceUsers(iface.name):
+ if netinfo.anyIfaceUsers(iface.name):
confParams = netinfo.getIfaceCfg(iface.name)
if not ipaddr and bootproto != 'dhcp':
ipaddr = confParams.get('IPADDR', None)
diff --git a/vdsm/netconf/iproute2.py b/vdsm/netconf/iproute2.py
index 86727d3..508c0c0 100644
--- a/vdsm/netconf/iproute2.py
+++ b/vdsm/netconf/iproute2.py
@@ -131,8 +131,7 @@
self.configApplier.removeBond(bonding)
def removeBond(self, bonding):
- _netinfo = netinfo.NetInfo()
- toBeRemoved = not _netinfo.ifaceUsers(bonding.name)
+ toBeRemoved = not netinfo.anyIfaceUsers(bonding.name)
if toBeRemoved:
if bonding.master is None:
@@ -146,12 +145,10 @@
netinfo.DEFAULT_MTU)
self.configApplier.ifdown(bonding)
else:
- self._setNewMtu(bonding,
- _netinfo.getVlanDevsForIface(bonding.name))
+ self._setNewMtu(bonding, netinfo.vlanDevsForIface(bonding.name))
def removeNic(self, nic):
- _netinfo = netinfo.NetInfo()
- toBeRemoved = not _netinfo.ifaceUsers(nic.name)
+ toBeRemoved = not netinfo.anyIfaceUsers(nic.name)
if toBeRemoved:
if nic.master is None:
@@ -161,8 +158,7 @@
netinfo.DEFAULT_MTU)
self.configApplier.ifdown(nic)
else:
- self._setNewMtu(nic,
- _netinfo.getVlanDevsForIface(nic.name))
+ self._setNewMtu(nic, netinfo.vlanDevsForIface(nic.name))
@staticmethod
def configureSourceRoute(routes, rules, device):
diff --git a/vdsm/netmodels.py b/vdsm/netmodels.py
index 5fde4e0..25b6963 100644
--- a/vdsm/netmodels.py
+++ b/vdsm/netmodels.py
@@ -89,7 +89,7 @@
# in a limited condition, we should not touch the nic config
if (self.vlan and
netinfo.operstate(self.name) == netinfo.OPERSTATE_UP and
- netinfo.NetInfo().ifaceUsers(self.name) and
+ netinfo.anyIfaceUsers(self.name) and
self.mtu <= netinfo.getMtu(self.name)):
return
@@ -195,7 +195,7 @@
if (self.vlan and
self.name in netinfo.bondings() and
netinfo.operstate(self.name) == netinfo.OPERSTATE_UP and
- netinfo.NetInfo().ifaceUsers(self.name) and
+ netinfo.anyIfaceUsers(self.name) and
self.mtu <= netinfo.getMtu(self.name) and
self.areOptionsApplied() and
frozenset(slave.name for slave in self.slaves) ==
--
To view, visit http://gerrit.ovirt.org/23577
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I424e0ba9a99951432dea0365e12a02b7fb7a5bf7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap(a)redhat.com>
10 years, 2 months
Change in vdsm[master]: tests: Add blockSD tests
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: tests: Add blockSD tests
......................................................................
tests: Add blockSD tests
Add test for the metadataValidity before modiying its confusing
implementation.
Change-Id: I31838ac5847e464ee6ba3e1b3bd85312b4f9cee7
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M tests/Makefile.am
A tests/blocksdTests.py
2 files changed, 51 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/25207/1
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 379e9fd..37d1527 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -25,6 +25,7 @@
test_modules = \
alignmentScanTests.py \
apiTests.py \
+ blocksdTests.py \
cPopenTests.py \
capsTests.py \
clientifTests.py \
diff --git a/tests/blocksdTests.py b/tests/blocksdTests.py
new file mode 100644
index 0000000..9c09bd7
--- /dev/null
+++ b/tests/blocksdTests.py
@@ -0,0 +1,50 @@
+#
+# Copyright 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import collections
+from storage import blockSD
+from vdsm import constants
+from testrunner import VdsmTestCase as TestCaseBase
+
+# Make it easy to test the values we care about
+VG = collections.namedtuple("VG", ['vg_mda_size', 'vg_mda_free'])
+
+
+class BlockSDTests(TestCaseBase):
+
+ MIN_MD_SIZE = blockSD.VG_METADATASIZE * constants.MEGAB / 2
+ MIN_MD_FREE = MIN_MD_SIZE * blockSD.VG_MDA_MIN_THRESHOLD
+
+ def test_metadataValidity_valid_ok(self):
+ vg = VG(self.MIN_MD_SIZE, self.MIN_MD_FREE)
+ self.assertEquals(True, blockSD.metadataValidity(vg)['mdavalid'])
+
+ def test_metadataValidity_valid_bad(self):
+ vg = VG(self.MIN_MD_SIZE - 1, self.MIN_MD_FREE)
+ self.assertEquals(False, blockSD.metadataValidity(vg)['mdavalid'])
+
+ def test_metadataValidity_threshold_ok(self):
+ vg = VG(self.MIN_MD_SIZE, self.MIN_MD_FREE + 1)
+ self.assertEquals(True, blockSD.metadataValidity(vg)['mdathreshold'])
+
+ def test_metadataValidity_threshold_bad(self):
+ vg = VG(self.MIN_MD_SIZE, self.MIN_MD_FREE)
+ self.assertEquals(False, blockSD.metadataValidity(vg)['mdathreshold'])
--
To view, visit http://gerrit.ovirt.org/25207
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I31838ac5847e464ee6ba3e1b3bd85312b4f9cee7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
10 years, 2 months
Change in vdsm[master]: net tests: more combinations mtu combinations
by Dan Kenigsberg
Hello Mark Wu,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/19230
to review the following change.
Change subject: net tests: more combinations mtu combinations
......................................................................
net tests: more combinations mtu combinations
The tests were splitted from the former functional fix in order to
expedite the merge of the latter one.
Change-Id: Id7a1f5b628261ae4ebf400494bcdb2939a9cd535
Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M tests/functional/networkTests.py
1 file changed, 114 insertions(+), 25 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/19230/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 3825bf5..dff7ce2 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -1024,48 +1024,62 @@
{})
self.assertEquals(status, SUCCESS, msg)
+ def _createBondedNetAndCheck(self, netNum, bondDict, bridged,
+ **networkOpts):
+ netName = NETWORK_NAME + str(netNum)
+ networks = {netName: dict(bonding=BONDING_NAME, bridged=bridged,
+ vlan=str(int(VLAN_ID) + netNum),
+ **networkOpts)}
+ status, msg = self.vdsm_net.setupNetworks(networks,
+ {BONDING_NAME: bondDict},
+ {})
+ self.assertEquals(status, SUCCESS, msg)
+ self.vdsm_net.networkExists(netName, bridged=bridged)
+ self.vdsm_net.bondExists(BONDING_NAME, bondDict['nics'])
+ if 'mtu' in networkOpts:
+ self.assertEquals(networkOpts['mtu'],
+ self.vdsm_net.getMtu(netName))
+
@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
def testSetupNetworksStableBond(self, bridged):
- def createBondedNetAndCheck(netNum, bondDict):
- netName = NETWORK_NAME + str(netNum)
- networks = {netName: dict(bonding=BONDING_NAME, bridged=bridged,
- vlan=str(int(VLAN_ID) + netNum))}
- status, msg = self.vdsm_net.setupNetworks(networks,
- {BONDING_NAME: bondDict},
- {})
- self.assertEquals(status, SUCCESS, msg)
- self.vdsm_net.networkExists(netName, bridged=bridged)
- self.vdsm_net.bondExists(BONDING_NAME, bondDict['nics'])
-
with dummyIf(3) as nics:
with self.vdsm_net.pinger():
# Add initial vlanned net over bond
- createBondedNetAndCheck(0, {'nics': nics[:2],
- 'options': 'mode=3 miimon=250'})
+ self._createBondedNetAndCheck(0, {'nics': nics[:2],
+ 'options': 'mode=3 miimon=250'},
+ bridged)
with nonChangingOperstate(BONDING_NAME):
# Add additional vlanned net over the bond
- createBondedNetAndCheck(1,
- {'nics': nics[:2],
- 'options': 'mode=3 miimon=250'})
+ self._createBondedNetAndCheck(1,
+ {'nics': nics[:2],
+ 'options':
+ 'mode=3 miimon=250'},
+ bridged)
# Add additional vlanned net over the increasing bond
- createBondedNetAndCheck(2,
- {'nics': nics,
- 'options': 'mode=3 miimon=250'})
+ self._createBondedNetAndCheck(2,
+ {'nics': nics,
+ 'options':
+ 'mode=3 miimon=250'},
+ bridged)
# Add additional vlanned net over the changing bond
- createBondedNetAndCheck(3,
- {'nics': nics[1:],
- 'options': 'mode=3 miimon=250'})
+ self._createBondedNetAndCheck(3,
+ {'nics': nics[1:],
+ 'options':
+ 'mode=3 miimon=250'},
+ bridged)
# Add a network changing bond options
with self.assertRaises(OperStateChangedError):
with nonChangingOperstate(BONDING_NAME):
- createBondedNetAndCheck(4,
- {'nics': nics[1:],
- 'options': 'mode=4 miimon=9'})
+ self._createBondedNetAndCheck(4,
+ {'nics': nics[1:],
+ 'options':
+ 'mode=4 miimon=9'},
+ bridged)
# cleanup
networks = dict((NETWORK_NAME + str(num), {'remove': True}) for
@@ -1078,6 +1092,81 @@
@cleanupNet
@permutations([[True], [False]])
+ @RequireDummyMod
+ @ValidateRunningAsRoot
+ def testSetupNetworksMultiMTUsOverBond(self, bridged):
+ with dummyIf(2) as nics:
+ with self.vdsm_net.pinger():
+ # Add initial vlanned net over bond
+ self._createBondedNetAndCheck(0, {'nics': nics}, bridged,
+ mtu='1500')
+ self.assertEquals('1500',
+ self.vdsm_net.getMtu(BONDING_NAME))
+
+ with nonChangingOperstate(BONDING_NAME):
+ # Add a network with MTU smaller than existing network
+ self._createBondedNetAndCheck(1, {'nics': nics},
+ bridged, mtu='1400')
+ self.assertEquals('1500',
+ self.vdsm_net.getMtu(BONDING_NAME))
+
+ # Add a network with MTU bigger than existing network
+ self._createBondedNetAndCheck(2, {'nics': nics},
+ bridged, mtu='1600')
+ self.assertEquals('1600',
+ self.vdsm_net.getMtu(BONDING_NAME))
+
+ # cleanup
+ networks = dict((NETWORK_NAME + str(num), {'remove': True}) for
+ num in range(3))
+ status, msg = self.vdsm_net.setupNetworks(networks,
+ {BONDING_NAME:
+ dict(remove=True)},
+ {})
+ self.assertEquals(status, SUCCESS, msg)
+
+ def _createVlanedNetOverNicAndCheck(self, netNum, bridged, **networkOpts):
+ netName = NETWORK_NAME + str(netNum)
+ networks = {netName: dict(bridged=bridged,
+ vlan=str(int(VLAN_ID) + netNum),
+ **networkOpts)}
+ status, msg = self.vdsm_net.setupNetworks(networks, {}, {})
+ self.assertEquals(status, SUCCESS, msg)
+ self.vdsm_net.networkExists(netName, bridged=bridged)
+ if 'mtu' in networkOpts:
+ self.assertEquals(networkOpts['mtu'],
+ self.vdsm_net.getMtu(netName))
+
+ @cleanupNet
+ @permutations([[True], [False]])
+ @RequireDummyMod
+ @ValidateRunningAsRoot
+ def testSetupNetworksMultiMTUsOverNic(self, bridged):
+ with dummyIf(1) as nics:
+ nic, = nics
+ with self.vdsm_net.pinger():
+ # Add initial vlanned net over bond
+ self._createVlanedNetOverNicAndCheck(0, bridged, nic=nic,
+ mtu='1500')
+ self.assertEquals('1500', self.vdsm_net.getMtu(nic))
+
+ # Add a network with MTU smaller than existing network
+ self._createVlanedNetOverNicAndCheck(1, bridged, nic=nic,
+ mtu='1400')
+ self.assertEquals('1500', self.vdsm_net.getMtu(nic))
+
+ # Add a network with MTU bigger than existing network
+ self._createVlanedNetOverNicAndCheck(2, bridged, nic=nic,
+ mtu='1600')
+ self.assertEquals('1600', self.vdsm_net.getMtu(nic))
+
+ # cleanup
+ networks = dict((NETWORK_NAME + str(num), {'remove': True}) for
+ num in range(3))
+ status, msg = self.vdsm_net.setupNetworks(networks, {}, {})
+ self.assertEquals(status, SUCCESS, msg)
+
+ @permutations([[True], [False]])
def testSetupNetworksAddBadParams(self, bridged):
attrs = dict(vlan=VLAN_ID, bridged=bridged)
status, msg = self.vdsm_net.setupNetworks({NETWORK_NAME: attrs},
--
To view, visit http://gerrit.ovirt.org/19230
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id7a1f5b628261ae4ebf400494bcdb2939a9cd535
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
10 years, 2 months
Change in vdsm[ovirt-3.3]: vdsm-tool: handle error during module loading
by ybronhei＠redhat.com
Hello Dan Kenigsberg, Zhou Zheng Sheng,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/25238
to review the following change.
Change subject: vdsm-tool: handle error during module loading
......................................................................
vdsm-tool: handle error during module loading
While vdsm-tool initiates it loads all python modules under tool folder.
If one of them throws exception vdsm-tool fails to run. This patch
handles such exception and reports it to syslog.
Change-Id: I023ad390723617d10664f9bf5b8d1460131efef2
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/23456
Reviewed-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm-tool/vdsm-tool
1 file changed, 11 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/25238/1
diff --git a/vdsm-tool/vdsm-tool b/vdsm-tool/vdsm-tool
index 9f75a06..8336de4 100755
--- a/vdsm-tool/vdsm-tool
+++ b/vdsm-tool/vdsm-tool
@@ -23,6 +23,8 @@
import imp
import getopt
import textwrap
+import syslog
+import traceback
import vdsm.tool
@@ -65,13 +67,21 @@
global tool_modules, tool_command
mod_path = os.path.dirname(vdsm.tool.__file__)
+ package_name = vdsm.tool.__name__
for mod_name in _listPathModules(mod_path):
if mod_name.startswith("_"):
continue
mod_fobj, mod_absp, mod_desc = imp.find_module(mod_name, [mod_path])
- module = imp.load_module(mod_name, mod_fobj, mod_absp, mod_desc)
+ try:
+ module = imp.load_module(package_name + '.' + mod_name, mod_fobj,
+ mod_absp, mod_desc)
+ except Exception:
+ # py module was failed to load. report to syslog and continue
+ syslog.syslog("module %s could not load to vdsm-tool: %s" %
+ (mod_name, traceback.format_exc()))
+ continue
mod_cmds = []
--
To view, visit http://gerrit.ovirt.org/25238
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I023ad390723617d10664f9bf5b8d1460131efef2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
10 years, 2 months
Change in vdsm[ovirt-3.4]: utils: Ensure that XMLRPC threads do not delay shutdown
by Nir Soffer
Hello Adam Litke, Yaniv Bronhaim, Antoni Segura Puimedon, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/25122
to review the following change.
Change subject: utils: Ensure that XMLRPC threads do not delay shutdown
......................................................................
utils: Ensure that XMLRPC threads do not delay shutdown
The threads created by the XMLRPC server are not daemon threads, so they
may delay vdsm shutdown if a long request happen to run during shutdown.
This patch configure XMLRPC server to use daemon threads.
The daemon_threads class attribute is used by
SocketServer.ThreadingMixIn. When defined in base class, both the simple
and secure servers get this configuration through inheritance.
This change is required for adding HTTP/1.1 support, where XMLRPC server
threads become long living threads.
Change-Id: I67d3a1b062e7de61e7878b47291cca946f0f13af
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24555
Reviewed-by: Antoni Segura Puimedon <asegurap(a)redhat.com>
Reviewed-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
Reviewed-by: Adam Litke <alitke(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/utils.py
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/25122/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index 917dc36..92d6f25 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -150,6 +150,10 @@
class IPXMLRPCServer(SimpleXMLRPCServer):
+
+ # Create daemon threads when mixed with SocketServer.ThreadingMixIn
+ daemon_threads = True
+
def __init__(self, addr, requestHandler=IPXMLRPCRequestHandler,
logRequests=True, allow_none=False, encoding=None,
bind_and_activate=True):
--
To view, visit http://gerrit.ovirt.org/25122
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I67d3a1b062e7de61e7878b47291cca946f0f13af
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
10 years, 2 months
Change in vdsm[ovirt-3.4]: xmlrpc: Support HTTP 1.1
by Nir Soffer
Hello Adam Litke, Antoni Segura Puimedon, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/25124
to review the following change.
Change subject: xmlrpc: Support HTTP 1.1
......................................................................
xmlrpc: Support HTTP 1.1
Supporting HTTP 1.1 allow using keep-alive on the client side,
decreasing the latency and load on the engine and increasing the number
of host that engine can support.
This patch override do_POST and report_404 methods that wrongly shutdown
the connection at the end of the do_POST request. This is the
responsibility of the server, and not the request handler. The fixed
methods are used only on Python 2.6. Python 2.7 already include these
changes. Then the protocol_version is set to HTTP/1.1, enabling
automatic keep-alive.
A new configuration option "xmlrpc_http11" can be used to disbale this
feature and use HTTP/1.0 as used before.
We can see in the logs that only few threads are created now for service
XMLRPC requests, and most requests are handled by the same thread.
Change-Id: Ie033d5c53c81c8db99d5c26697a1727be030e0b4
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24294
Reviewed-by: Antoni Segura Puimedon <asegurap(a)redhat.com>
Reviewed-by: Adam Litke <alitke(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/config.py.in
M lib/vdsm/utils.py
2 files changed, 86 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/25124/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 0f84f16..a3be658 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -179,6 +179,9 @@
('xmlrpc_enable', 'true', 'Enable the xmlrpc server'),
+ ('xmlrpc_http11', 'true',
+ 'Enable HTTP/1.1 keep-alive connections'),
+
('jsonrpc_enable', 'true', 'Enable the JSON RPC server'),
('report_host_threads_as_cores', 'false',
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index 92d6f25..3c6bdcb 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -55,6 +55,7 @@
import zombiereaper
from cpopen import CPopen
+from .config import config
from . import constants
# Buffsize is 1K because I tested it on some use cases and 1K was fastest. If
@@ -146,7 +147,88 @@
else:
raise
-IPXMLRPCRequestHandler = SimpleXMLRPCRequestHandler
+
+class IPXMLRPCRequestHandler(SimpleXMLRPCRequestHandler):
+
+ if config.getboolean('vars', 'xmlrpc_http11'):
+ protocol_version = "HTTP/1.1"
+ else:
+ protocol_version = "HTTP/1.0"
+
+ # Override Python 2.6 version to support HTTP 1.1.
+ #
+ # This is the same code as Python 2.6, not shutting down the connection
+ # when a request is finished. The server is responsible for closing the
+ # connection, based on the http version and keep-alive and connection:
+ # close headers.
+ #
+ # Additionally, add "Content-Length: 0" header on internal errors, when we
+ # don't send any content. This is required by HTTP 1.1, otherwise the
+ # client does not have any clue that the response was finished.
+ #
+ # These changes were taken from Python 2.7 version of this class. If we are
+ # running on Python 2.7, these changes are not needed, hence we override
+ # the methods only on Python 2.6.
+
+ if sys.version_info[:2] == (2, 6):
+
+ def do_POST(self):
+ # Check that the path is legal
+ if not self.is_rpc_path_valid():
+ self.report_404()
+ return
+
+ try:
+ # Get arguments by reading body of request.
+ # We read this in chunks to avoid straining
+ # socket.read(); around the 10 or 15Mb mark, some platforms
+ # begin to have problems (bug #792570).
+ max_chunk_size = 10*1024*1024
+ size_remaining = int(self.headers["content-length"])
+ L = []
+ while size_remaining:
+ chunk_size = min(size_remaining, max_chunk_size)
+ chunk = self.rfile.read(chunk_size)
+ if not chunk:
+ break
+ L.append(chunk)
+ size_remaining -= len(L[-1])
+ data = ''.join(L)
+
+ # In previous versions of SimpleXMLRPCServer, _dispatch
+ # could be overridden in this class, instead of in
+ # SimpleXMLRPCDispatcher. To maintain backwards compatibility,
+ # check to see if a subclass implements _dispatch and dispatch
+ # using that method if present.
+ response = self.server._marshaled_dispatch(
+ data, getattr(self, '_dispatch', None))
+ except Exception, e:
+ # This should only happen if the module is buggy
+ # internal error, report as HTTP server error
+ self.send_response(500)
+
+ # Send information about the exception if requested
+ if getattr(self.server, '_send_traceback_header', False):
+ self.send_header("X-exception", str(e))
+ self.send_header("X-traceback", traceback.format_exc())
+
+ self.send_header("Content-length", '0')
+ self.end_headers()
+ else:
+ # got a valid XML RPC response
+ self.send_response(200)
+ self.send_header("Content-type", "text/xml")
+ self.send_header("Content-length", str(len(response)))
+ self.end_headers()
+ self.wfile.write(response)
+
+ def report_404(self):
+ self.send_response(404)
+ response = 'No such page'
+ self.send_header("Content-type", "text/plain")
+ self.send_header("Content-length", str(len(response)))
+ self.end_headers()
+ self.wfile.write(response)
class IPXMLRPCServer(SimpleXMLRPCServer):
--
To view, visit http://gerrit.ovirt.org/25124
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie033d5c53c81c8db99d5c26697a1727be030e0b4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
10 years, 2 months
Change in vdsm[ovirt-3.4]: threadLocal: Cleanup thread local storage
by Nir Soffer
Hello Adam Litke, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/25123
to review the following change.
Change subject: threadLocal: Cleanup thread local storage
......................................................................
threadLocal: Cleanup thread local storage
We keep some data in thread local storage, but never clean up. This can
lead to using stale data when handling many requests in the same tread,
or to temporary leak, when a long running thread holds a reference to a
finished task, preventing garbage collection of anything referenced from
the task.
This patch ensures that anything we set thread local storage is cleaned
up when not needed.
Change-Id: I97b110db1c5f94e8561b4ba15a3c27b43043ed6f
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24309
Reviewed-by: Adam Litke <alitke(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm/BindingXMLRPC.py
M vdsm/storage/dispatcher.py
M vdsm/storage/storage_mailbox.py
M vdsm/storage/task.py
4 files changed, 25 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/25123/1
diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py
index 9b56ea1..bafc7c7 100644
--- a/vdsm/BindingXMLRPC.py
+++ b/vdsm/BindingXMLRPC.py
@@ -212,6 +212,12 @@
threadLocal.flowID = self.headers.get(HTTP_HEADER_FLOWID)
return r
+ def finish(self):
+ basehandler.finish(self)
+ threadLocal.client = None
+ threadLocal.server = None
+ threadLocal.flowID = None
+
if self.enableSSL:
KEYFILE, CERTFILE, CACERT = self._getKeyCertFilenames()
server = SecureXMLRPCServer.SecureThreadedXMLRPCServer(
diff --git a/vdsm/storage/dispatcher.py b/vdsm/storage/dispatcher.py
index cbeb23c..da838e8 100644
--- a/vdsm/storage/dispatcher.py
+++ b/vdsm/storage/dispatcher.py
@@ -23,7 +23,6 @@
import task
import storage_exception as se
-from threadLocal import vars
_EXPORTED_ATTRIBUTE = "__dispatcher_exported__"
@@ -56,7 +55,6 @@
def run(self, *args, **kwargs):
try:
ctask = task.Task(id=None, name=self.name)
- vars.task = ctask
try:
response = self.STATUS_OK.copy()
result = ctask.prepare(self.func, *args, **kwargs)
diff --git a/vdsm/storage/storage_mailbox.py b/vdsm/storage/storage_mailbox.py
index d56a2d7..4f54066 100644
--- a/vdsm/storage/storage_mailbox.py
+++ b/vdsm/storage/storage_mailbox.py
@@ -33,7 +33,6 @@
import sd
import misc
import task
-from threadLocal import vars
from threadPool import ThreadPool
from storage_exception import InvalidParameterException
from vdsm import constants
@@ -77,7 +76,6 @@
cmd = args
args = None
ctask = task.Task(id=None, name=cmd)
- vars.task = ctask
ctask.prepare(cmd, *args)
diff --git a/vdsm/storage/task.py b/vdsm/storage/task.py
index 4eff5c1..88ab5fa 100644
--- a/vdsm/storage/task.py
+++ b/vdsm/storage/task.py
@@ -49,6 +49,7 @@
import logging
import threading
import types
+from functools import wraps
import storage_exception as se
import uuid
@@ -88,6 +89,21 @@
def _eq_decode(s):
return s.replace(KEY_SEPARATOR_ENCODED, KEY_SEPARATOR)
+
+
+def threadlocal_task(m):
+ """
+ Decorator that set the task object in thread local storage task attribute
+ while the decorated method is running.
+ """
+ @wraps(m)
+ def wrapper(self, *a, **kw):
+ vars.task = self
+ try:
+ return m(self, *a, **kw)
+ finally:
+ vars.task = None
+ return wrapper
class State:
@@ -1133,6 +1149,7 @@
t._load(store, ext)
return t
+ @threadlocal_task
def prepare(self, func, *args, **kwargs):
message = self.error
try:
@@ -1172,9 +1189,9 @@
finally:
self._decref()
+ @threadlocal_task
def commit(self, args=None):
self.log.debug("committing task: %s", self.id)
- vars.task = self
try:
self._incref()
except se.TaskAborted:
@@ -1210,13 +1227,13 @@
finally:
self._decref(force)
+ @threadlocal_task
def recover(self, args=None):
''' Do not call this function while the task is actually running. this
method should only be used to recover tasks state after
(vdsmd) restart.
'''
self.log.debug('(recover): recovering: state %s', self.state)
- vars.task = self
try:
self._incref(force=True)
except se.TaskAborted:
--
To view, visit http://gerrit.ovirt.org/25123
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I97b110db1c5f94e8561b4ba15a3c27b43043ed6f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
10 years, 2 months
Change in vdsm[ovirt-3.4]: vm: snapshot transient disk check should be per disk
by Federico Simoncelli
Federico Simoncelli has uploaded a new change for review.
Change subject: vm: snapshot transient disk check should be per disk
......................................................................
vm: snapshot transient disk check should be per disk
Live snapshot is allowed if all the disks in the snapshot request are
not transient.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1065886
Change-Id: I3775e2218186e56e99740c6f91c8c98484529892
Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/24867
Reviewed-by: Daniel Erez <derez(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
(cherry picked from commit ed8b6b28122ec4c8917f85ab19695691d2866e0b)
---
M vdsm/vm.py
1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/25007/1
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 8b74187..5d38a55 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -3889,9 +3889,6 @@
if self.isMigrating():
return errCode['migInProgress']
- if self.hasTransientDisks():
- return errCode['transientErr']
-
for drive in snapDrives:
baseDrv, tgetDrv = _normSnapDriveParams(drive)
@@ -3915,8 +3912,13 @@
return errCode['snapshotErr']
if vmDrive.hasVolumeLeases:
+ self.log.error('disk %s has volume leases', vmDrive.name)
return errCode['noimpl']
+ if vmDrive.transientDisk:
+ self.log.error('disk %s is a transient disk', vmDrive.name)
+ return errCode['transientErr']
+
vmDevName = vmDrive.name
newDrives[vmDevName] = tgetDrv.copy()
--
To view, visit http://gerrit.ovirt.org/25007
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3775e2218186e56e99740c6f91c8c98484529892
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
10 years, 2 months
