Change in vdsm[ovirt-3.4]: vnic QoS: Fix specParams to XML definition conversion
by asegurap＠redhat.com
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/26254
to review the following change.
Change subject: vnic QoS: Fix specParams to XML definition conversion
......................................................................
vnic QoS: Fix specParams to XML definition conversion
VDSM API specifies specParams input and output data to have values
specified as integers. In order to generate valid XML, those values
should have been converted to strings (and the test should have been
passing integers as per VDSM schema).
This patch fixes the conversion code and the tests for it (fix one,
add a new one for the update flow).
Change-Id: I25041b8f853c870388ecd9c9f9176bd384242234
Bug-Url: https://bugzilla.redhat.com/1067064
Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/25788
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com>
---
M tests/vmTests.py
M vdsm/vm.py
2 files changed, 46 insertions(+), 22 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/26254/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 6215041..e061d42 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -394,14 +394,42 @@
'network': 'ovirtmgmt', 'address': self.PCI_ADDR_DICT,
'device': 'bridge', 'type': 'interface',
'bootOrder': '1', 'filter': 'no-mac-spoofing',
- 'specParams': {'inbound': {'average': '1000', 'peak': '5000',
- 'burst': '1024'},
- 'outbound': {'average': '128', 'burst': '256'}}}
+ 'specParams': {'inbound': {'average': 1000, 'peak': 5000,
+ 'burst': 1024},
+ 'outbound': {'average': 128, 'burst': 256}}}
self.conf['custom'] = {'vhost': 'ovirtmgmt:true', 'sndbuf': '0'}
iface = vm.NetworkInterfaceDevice(self.conf, self.log, **dev)
self.assertXML(iface.getXML(), interfaceXML)
+ def testInterfaceXMLBandwidthUpdate(self):
+ originalBwidthXML = """
+ <bandwidth>
+ <inbound average="1000" burst="1024" peak="5000"/>
+ <outbound average="128" burst="256"/>
+ </bandwidth>"""
+ NEW_OUT = {'outbound': {'average': 1042, 'burst': 128, 'peak': 500}}
+ updatedBwidthXML = """
+ <bandwidth>
+ <inbound average="1000" burst="1024" peak="5000"/>
+ <outbound average="%(average)s" burst="%(burst)s"
+ peak="%(peak)s"/>
+ </bandwidth>""" % NEW_OUT['outbound']
+
+ dev = {'nicModel': 'virtio', 'macAddr': '52:54:00:59:F5:3F',
+ 'network': 'ovirtmgmt', 'address': self.PCI_ADDR_DICT,
+ 'device': 'bridge', 'type': 'interface',
+ 'bootOrder': '1', 'filter': 'no-mac-spoofing',
+ 'specParams': {'inbound': {'average': 1000, 'peak': 5000,
+ 'burst': 1024},
+ 'outbound': {'average': 128, 'burst': 256}}}
+ self.conf['custom'] = {'vhost': 'ovirtmgmt:true', 'sndbuf': '0'}
+ iface = vm.NetworkInterfaceDevice(self.conf, self.log, **dev)
+ originalBandwidth = iface.getXML().getElementsByTagName('bandwidth')[0]
+ self.assertXML(originalBandwidth, originalBwidthXML)
+ self.assertXML(iface.paramsToBandwidthXML(NEW_OUT, originalBandwidth),
+ updatedBwidthXML)
+
def testControllerXML(self):
devConfs = [
{'device': 'ide', 'index': '0', 'address': self.PCI_ADDR_DICT},
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 832bca8..09e6031 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -1413,27 +1413,23 @@
if hasattr(self, 'specParams'):
if 'inbound' in self.specParams or 'outbound' in self.specParams:
- iface.appendChild(self.getXMLBandwidth(self.specParams))
+ iface.appendChild(self.paramsToBandwidthXML(self.specParams))
return iface
- def getXMLBandwidth(self, specParams, oldBandwidth=None):
+ def paramsToBandwidthXML(self, specParams, oldBandwidth=None):
+ """Returns a valid libvirt xml dom element object."""
bandwidth = self.createXmlElem('bandwidth', None)
- # Inbound and Outbound traffic can be independently shaped.
- for attr in ('inbound', 'outbound'):
- newSetting = specParams.get(attr)
- # if newSetting is specified, replace current settings
- if newSetting:
- bandwidth.appendChildWithArgs(attr, **newSetting)
- # if newSetting is not specified, keep current settings
- elif newSetting is None:
- if oldBandwidth is not None:
- attrXMLs = oldBandwidth.getElementsByTagName
- attrXML = attrXMLs[0] if len(attrXMLs) else None
- else:
- attrXML = None
- if attrXML is not None:
- bandwidth.appendChild(attrXML)
- # if newSetting is {} do nothing = remove current settings
+ old = {} if oldBandwidth is None else dict(
+ (elem.nodeName, elem) for elem in oldBandwidth.childNodes)
+ for key in ('inbound', 'outbound'):
+ elem = specParams.get(key)
+ if elem is None: # Use the old setting if present
+ if key in old:
+ bandwidth.appendChild(old[key])
+ elif elem:
+ # Convert the values to string for adding them to the XML def
+ attrs = dict((key, str(value)) for key, value in elem.items())
+ bandwidth.appendChildWithArgs(key, **attrs)
return bandwidth
@@ -3344,7 +3340,7 @@
('inbound' in specParams or 'outbound' in specParams)):
oldBandwidths = vnicXML.getElementsByTagName('bandwidth')
oldBandwidth = oldBandwidths[0] if len(oldBandwidths) else None
- newBandwidth = dev.getXMLBandwidth(specParams, oldBandwidth)
+ newBandwidth = dev.paramsToBandwidthXML(specParams, oldBandwidth)
if oldBandwidth is None:
vnicXML.appendChild(newBandwidth)
else:
--
To view, visit http://gerrit.ovirt.org/26254
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I25041b8f853c870388ecd9c9f9176bd384242234
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
10 years
Change in vdsm[master]: pep8: make pep8-1.5 happy with E265
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: pep8: make pep8-1.5 happy with E265
......................................................................
Patch Set 3:
I got a few more fixes for pep8 1.5.x in the pipeline, I'll post some patches soon
--
To view, visit http://gerrit.ovirt.org/26248
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I91c49e1c0f36590b8052462b9e066c110eacd67b
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com>
Gerrit-Reviewer: Eyal Edri <eedri(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Ohad Basan <obasan(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10 years
Change in vdsm[master]: gluster: correct GLUSTER_RPM_PACKAGES
by Jenkins CI RO
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: correct GLUSTER_RPM_PACKAGES
......................................................................
Patch Set 3:
No Builds Executed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7042/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7944/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7833/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
--
To view, visit http://gerrit.ovirt.org/26379
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c549b7d36d9c44badb0c1edee7c105f1677a00
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Stanislav Mikhalevich <msio57(a)gmail.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10 years
Change in vdsm[master]: gluster: correct GLUSTER_RPM_PACKAGES
by Jenkins CI RO
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: correct GLUSTER_RPM_PACKAGES
......................................................................
Patch Set 2:
No Builds Executed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7041/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7943/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7832/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
--
To view, visit http://gerrit.ovirt.org/26379
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c549b7d36d9c44badb0c1edee7c105f1677a00
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Stanislav Mikhalevich <msio57(a)gmail.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10 years
Change in vdsm[master]: gluster: correct GLUSTER_RPM_PACKAGES
by Jenkins CI RO
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: correct GLUSTER_RPM_PACKAGES
......................................................................
Patch Set 1:
No Builds Executed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7040/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7942/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7831/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
--
To view, visit http://gerrit.ovirt.org/26379
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c549b7d36d9c44badb0c1edee7c105f1677a00
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Stanislav Mikhalevich <msio57(a)gmail.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10 years
Change in vdsm[master]: lvm: remove unused add/remove tags
by ykaplan＠redhat.com
Yeela Kaplan has uploaded a new change for review.
Change subject: lvm: remove unused add/remove tags
......................................................................
lvm: remove unused add/remove tags
Change-Id: Ibffb9138cb28e150f18e2283c24a60c165cde337
Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com>
---
M vdsm/storage/lvm.py
M vdsm/storage/storage_exception.py
2 files changed, 0 insertions(+), 35 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/25013/1
diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 24d023d..7e7ad06 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -1339,33 +1339,8 @@
(vgName, ", ".join(delTags), ", ".join(addTags), err[-1]))
-def addVGTag(vgName, tag):
- _lvminfo._invalidatevgs(vgName)
- cmd = ["vgchange", "--addtag", tag, vgName]
- rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))
- if rc != 0:
- raise se.VolumeGroupAddTagError("Failed adding tag %s to VG %s." %
- (tag, vgName))
-
-
-def remVGTag(vgName, tag):
- _lvminfo._invalidatevgs(vgName)
- cmd = ["vgchange", "--deltag", tag, vgName]
- rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))
- if rc != 0:
- raise se.VolumeGroupRemoveTagError(vgName)
-
-
def replaceVGTag(vg, oldTag, newTag):
changeVGTags(vg, [oldTag], [newTag])
-
-
-def addVGTags(vgName, tags):
- changeVGTags(vgName, addTags=tags)
-
-
-def remVGTags(vgName, tags):
- changeVGTags(vgName, delTags=tags)
def getFirstExt(vg, lv):
diff --git a/vdsm/storage/storage_exception.py b/vdsm/storage/storage_exception.py
index ae68a6f..0317794 100644
--- a/vdsm/storage/storage_exception.py
+++ b/vdsm/storage/storage_exception.py
@@ -1249,16 +1249,6 @@
message = "Read Volume Group's tag error"
-class VolumeGroupAddTagError(StorageException):
- code = 511
- message = "Add tag to Volume Group error"
-
-
-class VolumeGroupRemoveTagError(StorageException):
- code = 512
- message = "Remove tag from Volume Group error"
-
-
class VolumeGroupScanError(StorageException):
code = 513
message = "Volume Group scanning error"
--
To view, visit http://gerrit.ovirt.org/25013
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibffb9138cb28e150f18e2283c24a60c165cde337
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
10 years
Change in vdsm[master]: core: fixing attribute error on fenceSpmStorage
by laravot＠redhat.com
Liron Ar has uploaded a new change for review.
Change subject: core: fixing attribute error on fenceSpmStorage
......................................................................
core: fixing attribute error on fenceSpmStorage
In change I75493d1db60e51cccd5231b516f963c970d24c99 the pool metadata
operations were moved to the pool backend, while the call to
invalidateMetadata in fenceSpmStorage was left as it was, leading to an
attribute error.
This patch fixes it by checking the backend type before executing it,
though it's not the nicest way for doing so, it's the pattern
already being used in other places in hsm.py,therefore in the
meanwhile it can be fixed the same way.
Change-Id: I48bb2211c6e62d1589f1a56616df105c52654344
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1082365
Signed-off-by: Liron Aravot <laravot(a)redhat.com>
---
M vdsm/storage/hsm.py
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/57/26357/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 0ebe129..d9697ef 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3529,7 +3529,8 @@
se.SpmFenceError("spUUID=%s, lastOwner=%s, lastLver=%s" %
(spUUID, lastOwner, lastLver)))
pool = self.getPool(spUUID)
- pool.invalidateMetadata()
+ if isinstance(pool.getBackend(), StoragePoolDiskBackend):
+ pool.getBackend().invalidateMetadata()
vars.task.getExclusiveLock(STORAGE, spUUID)
pool.forceFreeSpm()
return dict(spm_st=self._getSpmStatusInfo(pool))
--
To view, visit http://gerrit.ovirt.org/26357
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I48bb2211c6e62d1589f1a56616df105c52654344
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot(a)redhat.com>
10 years
Change in vdsm[master]: Do not allow empty bridge names
by asegurap＠redhat.com
Antoni Segura Puimedon has uploaded a new change for review.
Change subject: Do not allow empty bridge names
......................................................................
Do not allow empty bridge names
It was possible to create empty bridge names that ended up in
libvirt networks called 'vdsm-'. This patch solved it returning
the appropriate error.
Change-Id: I1d1e5d9fb284eb67936daf197e3025314a29f58b
Bug-Url: https://bugzilla.redhat.com/1078899
Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com>
---
M tests/netmodelsTests.py
M vdsm/network/models.py
2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/63/26263/1
diff --git a/tests/netmodelsTests.py b/tests/netmodelsTests.py
index 551c836..a2a3ee3 100644
--- a/tests/netmodelsTests.py
+++ b/tests/netmodelsTests.py
@@ -49,7 +49,7 @@
self.assertEqual(Vlan.validateTag(Vlan.MAX_ID), None)
def testIsBridgeNameValid(self):
- invalidBrName = ('-abc', 'abcdefghijklmnop', 'a:b', 'a.b')
+ invalidBrName = ('', '-abc', 'abcdefghijklmnop', 'a:b', 'a.b')
for i in invalidBrName:
with self.assertRaises(errors.ConfigNetworkError) as cneContext:
Bridge.validateName(i)
diff --git a/vdsm/network/models.py b/vdsm/network/models.py
index bce7802..ced0a30 100644
--- a/vdsm/network/models.py
+++ b/vdsm/network/models.py
@@ -165,7 +165,7 @@
@classmethod
def validateName(cls, name):
- if not (name and len(name) <= cls.MAX_NAME_LEN and
+ if not (name and 0 < len(name) <= cls.MAX_NAME_LEN and
len(set(name) & cls.ILLEGAL_CHARS) == 0 and
not name.startswith('-')):
raise ConfigNetworkError(ne.ERR_BAD_BRIDGE,
--
To view, visit http://gerrit.ovirt.org/26263
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1d1e5d9fb284eb67936daf197e3025314a29f58b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap(a)redhat.com>
10 years
Change in vdsm[master]: caps: Collect numa information
by michal.skrivanek＠redhat.com
Michal Skrivanek has posted comments on this change.
Change subject: caps: Collect numa information
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.ovirt.org/#/c/23703/8/vdsm/caps.py
File vdsm/caps.py:
Line 165:
Line 166:
Line 167: @utils.memoized
Line 168: def _getNumaTopology():
Line 169: capabilities = _getCapsXMLStr()
I'd suggest to use the same code as at line 146. And then as a separate patch it should be refactored so we don't have to do this in every function:)
Line 170: caps = minidom.parseString(capabilities)
Line 171: host = caps.getElementsByTagName('host')[0]
Line 172: cells = host.getElementsByTagName('cells')[0]
Line 173: cellsInfo = {}
--
To view, visit http://gerrit.ovirt.org/23703
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I63eeb697ab986c3b9cad0dc44f41924f329e52cd
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xiaolei Shi <xiao-lei.shi(a)hp.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchaplik(a)redhat.com>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa <gustavo.pedrosa(a)eldorado.org.br>
Gerrit-Reviewer: Leonardo Bianconi <leonardo.bianconi(a)eldorado.org.br>
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vitor de Lima <vitor.lima(a)eldorado.org.br>
Gerrit-Reviewer: Xiaolei Shi <xiao-lei.shi(a)hp.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
10 years