Change in vdsm[master]: ioprocess: Have tests check ioprocess and not RHF
by smizrahi@redhat.com
Saggi Mizrahi has uploaded a new change for review.
Change subject: ioprocess: Have tests check ioprocess and not RHF
......................................................................
ioprocess: Have tests check ioprocess and not RHF
Change-Id: I0181512d0f09327d985e26de890e9a6e6863e7cf
Signed-off-by: Saggi Mizrahi <smizrahi(a)redhat.com>
---
M tests/outOfProcessTests.py
1 file changed, 2 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/83/34083/1
diff --git a/tests/outOfProcessTests.py b/tests/outOfProcessTests.py
index e314197..e49930f 100644
--- a/tests/outOfProcessTests.py
+++ b/tests/outOfProcessTests.py
@@ -26,6 +26,7 @@
class OopWrapperTests(TestCaseBase):
def setUp(self):
+ oop.setDefaultImpl(oop.IOPROC)
self.pool = oop.getGlobalProcPool()
def testEcho(self):
@@ -34,7 +35,7 @@
real discretion."""
# Henry Steele Commager
- self.assertEquals(self.pool.echo(data), data)
+ self.assertEquals(self.pool._ioproc.echo(data), data)
def testFileUtilsCall(self):
"""fileUtils is a custom module and calling it might break even though
@@ -45,9 +46,6 @@
def testSubModuleCall(self):
path = "/dev/null"
self.assertEquals(self.pool.os.path.exists(path), True)
-
- def testModuleCall(self):
- self.assertNotEquals(self.pool.os.getpid(), os.getpid())
def testUtilsFuncs(self):
tmpfd, tmpfile = tempfile.mkstemp()
--
To view, visit http://gerrit.ovirt.org/34083
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0181512d0f09327d985e26de890e9a6e6863e7cf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
9 years, 4 months
Change in vdsm[master]: oop: Use a single instance of IOProcess per SD
by ykaplan@redhat.com
Yeela Kaplan has uploaded a new change for review.
Change subject: oop: Use a single instance of IOProcess per SD
......................................................................
oop: Use a single instance of IOProcess per SD
Next step after switching from RFH to IOPROC is to
use a single IOProcess instance for each SD.
Currently only a single IOProcess instance is used
for all SDs.
To avoid load on the system by holding many unused
IOProcesses we will maintain dictionary:
KEY - domainID, VALUE - (timestamp, IOPROC)
When IOProc for domainID is used we will renew the
timestamp, or delete the item from dictionary
We will also maintain another dictionary:
KEY - domainID, VALUE - IOPROC weakref,
that will allow us to check if IOPROC obj is
still alive or needs to be recreated.
Change-Id: I383fe617ee4ce22de368ba54f980887d70ff37c5
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1126206
Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com>
---
M lib/vdsm/config.py.in
M vdsm/storage/outOfProcess.py
2 files changed, 36 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/31501/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 69185e0..518d3ff 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -319,6 +319,8 @@
('process_pool_timeout', '60', None),
+ ('ioprocess_timeout', '60', 'TTL of IOProcess that is not renewed'),
+
('process_pool_max_slots_per_domain', '10', None),
('iscsi_default_ifaces', 'default',
diff --git a/vdsm/storage/outOfProcess.py b/vdsm/storage/outOfProcess.py
index 6230fad..e5c82af 100644
--- a/vdsm/storage/outOfProcess.py
+++ b/vdsm/storage/outOfProcess.py
@@ -24,6 +24,8 @@
import stat
import sys
import types
+import time
+import weakref
from vdsm import constants
from vdsm.config import config
@@ -45,10 +47,13 @@
_oopImpl = RFH
DEFAULT_TIMEOUT = config.getint("irs", "process_pool_timeout")
+DEF_IOPROC_TIMEOUT = config.getint("irs", "ioprocess_timeout")
HELPERS_PER_DOMAIN = config.getint("irs", "process_pool_max_slots_per_domain")
-_procLock = threading.Lock()
-_proc = {}
+_procPoolLock = threading.Lock()
+_procPool = {}
+_refProcPool = {}
+_rfhPool = {}
log = logging.getLogger('oop')
@@ -63,18 +68,38 @@
def getProcessPool(clientName):
try:
- return _proc[clientName]
+ if _oopImpl == IOPROC:
+ with _procPoolLock:
+ keys = sorted(_procPool, key=_procPool.get)
+ for k in keys:
+ if (time.time() - _procPool[k][0]) > DEF_IOPROC_TIMEOUT:
+ del _procPool[k]
+ else:
+ break
+
+ procref = _refProcPool[clientName]
+ proc = procref()
+ if proc is None:
+ raise KeyError
+ else:
+ _procPool[clientName] = (time.time(), proc)
+ return proc
+ else:
+ return _rfhPool[clientName]
except KeyError:
- with _procLock:
+ with _procPoolLock:
if _oopImpl == IOPROC:
- if GLOBAL not in _proc:
- _proc[GLOBAL] = _OopWrapper(IOProcess(DEFAULT_TIMEOUT))
- _proc[clientName] = _proc[GLOBAL]
+ proc = _OopWrapper(IOProcess(DEFAULT_TIMEOUT))
+ _procPool[clientName] = (time.time(), proc)
+ _refProcPool[clientName] = weakref.ref(proc)
+ procref = _refProcPool[clientName]
+
+ return procref()
else:
- _proc[clientName] = _OopWrapper(
+ _rfhPool[clientName] = _OopWrapper(
RemoteFileHandlerPool(HELPERS_PER_DOMAIN))
- return _proc[clientName]
+ return _rfhPool[clientName]
def getGlobalProcPool():
--
To view, visit http://gerrit.ovirt.org/31501
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I383fe617ee4ce22de368ba54f980887d70ff37c5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
9 years, 4 months
Change in vdsm[master]: utils: Move functionality from storage.misc
by ykaplan@redhat.com
Yeela Kaplan has uploaded a new change for review.
Change subject: utils: Move functionality from storage.misc
......................................................................
utils: Move functionality from storage.misc
move rotateFiles and persistFile into utils
Change-Id: I2c547e3ac2ecf19359c32f7e717ad7b098bfbca4
Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com>
---
M lib/vdsm/utils.py
M tests/miscTests.py
M tests/utilsTests.py
M vdsm/storage/misc.py
M vdsm/storage/multipath.py
M vdsm/storage/sd.py
6 files changed, 115 insertions(+), 114 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/83/31583/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index 84b7883..e88007c 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -143,6 +143,67 @@
raise
+def rotateFiles(directory, prefixName, gen, cp=False, persist=False):
+ logging.debug("dir: %s, prefixName: %s, versions: %s" %
+ (directory, prefixName, gen))
+ gen = int(gen)
+ files = os.listdir(directory)
+ files = glob.glob("%s*" % prefixName)
+ fd = {}
+ for fname in files:
+ name = fname.rsplit('.', 1)
+ try:
+ ind = int(name[1])
+ except ValueError:
+ name[0] = fname
+ ind = 0
+ except IndexError:
+ ind = 0
+ except:
+ continue
+ if ind < gen:
+ fd[ind] = {'old': fname, 'new': name[0] + '.' + str(ind + 1)}
+
+ keys = fd.keys()
+ keys.sort(reverse=True)
+ logging.debug("versions found: %s" % (keys))
+
+ for key in keys:
+ oldName = os.path.join(directory, fd[key]['old'])
+ newName = os.path.join(directory, fd[key]['new'])
+ if isOvirtNode() and persist and not cp:
+ try:
+ execCmd([constants.EXT_UNPERSIST, oldName],
+ sudo=True)
+ execCmd([constants.EXT_UNPERSIST, newName],
+ sudo=True)
+ except:
+ pass
+ try:
+ if cp:
+ execCmd([constants.EXT_CP, oldName, newName], sudo=True)
+ if (isOvirtNode() and
+ persist and not os.path.exists(newName)):
+ execCmd([constants.EXT_PERSIST, newName],
+ sudo=True)
+
+ else:
+ os.rename(oldName, newName)
+ except:
+ pass
+ if isOvirtNode() and persist and not cp:
+ try:
+ execCmd([constants.EXT_PERSIST, newName],
+ sudo=True)
+ except:
+ pass
+
+
+def persistFile(name):
+ if isOvirtNode():
+ execCmd([constants.EXT_PERSIST, name], sudo=True)
+
+
def _parseMemInfo(lines):
"""
Parse the content of ``/proc/meminfo`` as list of strings
diff --git a/tests/miscTests.py b/tests/miscTests.py
index a40afb0..e25135a 100644
--- a/tests/miscTests.py
+++ b/tests/miscTests.py
@@ -203,54 +203,6 @@
self.assertRaises(ValueError, misc.itmap(int, data, 0).next)
-class RotateFiles(TestCaseBase):
- def testNonExistingDir(self, persist=False):
- """
- Tests that the method fails correctly when given a non existing dir.
- """
- self.assertRaises(OSError, misc.rotateFiles, "/I/DONT/EXIST", "prefix",
- 2, persist=persist)
-
- def testEmptyDir(self, persist=False):
- """
- Test that when given an empty dir the rotator works correctly.
- """
- prefix = "prefix"
- with namedTemporaryDir() as dir:
- misc.rotateFiles(dir, prefix, 0, persist=persist)
-
- def testFullDir(self, persist=False):
- """
- Test that rotator does it's basic functionality.
- """
- # Prepare
- prefix = "prefix"
- stubContent = ('"Multiple exclamation marks", '
- 'he went on, shaking his head, '
- '"are a sure sign of a diseased mind."')
- # (C) Terry Pratchet - Small Gods
- with namedTemporaryDir() as dir:
- gen = 10
-
- expectedDirContent = []
- for i in range(gen):
- fname = "%s.txt.%d" % (prefix, i + 1)
- expectedDirContent.append("%s.txt.%d" % (prefix, i + 1))
- f = open(os.path.join(dir, fname), "wb")
- f.write(stubContent)
- f.flush()
- f.close()
-
- # Rotate
- misc.rotateFiles(dir, prefix, gen, persist=persist)
-
- # Test result
- currentDirContent = os.listdir(dir)
- expectedDirContent.sort()
- currentDirContent.sort()
- self.assertEquals(currentDirContent, expectedDirContent)
-
-
class ParseHumanReadableSize(TestCaseBase):
def testValidInput(self):
"""
diff --git a/tests/utilsTests.py b/tests/utilsTests.py
index 6e66c02..3875d59 100644
--- a/tests/utilsTests.py
+++ b/tests/utilsTests.py
@@ -18,13 +18,14 @@
# Refer to the README and COPYING files for full details of the license
#
-import os.path
+import os
import contextlib
import errno
import logging
import sys
import threading
+from testlib import namedTemporaryDir
from testlib import VdsmTestCase as TestCaseBase
from testlib import permutations, expandPermutations
from testValidation import checkSudo
@@ -450,6 +451,54 @@
self.fail("Exception was not raised")
+class RotateFiles(TestCaseBase):
+ def testNonExistingDir(self, persist=False):
+ """
+ Tests that the method fails correctly when given a non existing dir.
+ """
+ self.assertRaises(OSError, utils.rotateFiles, "/I/DONT/EXIST",
+ "prefix", 2, persist=persist)
+
+ def testEmptyDir(self, persist=False):
+ """
+ Test that when given an empty dir the rotator works correctly.
+ """
+ prefix = "prefix"
+ with namedTemporaryDir() as dir:
+ utils.rotateFiles(dir, prefix, 0, persist=persist)
+
+ def testFullDir(self, persist=False):
+ """
+ Test that rotator does it's basic functionality.
+ """
+ # Prepare
+ prefix = "prefix"
+ stubContent = ('"Multiple exclamation marks", '
+ 'he went on, shaking his head, '
+ '"are a sure sign of a diseased mind."')
+ # (C) Terry Pratchet - Small Gods
+ with namedTemporaryDir() as dir:
+ gen = 10
+
+ expectedDirContent = []
+ for i in range(gen):
+ fname = "%s.txt.%d" % (prefix, i + 1)
+ expectedDirContent.append("%s.txt.%d" % (prefix, i + 1))
+ f = open(os.path.join(dir, fname), "wb")
+ f.write(stubContent)
+ f.flush()
+ f.close()
+
+ # Rotate
+ utils.rotateFiles(dir, prefix, gen, persist=persist)
+
+ # Test result
+ currentDirContent = os.listdir(dir)
+ expectedDirContent.sort()
+ currentDirContent.sort()
+ self.assertEquals(currentDirContent, expectedDirContent)
+
+
@expandPermutations
class ExecCmdTest(TestCaseBase):
CMD_TYPES = ((tuple,), (list,), (iter,))
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index d0869ee..fa4fe1e 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -33,7 +33,6 @@
from itertools import chain, imap
import contextlib
import errno
-import glob
import logging
import os
import Queue
@@ -449,67 +448,6 @@
if n < 0:
raise se.InvalidParameterException(name, number)
return n
-
-
-def rotateFiles(directory, prefixName, gen, cp=False, persist=False):
- log.debug("dir: %s, prefixName: %s, versions: %s" %
- (directory, prefixName, gen))
- gen = int(gen)
- files = os.listdir(directory)
- files = glob.glob("%s*" % prefixName)
- fd = {}
- for fname in files:
- name = fname.rsplit('.', 1)
- try:
- ind = int(name[1])
- except ValueError:
- name[0] = fname
- ind = 0
- except IndexError:
- ind = 0
- except:
- continue
- if ind < gen:
- fd[ind] = {'old': fname, 'new': name[0] + '.' + str(ind + 1)}
-
- keys = fd.keys()
- keys.sort(reverse=True)
- log.debug("versions found: %s" % (keys))
-
- for key in keys:
- oldName = os.path.join(directory, fd[key]['old'])
- newName = os.path.join(directory, fd[key]['new'])
- if utils.isOvirtNode() and persist and not cp:
- try:
- execCmd([constants.EXT_UNPERSIST, oldName],
- sudo=True)
- execCmd([constants.EXT_UNPERSIST, newName],
- sudo=True)
- except:
- pass
- try:
- if cp:
- execCmd([constants.EXT_CP, oldName, newName], sudo=True)
- if (utils.isOvirtNode() and
- persist and not os.path.exists(newName)):
- execCmd([constants.EXT_PERSIST, newName],
- sudo=True)
-
- else:
- os.rename(oldName, newName)
- except:
- pass
- if utils.isOvirtNode() and persist and not cp:
- try:
- execCmd([constants.EXT_PERSIST, newName],
- sudo=True)
- except:
- pass
-
-
-def persistFile(name):
- if utils.isOvirtNode():
- execCmd([constants.EXT_PERSIST, name], sudo=True)
def parseHumanReadableSize(size):
diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py
index ba98866..d626305 100644
--- a/vdsm/storage/multipath.py
+++ b/vdsm/storage/multipath.py
@@ -158,7 +158,7 @@
supported state. The original configuration, if any, is saved
"""
if os.path.exists(MPATH_CONF):
- misc.rotateFiles(
+ utils.rotateFiles(
os.path.dirname(MPATH_CONF),
os.path.basename(MPATH_CONF), MAX_CONF_COPIES,
cp=True, persist=True)
@@ -169,7 +169,7 @@
rc = misc.execCmd(cmd, sudo=True)[0]
if rc != 0:
raise se.MultipathSetupError()
- misc.persistFile(MPATH_CONF)
+ utils.persistFile(MPATH_CONF)
# Flush all unused multipath device maps
misc.execCmd([constants.EXT_MULTIPATH, "-F"], sudo=True)
diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py
index b34cc79..198586d 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -32,6 +32,7 @@
from resourceFactories import IMAGE_NAMESPACE, VOLUME_NAMESPACE
import resourceManager as rm
from vdsm import constants
+from vdsm import utils
import clusterlock
import outOfProcess as oop
from persistentDict import unicodeEncoder, unicodeDecoder
@@ -733,7 +734,7 @@
def setMetadata(self, newMetadata):
# Backup old md (rotate old backup files)
- misc.rotateFiles(self.mdBackupDir, self.sdUUID, self.mdBackupVersions)
+ utils.rotateFiles(self.mdBackupDir, self.sdUUID, self.mdBackupVersions)
oldMd = ["%s=%s\n" % (key, value)
for key, value in self.getMetadata().copy().iteritems()]
open(os.path.join(self.mdBackupDir, self.sdUUID),
--
To view, visit http://gerrit.ovirt.org/31583
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2c547e3ac2ecf19359c32f7e717ad7b098bfbca4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
9 years, 4 months
Change in vdsm[master]: netinfo:nicSpeed(): fix nicSpeed condition
by phoracek@redhat.com
Petr Horáček has uploaded a new change for review.
Change subject: netinfo:nicSpeed(): fix nicSpeed condition
......................................................................
netinfo:nicSpeed(): fix nicSpeed condition
In `if s not in (2 ** 16 - 1, 2 ** 32 - 1) or s > 0` first
part doesnt make sense with OR. Changed to AND.
I changed conditions to make the function more readable.
Change-Id: I34cfca16909f9695441e26d3ddd508e7a4210c12
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1157224
Signed-off-by: Petr Horáček <phoracek(a)redhat.com>
---
M lib/vdsm/netinfo.py
1 file changed, 9 insertions(+), 12 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/34694/1
diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index d2ef90b..780f4e5 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -247,19 +247,16 @@
def nicSpeed(nicName):
- """Returns the nic speed if it is a legal value and nicName refers to a
- nic, 0 otherwise."""
+ """Returns the nic speed if it is a legal value, nicName refers to a
+ nic and nic is UP, 0 otherwise."""
try:
- # if the device is not up we must report 0
- if operstate(nicName) != OPERSTATE_UP:
- return 0
- with open('/sys/class/net/%s/speed' % nicName) as speedFile:
- s = int(speedFile.read())
- # the device may have been disabled/downed after checking
- # so we validate the return value as sysfs may return
- # special values to indicate the device is down/disabled
- if s not in (2 ** 16 - 1, 2 ** 32 - 1) or s > 0:
- return s
+ if operstate(nicName) == OPERSTATE_UP:
+ with open('/sys/class/net/%s/speed' % nicName) as speedFile:
+ s = int(speedFile.read())
+ # the device may have been disabled/downed after checking
+ # so we validate the return value as sysfs may return
+ # special values to indicate the device is down/disabled
+ return s if s not in (2 ** 16 - 1, 2 ** 32 - 1) and s > 0 else 0
except IOError as ose:
if ose.errno == errno.EINVAL:
return _ibHackedSpeed(nicName)
--
To view, visit http://gerrit.ovirt.org/34694
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I34cfca16909f9695441e26d3ddd508e7a4210c12
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
9 years, 4 months
Change in vdsm[master]: WIP: vm: rationalize device setup
by fromani@redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: WIP: vm: rationalize device setup
......................................................................
WIP: vm: rationalize device setup
WIP - still evaluating
Change-Id: I233569ba0104171807f5bcf0b6fedbd2d9b748f9
Bug-Url: https://bugzilla.redhat.com/1143968
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M tests/vmTests.py
M tests/vmTestsData.py
M vdsm/virt/vm.py
3 files changed, 28 insertions(+), 33 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/34667/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 6daeeec..95d7b17 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -1516,8 +1516,6 @@
{'type': 'graphics', 'device': 'vnc', 'port': '-1'}]
with fake.VM(self.VM_PARAMS, devices) as testvm:
- testvm._updateDevices(
- testvm.setupDevicesConf(testvm.conf['devices']))
res = testvm.getStats()
self.assertIn('displayPort', res)
self.assertEqual(res['displayType'],
diff --git a/tests/vmTestsData.py b/tests/vmTestsData.py
index 3316598..b68e81c 100644
--- a/tests/vmTestsData.py
+++ b/tests/vmTestsData.py
@@ -29,7 +29,7 @@
'smp': '1', 'cpuPinning': {}, 'numaTune': {}, 'maxVCpus': '160',
'tabletEnable': False,
'displayNetwork': 'mydisp', 'custom': {},
- 'guestNumaNodes': []},
+ 'guestNumaNodes': [], 'devices': []},
"""<?xml version="1.0" encoding="utf-8"?>
<domain type="kvm">
@@ -93,7 +93,7 @@
'smp': '1', 'cpuPinning': {}, 'numaTune': {}, 'maxVCpus': '160',
'tabletEnable': False,
'displayNetwork': 'mydisp', 'custom': {},
- 'guestNumaNodes': []},
+ 'guestNumaNodes': [], 'devices': []},
"""<?xml version="1.0" encoding="utf-8"?>
<domain type="kvm"
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 6329166..648abf0 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -20,10 +20,11 @@
# stdlib imports
-from collections import namedtuple
+from collections import namedtuple, defaultdict
from contextlib import contextmanager
from copy import deepcopy
from xml.dom.minidom import parseString as _domParseStr
+import itertools
import logging
import os
import tempfile
@@ -1211,9 +1212,6 @@
(SMARTCARD_DEVICES, vmdevices.Smartcard),
(TPM_DEVICES, vmdevices.Tpm))
- def _makeDeviceDict(self):
- return dict((dev, []) for dev, _ in self.DeviceMapping)
-
def _makeChannelPath(self, deviceName):
return constants.P_LIBVIRT_VMCHANNELS + self.id + '.' + deviceName
@@ -1279,7 +1277,6 @@
self.stopDisksStatsCollection()
self._vmCreationEvent = threading.Event()
self._pathsPreparedEvent = threading.Event()
- self._devices = self._makeDeviceDict()
self._connection = libvirtconnection.get(cif)
if 'vmName' not in self.conf:
@@ -1301,6 +1298,8 @@
self._powerDownEvent = threading.Event()
self._liveMergeCleanupThreads = {}
self._shutdownReason = None
+
+ self.setupDevices()
def _get_lastStatus(self):
PAUSED_STATES = (vmstatus.POWERING_DOWN, vmstatus.REBOOT_IN_PROGRESS,
@@ -1390,7 +1389,7 @@
Return the "devices" section of this Vm's conf.
If missing, create it according to old API.
"""
- devices = self._makeDeviceDict()
+ devices = defaultdict(list)
# For BC we need to save previous behaviour for old type parameters.
# The new/old type parameter will be distinguished
@@ -1511,7 +1510,7 @@
'type': GRAPHICS_DEVICES,
'device': (
'spice'
- if self.conf['display'] in ('qxl', 'qxlnc')
+ if self.conf.get('display') in ('qxl', 'qxlnc')
else 'vnc'),
'specParams': makeSpecParams(self.conf)}]
@@ -2638,10 +2637,18 @@
except Exception:
self.log.warning('failed to set Vm niceness', exc_info=True)
+ def setupDevices(self):
+ devtree = self.setupDevicesConf(self.conf.get('devices'))
+ devices = defaultdict(list)
+ for devType, devClass in self.DeviceMapping:
+ for dev in devtree[devType]:
+ devices[devType].append(devClass(self.conf, self.log, **dev))
+ self._devices = devices
+ self.conf['devices'] = _flatten(devtree)
+
def _run(self):
self.log.info("VM wrapper has started")
- devices = self.setupDevicesConf(self.conf.get('devices'))
- self.normalizeStorageDevicesConf(devices[DISK_DEVICES])
+ self.normalizeStorageDevicesConf(self.getDiskDevices())
# recovery flow note:
# we do not start disk stats collection here since
@@ -2649,9 +2656,8 @@
# Disk stats collection is started from clientIF at the end
# of the recovery process.
if not self.recovering:
- self.preparePaths(devices[DISK_DEVICES])
- self._prepareTransientDisks(devices[DISK_DEVICES])
- self._updateDevices(devices)
+ self.preparePaths(self.getDiskDevices())
+ self._prepareTransientDisks(self.getDiskDevices())
# We need to save conf here before we actually run VM.
# It's not enough to save conf only on status changes as we did
# before, because if vdsm will restarted between VM run and conf
@@ -2659,11 +2665,6 @@
# So, to get proper device objects during VM recovery flow
# we must to have updated conf before VM run
self.saveState()
-
- for devType, devClass in self.DeviceMapping:
- for dev in devices[devType]:
- self._devices[devType].append(devClass(self.conf, self.log,
- **dev))
# We should set this event as a last part of drives initialization
self._pathsPreparedEvent.set()
@@ -2718,18 +2719,6 @@
if initDomain:
self._domDependentInit()
-
- def _updateDevices(self, devices):
- """
- Update self.conf with updated devices
- For old type vmParams, new 'devices' key will be
- created with all devices info
- """
- newDevices = []
- for dev in devices.values():
- newDevices.extend(dev)
-
- self.conf['devices'] = newDevices
def _correctDiskVolumes(self, srcDomXML):
"""
@@ -5447,3 +5436,11 @@
ip = '0'
logging.info('network %s: using %s', network, ip)
return ip
+
+
+def _flatten(seq):
+ """
+ flattens an iterable of lists into a list.
+ itertools recipe.
+ """
+ return list(itertools.chain.from_iterable(seq))
--
To view, visit http://gerrit.ovirt.org/34667
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I233569ba0104171807f5bcf0b6fedbd2d9b748f9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
9 years, 4 months
Change in vdsm[master]: vm: split storage part from device normalization
by fromani@redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: vm: split storage part from device normalization
......................................................................
vm: split storage part from device normalization
We learned the hard way multiple times in the past
that storage access is tricky, potentially slow
in the good case, blocking for long time or even
forever in the worst case.
For the sake of being careful, split out
the storage device normalization from the
regular device normalization.
The rest of device configuration setup
is fast and cheap, because it just rearrange
internal VDSM data structures, with no
need of external access whatsoever.
Change-Id: I32a6545d637b8d3866e353f7890d0d76691d66a0
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/clientIF.py
M vdsm/virt/vm.py
2 files changed, 10 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/34666/1
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 24087c2..6bca4cc 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -457,9 +457,10 @@
try:
# Do not prepare volumes when system goes down
if self._enabled:
- devices = vmObj.setupDevicesConf(
- vmObj.conf.get('devices'))
- vmObj.preparePaths(devices[vm.DISK_DEVICES])
+ drives = vmObj.setupDevicesConf(
+ vmObj.conf.get('devices'))[vm.DISK_DEVICES]
+ vmObj.normalizeStorageDevicesConf(drives)
+ vmObj.preparePaths(drives)
except:
self.log.error("Vm %s recovery failed",
vmId, exc_info=True)
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 9556484..6329166 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1420,8 +1420,11 @@
# to all guests, we need to specifically request not to add it.
self._normalizeBalloonDevice(devices[BALLOON_DEVICES])
+ return devices
+
+ def normalizeStorageDevicesConf(self, drives):
# Normalize vdsm images
- for drv in devices[DISK_DEVICES]:
+ for drv in drives:
if isVdsmImage(drv):
try:
self._normalizeVdsmImg(drv)
@@ -1432,9 +1435,7 @@
if not self.recovering:
raise
- self.normalizeDrivesIndices(devices[DISK_DEVICES])
-
- return devices
+ self.normalizeDrivesIndices(drives)
def _normalizeBalloonDevice(self, balloonDevices):
EMPTY_BALLOON = {'type': BALLOON_DEVICES,
@@ -2640,6 +2641,7 @@
def _run(self):
self.log.info("VM wrapper has started")
devices = self.setupDevicesConf(self.conf.get('devices'))
+ self.normalizeStorageDevicesConf(devices[DISK_DEVICES])
# recovery flow note:
# we do not start disk stats collection here since
--
To view, visit http://gerrit.ovirt.org/34666
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I32a6545d637b8d3866e353f7890d0d76691d66a0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
9 years, 4 months
Change in vdsm[master]: vm: rename buildConfDevices
by fromani@redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: vm: rename buildConfDevices
......................................................................
vm: rename buildConfDevices
Them Vm.buildConfDevices method is not building configuration.
It is rather making sure the device configuration structure
is correct and in the form VDSM expect.
There is indeed fallback code in the case Engine is older,
but still this is more normalization than build.
This patch rearranges the method flows, with no
changes in behaviour, to reflect the real purpose
of the method.
Change-Id: I1df2a31458a25e06f3713bc4dd978bf4d9ea2439
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M tests/vmTests.py
M tests/vmfakelib.py
M vdsm/clientIF.py
M vdsm/virt/vm.py
4 files changed, 18 insertions(+), 12 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/65/34665/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 6eecdcc..6daeeec 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -1516,7 +1516,8 @@
{'type': 'graphics', 'device': 'vnc', 'port': '-1'}]
with fake.VM(self.VM_PARAMS, devices) as testvm:
- testvm._updateDevices(testvm.buildConfDevices())
+ testvm._updateDevices(
+ testvm.setupDevicesConf(testvm.conf['devices']))
res = testvm.getStats()
self.assertIn('displayPort', res)
self.assertEqual(res['displayType'],
@@ -1609,13 +1610,13 @@
for conf in self.confDisplay:
conf.update(self.conf)
with fake.VM(conf) as testvm:
- devs = testvm.buildConfDevices()
+ devs = testvm.setupDevicesConf(testvm.conf['devices'])
self.assertTrue(devs['graphics'])
def testGraphicsDevice(self):
for dev in self.confDeviceGraphics:
with fake.VM(self.conf, dev) as testvm:
- devs = testvm.buildConfDevices()
+ devs = testvm.setupDevicesConf(testvm.conf['devices'])
self.assertTrue(devs['graphics'])
def testGraphicsDeviceMixed(self):
@@ -1627,7 +1628,7 @@
conf.update(self.conf)
for dev in self.confDeviceGraphics:
with fake.VM(self.conf, dev) as testvm:
- devs = testvm.buildConfDevices()
+ devs = testvm.setupDevicesConf(testvm.conf['devices'])
self.assertEqual(len(devs['graphics']), 1)
self.assertEqual(devs['graphics'][0]['device'],
dev[0]['device'])
@@ -1673,7 +1674,7 @@
devices = [{'type': 'graphics', 'device': primary},
{'type': 'graphics', 'device': secondary}]
with fake.VM(self.conf, devices) as testvm:
- devs = testvm.buildConfDevices()
+ devs = testvm.setupDevicesConf(testvm.conf['devices'])
self.assertTrue(len(devs['graphics']) == 2)
@permutations([['vnc'], ['spice']])
@@ -1681,7 +1682,9 @@
devices = [{'type': 'graphics', 'device': devType},
{'type': 'graphics', 'device': devType}]
with fake.VM(self.conf, devices) as testvm:
- self.assertRaises(ValueError, testvm.buildConfDevices)
+ self.assertRaises(ValueError,
+ testvm.setupDevicesConf,
+ testvm.conf['devices'])
@expandPermutations
diff --git a/tests/vmfakelib.py b/tests/vmfakelib.py
index 83c5b7a..1baa681 100644
--- a/tests/vmfakelib.py
+++ b/tests/vmfakelib.py
@@ -169,6 +169,8 @@
(libvirtconnection, 'get', Connection)]):
vmParams = {'vmId': 'TESTING'}
vmParams.update({} if params is None else params)
+ if 'devices' not in vmParams:
+ vmParams['devices'] = tuple()
cif = ClientIF()
fake = vm.Vm(cif, vmParams)
cif.vmContainer[fake.id] = fake
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 440e5e7..24087c2 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -457,8 +457,9 @@
try:
# Do not prepare volumes when system goes down
if self._enabled:
- vmObj.preparePaths(
- vmObj.buildConfDevices()[vm.DISK_DEVICES])
+ devices = vmObj.setupDevicesConf(
+ vmObj.conf.get('devices'))
+ vmObj.preparePaths(devices[vm.DISK_DEVICES])
except:
self.log.error("Vm %s recovery failed",
vmId, exc_info=True)
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 87c6517..9556484 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1385,7 +1385,7 @@
'truesize': 0})
return removables
- def buildConfDevices(self):
+ def setupDevicesConf(self, devconf):
"""
Return the "devices" section of this Vm's conf.
If missing, create it according to old API.
@@ -1395,7 +1395,7 @@
# For BC we need to save previous behaviour for old type parameters.
# The new/old type parameter will be distinguished
# by existence/absence of the 'devices' key
- if self.conf.get('devices') is None:
+ if devconf is None:
devices[DISK_DEVICES] = self.getConfDrives()
devices[NIC_DEVICES] = self.getConfNetworkInterfaces()
devices[SOUND_DEVICES] = self.getConfSound()
@@ -1403,7 +1403,7 @@
devices[GRAPHICS_DEVICES] = self.getConfGraphics()
devices[CONTROLLER_DEVICES] = self.getConfController()
else:
- for dev in self.conf.get('devices'):
+ for dev in devconf:
try:
devices[dev['type']].append(dev)
except KeyError:
@@ -2639,7 +2639,7 @@
def _run(self):
self.log.info("VM wrapper has started")
- devices = self.buildConfDevices()
+ devices = self.setupDevicesConf(self.conf.get('devices'))
# recovery flow note:
# we do not start disk stats collection here since
--
To view, visit http://gerrit.ovirt.org/34665
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1df2a31458a25e06f3713bc4dd978bf4d9ea2439
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
9 years, 4 months
Change in vdsm[master]: vm: reorder device normalization
by fromani@redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: vm: reorder device normalization
......................................................................
vm: reorder device normalization
as part of Vm startup, device configuration is checked
and device is normalized.
The last step of normalization is for balloon devices.
But there are no solid reasons for existing ordering,
which is most likelt due to historical reasons only.
This patch reorder the check and the normalization to
make the storage device the last being checked, so
the road is paved to split the fast check and the slow
checks.
Change-Id: If211e80d731de27dd5d5154c32dd63bef5c2be2e
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 4 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/34664/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 8b17a03..87c6517 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1416,6 +1416,10 @@
self._checkDeviceLimits(devices)
+ # Preserve old behavior. Since libvirt add a memory balloon device
+ # to all guests, we need to specifically request not to add it.
+ self._normalizeBalloonDevice(devices[BALLOON_DEVICES])
+
# Normalize vdsm images
for drv in devices[DISK_DEVICES]:
if isVdsmImage(drv):
@@ -1429,10 +1433,6 @@
raise
self.normalizeDrivesIndices(devices[DISK_DEVICES])
-
- # Preserve old behavior. Since libvirt add a memory balloon device
- # to all guests, we need to specifically request not to add it.
- self._normalizeBalloonDevice(devices[BALLOON_DEVICES])
return devices
--
To view, visit http://gerrit.ovirt.org/34664
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If211e80d731de27dd5d5154c32dd63bef5c2be2e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
9 years, 4 months
Change in vdsm[master]: schema: fix guest disk mapping versioning
by laravot@redhat.com
Liron Aravot has uploaded a new change for review.
Change subject: schema: fix guest disk mapping versioning
......................................................................
schema: fix guest disk mapping versioning
The guest disk mapping report feature is being merged to the ovirt-3.5
branch now, as the current version is 4.16.8 the version on the master
branch should be fixed as well.
Change-Id: I62a532ddef597ecafcdaf454f70dfe5ec2434e1a
Signed-off-by: Liron Aravot <laravot(a)redhat.com>
---
M vdsm/rpc/vdsmapi-schema.json
1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/36/34636/1
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index b6a835e..624b686 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -3579,7 +3579,7 @@
#
# @name: device logical name within the guest.
#
-# Since: 4.16.0
+# Since: 4.16.8
##
{'type': 'GuestDiskMappingInfo',
'data': {'name': 'str'}}
@@ -3590,7 +3590,7 @@
#
# guest disks mapping information indexed by serial.
#
-# Since: 4.16.0
+# Since: 4.16.8
##
{'map': 'GuestDisksMappingMap',
'key': 'str', 'value': 'GuestDiskMappingInfo'}
@@ -3664,7 +3664,7 @@
# @guestDiskMapping: A dictionary containing information about the disk
# mapping within the guest. The key is the device
# serial and the value is the mapping information.
-# (New in version 4.16.0.)
+# (New in version 4.16.8.)
#
# @smartcardEnable: Info whether smartcard is enabled.
#
--
To view, visit http://gerrit.ovirt.org/34636
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I62a532ddef597ecafcdaf454f70dfe5ec2434e1a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <laravot(a)redhat.com>
9 years, 4 months
Change in vdsm[ovirt-3.5]: vm: return diskMapping on status()
by laravot@redhat.com
Liron Aravot has uploaded a new change for review.
Change subject: vm: return diskMapping on status()
......................................................................
vm: return diskMapping on status()
status() is used by API.Global.getVMList and should return also the disk
mapping retrieved by the guest agent.
Change-Id: Ia68ff3c36ba91cde893876a16fc6702419a60d49
Signed-off-by: Liron Aravot <laravot(a)redhat.com>
---
M tests/vmTests.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/virt/vm.py
3 files changed, 39 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/33197/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 32fa5e0..a89a8d3 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -894,6 +894,9 @@
class FakeGuestAgent(object):
+ def __init__(self):
+ self.guestDiskMapping = {}
+
def getGuestInfo(self):
return {
'username': 'Unknown',
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index db441f5..a2f2865 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -3434,6 +3434,29 @@
'returns': ['UUID']}
##
+# @GuestDiskMappingInfo:
+#
+# disk mapping information indexed by property name.
+#
+# @name: device logical name within the guest.
+#
+# Since: 4.16.0
+##
+{'type': 'GuestDiskMappingInfo',
+ 'data': {'name': 'str'}}
+
+
+##
+# @GuestDisksMappingMap:
+#
+# guest disks mapping information indexed by serial.
+#
+# Since: 4.16.0
+##
+{'map': 'GuestDisksMappingMap',
+ 'key': 'str', 'value': 'GuestDiskMappingInfo'}
+
+##
# @VMFullInfo:
#
# Full information about VM.
@@ -3499,6 +3522,11 @@
#
# @guestIPs: A space separated string of assigned IPv4 addresses
#
+# @guestDiskMapping: A dictionary containing information about the disk
+# mapping within the guest. The key is the device
+# serial and the value is the mapping information.
+# (New in version 4.16.0.)
+#
# @smartcardEnable: Info whether smartcard is enabled.
#
# @nicModel: The type of device that is exposed to the VM operating system
@@ -3521,9 +3549,10 @@
'username': 'str', 'emulatedMachine': 'str', 'pid': 'uint',
'spiceSslCipherSuite': 'str', 'cpuType': 'str', 'pauseCode': 'str',
'guestFQDN': 'str', 'displayIp': 'str', 'keyboardLayout': 'str',
- 'displayPort': 'uint', 'guestIPs': 'str', 'smartcardEnable': 'bool',
- 'nicModel': 'VmInterfaceDeviceModel', 'pitReinjection': 'bool',
- 'status': 'str', 'clientIp': 'str'}}
+ 'displayPort': 'uint', 'guestIPs': 'str',
+ 'guestDiskMapping': 'GuestDisksMappingMap',
+ 'smartcardEnable': 'bool', 'nicModel': 'VmInterfaceDeviceModel',
+ 'pitReinjection': 'bool', 'status': 'str', 'clientIp': 'str'}}
##
# @Host.getVMFullList:
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 0f9e4ce..548ce18 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -2852,8 +2852,10 @@
# used by API.Global.getVMList
self.conf['status'] = self.lastStatus
# Filter out any internal keys
- return dict((k, v) for k, v in self.conf.iteritems()
- if not k.startswith("_"))
+ status = dict((k, v) for k, v in self.conf.iteritems()
+ if not k.startswith("_"))
+ status['guestDiskMapping'] = self.guestAgent.guestDiskMapping
+ return status
def getStats(self):
"""
--
To view, visit http://gerrit.ovirt.org/33197
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia68ff3c36ba91cde893876a16fc6702419a60d49
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Liron Aravot <laravot(a)redhat.com>
9 years, 4 months