Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/30078
to review the following change.
Change subject: Change method names in vmtune.py to the PEP8 style
......................................................................
Change method names in vmtune.py to the PEP8 style
Change-Id: Ib5e9dcaf393ebafe56f6bc78cc9cb02d37a257ae
Signed-off-by: Martin Sivak <msivak(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/29929
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M tests/vmTests.py
M vdsm/virt/vm.py
M vdsm/virt/vmtune.py
3 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/78/30078/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 498aeac..999de1b 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -30,7 +30,7 @@
from virt import vm
from virt import vmexitreason
-from virt.vmtune import ioTuneMerge, ioTuneDomToValues, ioTuneToDom
+from virt.vmtune import io_tune_merge, io_tune_dom_to_values, io_tune_to_dom
from vdsm import constants
from vdsm import define
from testrunner import VdsmTestCase as TestCaseBase
@@ -984,8 +984,8 @@
}
}
- dom = ioTuneToDom(ioTuneValues)
- parsed = ioTuneDomToValues(dom)
+ dom = io_tune_to_dom(ioTuneValues)
+ parsed = io_tune_dom_to_values(dom)
self.assertEqual(ioTuneValues, parsed)
@@ -1037,7 +1037,7 @@
}
}
- ioTuneMerged = ioTuneMerge(ioTuneValues1, ioTuneValues2)
+ ioTuneMerged = io_tune_merge(ioTuneValues1, ioTuneValues2)
self.assertEqual(ioTuneMerged, ioTuneExpectedValues)
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 536c146..9581357 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -64,8 +64,8 @@
from . import sampling
from . import vmexitreason
from . import vmstatus
-from .vmtune import updateIoTuneDom, collectInnerElements
-from .vmtune import ioTuneValuesToDom, ioTuneDomToValues
+from .vmtune import update_io_tune_dom, collect_inner_elements
+from .vmtune import io_tune_values_to_dom, io_tune_dom_to_values
from .utils import XMLElement
from vmpowerdown import VmShutdown, VmReboot
@@ -3689,7 +3689,7 @@
qos.appendChild(ioTuneElement)
metadata_modified = True
- if updateIoTuneDom(ioTuneList[0], params["ioTune"]) > 0:
+ if update_io_tune_dom(ioTuneList[0], params["ioTune"]) > 0:
metadata_modified = True
del params['ioTune']
@@ -3761,7 +3761,7 @@
return []
for device in ioTuneList[0].getElementsByTagName("device"):
- tunables.append(ioTuneDomToValues(device))
+ tunables.append(io_tune_dom_to_values(device))
return tunables
@@ -3784,7 +3784,7 @@
io_dom_list = dom.getElementsByTagName("iotune")
old_io_tune = {}
if io_dom_list:
- collectInnerElements(io_dom_list[0], old_io_tune)
+ collect_inner_elements(io_dom_list[0], old_io_tune)
old_io_tune.update(io_tune)
io_tune = old_io_tune
@@ -3813,7 +3813,7 @@
if io_dom_list:
dom.removeChild(io_dom_list[0])
io_dom = XMLElement("iotune")
- ioTuneValuesToDom(io_tune, io_dom)
+ io_tune_values_to_dom(io_tune, io_dom)
dom.appendChild(io_dom)
found_device.specParams['ioTune'] = io_tune
diff --git a/vdsm/virt/vmtune.py b/vdsm/virt/vmtune.py
index 06558fe..ed5d575 100644
--- a/vdsm/virt/vmtune.py
+++ b/vdsm/virt/vmtune.py
@@ -27,7 +27,7 @@
log = logging.getLogger(__name__)
-def ioTuneValuesToDom(values, dom):
+def io_tune_values_to_dom(values, dom):
"""
Create a DOM representation of the passed iotune values and
attach it to the dom object in the form of nodes.
@@ -46,7 +46,7 @@
dom.appendChild(el)
-def collectInnerElements(el, d):
+def collect_inner_elements(el, d):
"""
This helper method collects all nodes in el and adds them
to dictionary d.
@@ -64,7 +64,7 @@
log.exception("Invalid value for %s", chel.localName)
-def ioTuneDomToValues(dom):
+def io_tune_dom_to_values(dom):
"""
This method converts the VmDiskDeviceTuneLimits structure from its
XML representation to the dictionary representation.
@@ -83,18 +83,18 @@
els = dom.getElementsByTagName("guaranteed")
if els:
values["guaranteed"] = {}
- collectInnerElements(els[0], values["guaranteed"])
+ collect_inner_elements(els[0], values["guaranteed"])
els = dom.getElementsByTagName("maximum")
if els:
values["maximum"] = {}
for chel in els[0].childNodes:
- collectInnerElements(els[0], values["maximum"])
+ collect_inner_elements(els[0], values["maximum"])
return values
-def ioTuneToDom(tune):
+def io_tune_to_dom(tune):
"""
This method converts the VmDiskDeviceTuneLimits structure from the
dictionary representation to the XML representation.
@@ -113,17 +113,17 @@
if "maximum" in tune:
maximum = XMLElement("maximum")
device.appendChild(maximum)
- ioTuneValuesToDom(tune["maximum"], maximum)
+ io_tune_values_to_dom(tune["maximum"], maximum)
if "guaranteed" in tune:
guaranteed = XMLElement("guaranteed")
device.appendChild(guaranteed)
- ioTuneValuesToDom(tune["guaranteed"], guaranteed)
+ io_tune_values_to_dom(tune["guaranteed"], guaranteed)
return device
-def ioTuneMerge(old, new):
+def io_tune_merge(old, new):
"""
Merge two VmDiskDeviceTuneLimits structures in their dictionary form
and return the new iotune setting.
@@ -152,7 +152,7 @@
return result
-def createDeviceIndex(ioTune):
+def create_device_index(ioTune):
"""
Create by name / by path dictionaries from the XML representation.
Returns a tuple (by_name, by_path) where the items are the respective
@@ -176,7 +176,7 @@
return ioTuneByName, ioTuneByPath
-def updateIoTuneDom(ioTune, tunables):
+def update_io_tune_dom(ioTune, tunables):
"""
This method takes a list of VmDiskDeviceTuneLimits objects and applies
the changes to the XML element representing the current iotune settings,
@@ -191,7 +191,7 @@
count = 0
# Get all existing ioTune records and create name/path index
- ioTuneByName, ioTuneByPath = createDeviceIndex(ioTune)
+ ioTuneByName, ioTuneByPath = create_device_index(ioTune)
for limit_object in tunables:
old_tune = None
@@ -205,10 +205,10 @@
ioTune.removeChild(old_tune)
if old_tune is not None:
- old_object = ioTuneDomToValues(old_tune)
- limit_object = ioTuneMerge(old_object, limit_object)
+ old_object = io_tune_dom_to_values(old_tune)
+ limit_object = io_tune_merge(old_object, limit_object)
- new_tune = ioTuneToDom(limit_object)
+ new_tune = io_tune_to_dom(limit_object)
ioTune.appendChild(new_tune)
count += 1
--
To view, visit http://gerrit.ovirt.org/30078
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib5e9dcaf393ebafe56f6bc78cc9cb02d37a257ae
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/30074
to review the following change.
Change subject: Add IO tunables support to updateVmPolicy
......................................................................
Add IO tunables support to updateVmPolicy
This allows the engine to set the hard and soft limits for IO.
MoM will then use those limits as a boundaries for dynamic
iotune updates.
Change-Id: I4ed108fbb2bf9d9af80577b2905242bf9f8c4221
Signed-off-by: Martin Sivak <msivak(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/28715
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M tests/vmTests.py
M vdsm.spec.in
M vdsm/rpc/vdsmapi-schema.json
M vdsm/virt/Makefile.am
M vdsm/virt/vm.py
A vdsm/virt/vmtune.py
6 files changed, 588 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/30074/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 02b1339..5c3d445 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -30,6 +30,7 @@
from virt import vm
from virt import vmexitreason
+from virt.vmtune import ioTuneMerge, ioTuneDomToValues, ioTuneToDom
from vdsm import constants
from vdsm import define
from testrunner import VdsmTestCase as TestCaseBase
@@ -58,6 +59,7 @@
self._xml = xml
self.devXml = ''
self._virtError = virtError
+ self._metadata = ""
def _failIfRequested(self):
if self._virtError != libvirt.VIR_ERR_OK:
@@ -79,7 +81,15 @@
def metadata(self, type, uri, flags):
self._failIfRequested()
- return '<qos></qos>'
+
+ if not self._metadata:
+ e = libvirt.libvirtError("No metadata")
+ e.err = [libvirt.VIR_ERR_NO_DOMAIN_METADATA]
+ raise e
+ return self._metadata
+
+ def setMetadata(self, type, xml, prefix, uri, flags):
+ self._metadata = xml
def schedulerParameters(self):
return {'vcpu_quota': vm._NO_CPU_QUOTA,
@@ -862,6 +872,279 @@
fake._dom = FakeDomain(virtError=libvirt.VIR_ERR_NO_DOMAIN)
self.assertEqual(fake._getVmPolicy(), None)
+ def _xml_sanitizer(self, text):
+ return re.sub(">[\t\n ]+<", "><", text).strip()
+
+ def testUpdateVmPolicy(self):
+ with FakeVM() as machine:
+ dom = FakeDomain()
+ machine._dom = dom
+
+ policy = {
+ "vcpuLimit": 50,
+ "ioTune": [
+ {
+ "name": "test-device-by-name",
+ "maximum": {
+ "total_bytes_sec": 200, "total_iops_sec": 201,
+ "read_bytes_sec": 202, "read_iops_sec": 203,
+ "write_bytes_sec": 204, "write_iops_sec": 205
+ },
+ "guaranteed": {
+ "total_bytes_sec": 100, "total_iops_sec": 101,
+ "read_bytes_sec": 102, "read_iops_sec": 103,
+ "write_bytes_sec": 104, "write_iops_sec": 105
+ }
+ },
+ {
+ "path": "test-device-by-path",
+ "maximum": {
+ "total_bytes_sec": 400, "total_iops_sec": 401,
+ "read_bytes_sec": 402, "read_iops_sec": 403,
+ "write_bytes_sec": 404, "write_iops_sec": 405
+ },
+ "guaranteed": {
+ "total_bytes_sec": 300, "total_iops_sec": 301,
+ "read_bytes_sec": 302, "read_iops_sec": -1,
+ "write_bytes_sec": 304, "write_iops_sec": 305
+ }
+ }
+ ]
+ }
+
+ machine.updateVmPolicy(policy)
+
+ expected_xml = self._xml_sanitizer(u"""
+ <qos>
+ <vcpuLimit>50</vcpuLimit>
+ <ioTune>
+ <device name="test-device-by-name">
+ <maximum>
+ <total_bytes_sec>200</total_bytes_sec>
+ <total_iops_sec>201</total_iops_sec>
+ <read_bytes_sec>202</read_bytes_sec>
+ <read_iops_sec>203</read_iops_sec>
+ <write_bytes_sec>204</write_bytes_sec>
+ <write_iops_sec>205</write_iops_sec>
+ </maximum>
+ <guaranteed>
+ <total_bytes_sec>100</total_bytes_sec>
+ <total_iops_sec>101</total_iops_sec>
+ <read_bytes_sec>102</read_bytes_sec>
+ <read_iops_sec>103</read_iops_sec>
+ <write_bytes_sec>104</write_bytes_sec>
+ <write_iops_sec>105</write_iops_sec>
+ </guaranteed>
+ </device>
+ <device path="test-device-by-path">
+ <maximum>
+ <total_bytes_sec>400</total_bytes_sec>
+ <total_iops_sec>401</total_iops_sec>
+ <read_bytes_sec>402</read_bytes_sec>
+ <read_iops_sec>403</read_iops_sec>
+ <write_bytes_sec>404</write_bytes_sec>
+ <write_iops_sec>405</write_iops_sec>
+ </maximum>
+ <guaranteed>
+ <total_bytes_sec>300</total_bytes_sec>
+ <total_iops_sec>301</total_iops_sec>
+ <read_bytes_sec>302</read_bytes_sec>
+ <write_bytes_sec>304</write_bytes_sec>
+ <write_iops_sec>305</write_iops_sec>
+ </guaranteed>
+ </device>
+ </ioTune>
+ </qos>
+ """)
+
+ self.assertEqual(expected_xml, self._xml_sanitizer(dom._metadata))
+
+ def testIoTuneParser(self):
+ with FakeVM() as machine:
+ dom = FakeDomain()
+ machine._dom = dom
+
+ ioTuneValues = {
+ "name": "test-device-by-name",
+ "path": "test-path",
+ "maximum": {
+ "total_bytes_sec": 200, "total_iops_sec": 201,
+ "read_bytes_sec": 202, "read_iops_sec": 203,
+ "write_bytes_sec": 204, "write_iops_sec": 205
+ },
+ "guaranteed": {
+ "total_bytes_sec": 100, "total_iops_sec": 101,
+ "read_bytes_sec": 102, "read_iops_sec": 103,
+ "write_bytes_sec": 104, "write_iops_sec": 105
+ }
+ }
+
+ dom = ioTuneToDom(ioTuneValues)
+ parsed = ioTuneDomToValues(dom)
+
+ self.assertEqual(ioTuneValues, parsed)
+
+ def testIoTuneMerge(self):
+ with FakeVM() as machine:
+ dom = FakeDomain()
+ machine._dom = dom
+
+ ioTuneValues1 = {
+ "path": "test-path",
+ "maximum": {
+ "total_bytes_sec": 0, "total_iops_sec": 0,
+ "read_bytes_sec": 0,
+ "write_bytes_sec": 999, "write_iops_sec": 0
+ },
+ "guaranteed": {
+ "total_bytes_sec": 999, "total_iops_sec": 0,
+ "read_bytes_sec": 0, "read_iops_sec": 0,
+ "write_bytes_sec": 0, "write_iops_sec": 0
+ }
+ }
+
+ ioTuneValues2 = {
+ "name": "test-device-by-name",
+ "maximum": {
+ "total_bytes_sec": 200, "total_iops_sec": 201,
+ "read_bytes_sec": 202, "read_iops_sec": 203,
+ "write_iops_sec": 205
+ },
+ "guaranteed": {
+ "total_bytes_sec": -1, "total_iops_sec": 101,
+ "read_bytes_sec": 102, "read_iops_sec": 103,
+ "write_bytes_sec": 104, "write_iops_sec": 105
+ }
+ }
+
+ ioTuneExpectedValues = {
+ "name": "test-device-by-name",
+ "path": "test-path",
+ "maximum": {
+ "total_bytes_sec": 200, "total_iops_sec": 201,
+ "read_bytes_sec": 202, "read_iops_sec": 203,
+ "write_bytes_sec": 999, "write_iops_sec": 205
+ },
+ "guaranteed": {
+ "total_bytes_sec": -1, "total_iops_sec": 101,
+ "read_bytes_sec": 102, "read_iops_sec": 103,
+ "write_bytes_sec": 104, "write_iops_sec": 105
+ }
+ }
+
+ ioTuneMerged = ioTuneMerge(ioTuneValues1, ioTuneValues2)
+
+ self.assertEqual(ioTuneMerged, ioTuneExpectedValues)
+
+ def testUpdateExistingVmPolicy(self):
+ with FakeVM() as machine:
+ dom = FakeDomain()
+ dom._metadata = """
+ <qos>
+ <vcpuLimit>999</vcpuLimit>
+ <ioTune>
+ <device name='test-device-by-name'>
+ <maximum>
+ <totalBytes>9999</totalBytes>
+ </maximum>
+ </device>
+ <device name='other-device'>
+ <maximum>
+ <totalBytes>9999</totalBytes>
+ </maximum>
+ </device>
+ </ioTune>
+ </qos>
+ """
+
+ machine._dom = dom
+
+ policy = {
+ "vcpuLimit": 50,
+ "ioTune": [
+ {
+ "name": "test-device-by-name",
+ "maximum": {
+ "total_bytes_sec": 200, "total_iops_sec": 201,
+ "read_bytes_sec": 202, "read_iops_sec": 203,
+ "write_bytes_sec": 204, "write_iops_sec": 205
+ },
+ "guaranteed": {
+ "total_bytes_sec": 100, "total_iops_sec": 101,
+ "read_bytes_sec": 102, "read_iops_sec": 103,
+ "write_bytes_sec": 104, "write_iops_sec": 105
+ }
+ },
+ {
+ "path": "test-device-by-path",
+ "maximum": {
+ "total_bytes_sec": 400, "total_iops_sec": 401,
+ "read_bytes_sec": 402, "read_iops_sec": 403,
+ "write_bytes_sec": 404, "write_iops_sec": 405
+ },
+ "guaranteed": {
+ "total_bytes_sec": 300, "total_iops_sec": 301,
+ "read_bytes_sec": 302, "read_iops_sec": 303,
+ "write_bytes_sec": 304, "write_iops_sec": 305
+ }
+ }
+ ]
+ }
+
+ machine.updateVmPolicy(policy)
+
+ expected_xml = self._xml_sanitizer(u"""
+ <qos>
+ <ioTune>
+ <device name="other-device">
+ <maximum>
+ <totalBytes>9999</totalBytes>
+ </maximum>
+ </device>
+ <device name="test-device-by-name">
+ <maximum>
+ <total_bytes_sec>200</total_bytes_sec>
+ <total_iops_sec>201</total_iops_sec>
+ <read_bytes_sec>202</read_bytes_sec>
+ <read_iops_sec>203</read_iops_sec>
+ <write_bytes_sec>204</write_bytes_sec>
+ <write_iops_sec>205</write_iops_sec>
+ </maximum>
+ <guaranteed>
+ <total_bytes_sec>100</total_bytes_sec>
+ <total_iops_sec>101</total_iops_sec>
+ <read_bytes_sec>102</read_bytes_sec>
+ <read_iops_sec>103</read_iops_sec>
+ <write_bytes_sec>104</write_bytes_sec>
+ <write_iops_sec>105</write_iops_sec>
+ </guaranteed>
+ </device>
+ <device path="test-device-by-path">
+ <maximum>
+ <total_bytes_sec>400</total_bytes_sec>
+ <total_iops_sec>401</total_iops_sec>
+ <read_bytes_sec>402</read_bytes_sec>
+ <read_iops_sec>403</read_iops_sec>
+ <write_bytes_sec>404</write_bytes_sec>
+ <write_iops_sec>405</write_iops_sec>
+ </maximum>
+ <guaranteed>
+ <total_bytes_sec>300</total_bytes_sec>
+ <total_iops_sec>301</total_iops_sec>
+ <read_bytes_sec>302</read_bytes_sec>
+ <read_iops_sec>303</read_iops_sec>
+ <write_bytes_sec>304</write_bytes_sec>
+ <write_iops_sec>305</write_iops_sec>
+ </guaranteed>
+ </device>
+ </ioTune>
+ <vcpuLimit>50</vcpuLimit>
+ </qos>
+ """)
+
+ self.maxDiff = None
+ self.assertEqual(expected_xml, self._xml_sanitizer(dom._metadata))
+
class FakeGuestAgent(object):
def getGuestInfo(self):
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 8129935..9e32874 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -935,6 +935,7 @@
%{_datadir}/%{vdsm_name}/virt/utils.py*
%{_datadir}/%{vdsm_name}/virt/vmchannels.py*
%{_datadir}/%{vdsm_name}/virt/vmstatus.py*
+%{_datadir}/%{vdsm_name}/virt/vmtune.py*
%{_datadir}/%{vdsm_name}/virt/vm.py*
%{_datadir}/%{vdsm_name}/virt/vmexitreason.py*
%{_datadir}/%{vdsm_name}/virt/vmpowerdown.py*
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index 7cd8753..47124af 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -2310,6 +2310,44 @@
'*ioTune': 'VmDiskDeviceIoTuneParams'}}
##
+# @VmDiskDeviceTuneParams:
+#
+# Extra parameters for VM disk devices. At least one
+# device identifier (name/path) is needed.
+#
+# @name: #optional The name of the taget device
+#
+# @path: #optional The path of the taget device
+#
+# @ioTune: IO tune parameters
+#
+# Since: 4.15.0
+##
+{'type': 'VmDiskDeviceTuneParams',
+ 'data': {'*name': 'str', '*path': 'str',
+ 'ioTune': 'VmDiskDeviceIoTuneParams'}}
+
+##
+# @VmDiskDeviceTuneLimits:
+#
+# Extra parameters for VM disk devices.
+#
+# @name: #optional The name of the target device
+#
+# @path: #optional The path of the target device
+#
+# @guaranteed: IO tune parameters - guaranteed values
+#
+# @maximum: IO tune parameters - the hard limits
+#
+# Since: 4.15.0
+##
+{'type': 'VmDiskDeviceTuneLimits',
+ 'data': {'*name': 'str', '*path': 'str',
+ 'guaranteed': 'VmDiskDeviceIoTuneParams',
+ 'maximum': 'VmDiskDeviceIoTuneParams'}}
+
+##
# @VmDiskDeviceVolumeChainEntry:
#
# Identifies one volume in a VM disk device volume chain.
@@ -7493,14 +7531,32 @@
'returns': 'TaskStatus'}
##
+# @UpdateVmPolicyParams:
+#
+# QoS policy update data
+#
+# @vcpuLimit: #optional vcpu limit to set - the value is a percentage
+# representation of the amount of cpu from the Host that
+# the VM can consume
+#
+# @ioTune: #optional list of ioTune limits to set
+#
+# Since: 4.15.0
+##
+{'type': 'UpdateVmPolicyParams',
+ 'data': {'*vcpuLimit': 'int',
+ '*ioTune': ['VmDiskDeviceTuneLimits']}}
+
+##
# @VM.updateVmPolicy:
#
# Set VM SLA parameters
#
# @vmID: The UUID of the VM
#
-# @vcpuLimit: vcpu limit to set - the value is a percentage representation
-# of the amount of cpu from the Host that the VM can consume
+# @params: The UpdateVmPolicyParams structure that defines
+# the changes that are to be applied to the current
+# QoS policy.
#
# Returns:
# The VM definition, as updated
@@ -7508,4 +7564,5 @@
# Since: 4.15.0
##
{'command': {'class': 'VM', 'name': 'updateVmPolicy'},
- 'data': {'vmID': 'UUID', 'vcpuLimit': 'int'}}
+ 'data': {'vmID': 'UUID', 'params': 'UpdateVmPolicyParams'},
+ 'returns': 'TaskStatus'}
diff --git a/vdsm/virt/Makefile.am b/vdsm/virt/Makefile.am
index c3262a1..be480d2 100644
--- a/vdsm/virt/Makefile.am
+++ b/vdsm/virt/Makefile.am
@@ -31,4 +31,6 @@
vmexitreason.py \
vmpowerdown.py \
vmstatus.py \
+ vmtune.py \
+ utils.py \
$(NULL)
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index bc58899..b3dd8f2 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -64,6 +64,7 @@
from . import sampling
from . import vmexitreason
from . import vmstatus
+from .vmtune import updateIoTuneDom
from .utils import XMLElement
from vmpowerdown import VmShutdown, VmReboot
@@ -3632,6 +3633,7 @@
Supported properties are:
vcpuLimit - the CPU usage hard limit
+ ioTune - the IO limits
In the case not all properties are provided, the missing properties'
setting will be left intact.
@@ -3677,6 +3679,20 @@
metadata_modified = True
del params['vcpuLimit']
+ if 'ioTune' in params:
+ # Make sure the top level element exists
+ ioTuneList = qos.getElementsByTagName("ioTune")
+ if not ioTuneList:
+ ioTuneElement = XMLElement("ioTune")
+ ioTuneList.append(ioTuneElement)
+ qos.appendChild(ioTuneElement)
+ metadata_modified = True
+
+ if updateIoTuneDom(ioTuneList[0], params["ioTune"]) > 0:
+ metadata_modified = True
+
+ del params['ioTune']
+
# Check remaining fields in params and report the list of unsupported
# params to the log
diff --git a/vdsm/virt/vmtune.py b/vdsm/virt/vmtune.py
new file mode 100644
index 0000000..06558fe
--- /dev/null
+++ b/vdsm/virt/vmtune.py
@@ -0,0 +1,225 @@
+#
+# 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 itertools
+from copy import deepcopy
+import logging
+
+from .utils import XMLElement
+
+log = logging.getLogger(__name__)
+
+
+def ioTuneValuesToDom(values, dom):
+ """
+ Create a DOM representation of the passed iotune values and
+ attach it to the dom object in the form of nodes.
+
+ :param values: Dictionary mapping iotune key to its value
+ :param dom: XML DOM object to attach the result to
+ """
+ ops = ("total", "read", "write")
+ units = ("bytes", "iops")
+
+ for op, unit in itertools.product(ops, units):
+ name = op + "_" + unit + "_sec"
+ if name in values and values[name] >= 0:
+ el = XMLElement(name)
+ el.appendTextNode(str(values[name]))
+ dom.appendChild(el)
+
+
+def collectInnerElements(el, d):
+ """
+ This helper method collects all nodes in el and adds them
+ to dictionary d.
+
+ :param el: XML DOM element object with text only children
+ :param d: Dictionary to add the values to
+ """
+ for chel in el.childNodes:
+ # skip text body
+ if chel.localName is None:
+ continue
+ try:
+ d[chel.localName] = int(chel.childNodes[0].data)
+ except (IndexError, ValueError):
+ log.exception("Invalid value for %s", chel.localName)
+
+
+def ioTuneDomToValues(dom):
+ """
+ This method converts the VmDiskDeviceTuneLimits structure from its
+ XML representation to the dictionary representation.
+
+ :param dom: XML DOM object to parse
+ :return: The structure in the dictionary form
+ """
+ values = {}
+
+ if dom.hasAttribute("name"):
+ values["name"] = dom.getAttribute("name")
+
+ if dom.hasAttribute("path"):
+ values["path"] = dom.getAttribute("path")
+
+ els = dom.getElementsByTagName("guaranteed")
+ if els:
+ values["guaranteed"] = {}
+ collectInnerElements(els[0], values["guaranteed"])
+
+ els = dom.getElementsByTagName("maximum")
+ if els:
+ values["maximum"] = {}
+ for chel in els[0].childNodes:
+ collectInnerElements(els[0], values["maximum"])
+
+ return values
+
+
+def ioTuneToDom(tune):
+ """
+ This method converts the VmDiskDeviceTuneLimits structure from the
+ dictionary representation to the XML representation.
+
+ :param tune: Dictionary representation of VmDiskDeviceTuneLimits
+ :return: DOM XML of device node filled with values
+ """
+ device = XMLElement("device")
+
+ if "name" in tune and tune["name"]:
+ device.setAttribute("name", tune["name"])
+
+ if "path" in tune and tune["path"]:
+ device.setAttribute("path", tune["path"])
+
+ if "maximum" in tune:
+ maximum = XMLElement("maximum")
+ device.appendChild(maximum)
+ ioTuneValuesToDom(tune["maximum"], maximum)
+
+ if "guaranteed" in tune:
+ guaranteed = XMLElement("guaranteed")
+ device.appendChild(guaranteed)
+ ioTuneValuesToDom(tune["guaranteed"], guaranteed)
+
+ return device
+
+
+def ioTuneMerge(old, new):
+ """
+ Merge two VmDiskDeviceTuneLimits structures in their dictionary form
+ and return the new iotune setting.
+
+ :param old: VmDiskDeviceTuneLimits in dict form
+ :param new: VmDiskDeviceTuneLimits in dict form
+ :return: old + new (in this order) in the dict form
+ """
+
+ result = deepcopy(old)
+
+ if "name" in new:
+ result["name"] = new["name"]
+
+ if "path" in new:
+ result["path"] = new["path"]
+
+ result.setdefault("maximum", {})
+ if "maximum" in new:
+ result["maximum"].update(new["maximum"])
+
+ result.setdefault("guaranteed", {})
+ if "guaranteed" in new:
+ result["guaranteed"].update(new["guaranteed"])
+
+ return result
+
+
+def createDeviceIndex(ioTune):
+ """
+ Create by name / by path dictionaries from the XML representation.
+ Returns a tuple (by_name, by_path) where the items are the respective
+ dictionaries.
+
+ :param dom: The root element (devices) to traverse
+ :return: (by_name, by_path)
+ """
+
+ ioTuneByPath = {}
+ ioTuneByName = {}
+
+ for el in ioTune.getElementsByTagName("device"):
+ # Only one of the path and name fields is mandatory
+ if el.hasAttribute("path"):
+ ioTuneByPath[el.getAttribute("path")] = el
+
+ if el.hasAttribute("name"):
+ ioTuneByName[el.getAttribute("name")] = el
+
+ return ioTuneByName, ioTuneByPath
+
+
+def updateIoTuneDom(ioTune, tunables):
+ """
+ This method takes a list of VmDiskDeviceTuneLimits objects and applies
+ the changes to the XML element representing the current iotune settings,
+
+ The return value then specifies how many devices were updated.
+
+ :param ioTune: XML object representing the ioTune metadata node
+ :param tunables: list of VmDiskDeviceTuneLimits objects
+ :return: number of updated devices
+ """
+
+ count = 0
+
+ # Get all existing ioTune records and create name/path index
+ ioTuneByName, ioTuneByPath = createDeviceIndex(ioTune)
+
+ for limit_object in tunables:
+ old_tune = None
+ if ("name" in limit_object
+ and limit_object["name"] in ioTuneByName):
+ old_tune = ioTuneByName[limit_object["name"]]
+ ioTune.removeChild(old_tune)
+ elif ("path" in limit_object
+ and limit_object["path"] in ioTuneByPath):
+ old_tune = ioTuneByPath[limit_object["path"]]
+ ioTune.removeChild(old_tune)
+
+ if old_tune is not None:
+ old_object = ioTuneDomToValues(old_tune)
+ limit_object = ioTuneMerge(old_object, limit_object)
+
+ new_tune = ioTuneToDom(limit_object)
+ ioTune.appendChild(new_tune)
+ count += 1
+
+ # Make sure everything is OK when the same name is passed
+ # twice by updating the index
+ if ("name" in limit_object
+ and limit_object["name"] in ioTuneByName):
+ ioTuneByName[limit_object["name"]] = new_tune
+
+ if ("path" in limit_object
+ and limit_object["path"] in ioTuneByPath):
+ ioTuneByPath[limit_object["path"]] = new_tune
+
+ return count
--
To view, visit http://gerrit.ovirt.org/30074
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ed108fbb2bf9d9af80577b2905242bf9f8c4221
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Hello Nir Soffer, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/30071
to review the following change.
Change subject: Refactor XMLElement to virt.utils
......................................................................
Refactor XMLElement to virt.utils
This is needed to avoid cyclic dependencies when other files
need the xml helper class.
Change-Id: I74528a597ace8148adbb0295a810ba7972b1270c
Signed-off-by: Martin Sivak <msivak(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/29630
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M vdsm.spec.in
A vdsm/virt/utils.py
M vdsm/virt/vm.py
3 files changed, 55 insertions(+), 28 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/30071/1
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 0f81316..8129935 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -932,6 +932,7 @@
%{_datadir}/%{vdsm_name}/virt/__init__.py*
%{_datadir}/%{vdsm_name}/virt/guestagent.py*
%{_datadir}/%{vdsm_name}/virt/migration.py*
+%{_datadir}/%{vdsm_name}/virt/utils.py*
%{_datadir}/%{vdsm_name}/virt/vmchannels.py*
%{_datadir}/%{vdsm_name}/virt/vmstatus.py*
%{_datadir}/%{vdsm_name}/virt/vm.py*
diff --git a/vdsm/virt/utils.py b/vdsm/virt/utils.py
new file mode 100644
index 0000000..6184acd
--- /dev/null
+++ b/vdsm/virt/utils.py
@@ -0,0 +1,53 @@
+#
+# 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
+#
+
+"""
+shared utilities and common code for the virt package
+"""
+
+import xml
+
+
+class XMLElement(object):
+
+ def __init__(self, tagName, text=None, **attrs):
+ self._elem = xml.dom.minidom.Document().createElement(tagName)
+ self.setAttrs(**attrs)
+ if text is not None:
+ self.appendTextNode(text)
+
+ def __getattr__(self, name):
+ return getattr(self._elem, name)
+
+ def setAttrs(self, **attrs):
+ for attrName, attrValue in attrs.iteritems():
+ self._elem.setAttribute(attrName, attrValue)
+
+ def appendTextNode(self, text):
+ textNode = xml.dom.minidom.Document().createTextNode(text)
+ self._elem.appendChild(textNode)
+
+ def appendChild(self, element):
+ self._elem.appendChild(element)
+
+ def appendChildWithArgs(self, childName, text=None, **attrs):
+ child = XMLElement(childName, text, **attrs)
+ self._elem.appendChild(child)
+ return child
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index d7355dd..28c4819 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -65,6 +65,7 @@
from . import vmexitreason
from . import vmstatus
+from .utils import XMLElement
from vmpowerdown import VmShutdown, VmReboot
_VMCHANNEL_DEVICE_NAME = 'com.redhat.rhevm.vdsm'
@@ -633,34 +634,6 @@
raise toe
raise
return f
-
-
-class XMLElement(object):
-
- def __init__(self, tagName, text=None, **attrs):
- self._elem = xml.dom.minidom.Document().createElement(tagName)
- self.setAttrs(**attrs)
- if text is not None:
- self.appendTextNode(text)
-
- def __getattr__(self, name):
- return getattr(self._elem, name)
-
- def setAttrs(self, **attrs):
- for attrName, attrValue in attrs.iteritems():
- self._elem.setAttribute(attrName, attrValue)
-
- def appendTextNode(self, text):
- textNode = xml.dom.minidom.Document().createTextNode(text)
- self._elem.appendChild(textNode)
-
- def appendChild(self, element):
- self._elem.appendChild(element)
-
- def appendChildWithArgs(self, childName, text=None, **attrs):
- child = XMLElement(childName, text, **attrs)
- self._elem.appendChild(child)
- return child
class _DomXML:
--
To view, visit http://gerrit.ovirt.org/30071
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I74528a597ace8148adbb0295a810ba7972b1270c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Hello Antoni Segura Puimedon, Dan Kenigsberg, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/30099
to review the following change.
Change subject: janitorial: move isVdsmImage into utils
......................................................................
janitorial: move isVdsmImage into utils
this patch moves vdsm/virt/vm.isVdsmImage into utils without code
changes. In doing so, we bootstrap the vdsm/virt/utils module,
which will contain the common shared code in the virt package,
to shed some load from vm.py.
This change is a preliminary step to the move of VmSamplingThread
from vm.py to sampling.py.
Change-Id: I9cb288eef41b567da36849e00f848e1ba20a62af
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/28477
Reviewed-by: Antoni Segura Puimedon <asegurap(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M debian/vdsm.install
M vdsm.spec.in
M vdsm/clientIF.py
M vdsm/virt/Makefile.am
A vdsm/virt/utils.py
M vdsm/virt/vm.py
6 files changed, 42 insertions(+), 14 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/30099/1
diff --git a/debian/vdsm.install b/debian/vdsm.install
index 468d398..edc3faa 100644
--- a/debian/vdsm.install
+++ b/debian/vdsm.install
@@ -148,4 +148,5 @@
./usr/share/vdsm/virt/vmexitreason.py
./usr/share/vdsm/virt/vmpowerdown.py
./usr/share/vdsm/virt/vmstatus.py
+./usr/share/vdsm/virt/utils.py
./var/lib/polkit-1/localauthority/10-vendor.d/10-vdsm-libvirt-access.pkla
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 0f81316..06b7c78 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -938,6 +938,7 @@
%{_datadir}/%{vdsm_name}/virt/vmexitreason.py*
%{_datadir}/%{vdsm_name}/virt/vmpowerdown.py*
%{_datadir}/%{vdsm_name}/virt/sampling.py*
+%{_datadir}/%{vdsm_name}/virt/utils.py*
%{_datadir}/%{vdsm_name}/tool
%config(noreplace) %{_sysconfdir}/%{vdsm_name}/vdsm.conf
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 803b19b..d5372f3 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -47,6 +47,7 @@
from virt import vmstatus
from virt.vm import Vm
from virt.vmchannels import Listener
+from virt.utils import isVdsmImage
try:
import gluster.api as gapi
_glusterEnabled = True
@@ -265,7 +266,7 @@
if type(drive) is dict:
device = drive['device']
# PDIV drive format
- if device == 'disk' and vm.isVdsmImage(drive):
+ if device == 'disk' and isVdsmImage(drive):
res = self.irs.prepareImage(
drive['domainID'], drive['poolID'],
drive['imageID'], drive['volumeID'])
@@ -352,7 +353,7 @@
def teardownVolumePath(self, drive):
res = {'status': doneCode}
try:
- if vm.isVdsmImage(drive):
+ if isVdsmImage(drive):
res = self.irs.teardownImage(drive['domainID'],
drive['poolID'], drive['imageID'])
except TypeError:
diff --git a/vdsm/virt/Makefile.am b/vdsm/virt/Makefile.am
index c3262a1..4a6ad41 100644
--- a/vdsm/virt/Makefile.am
+++ b/vdsm/virt/Makefile.am
@@ -31,4 +31,5 @@
vmexitreason.py \
vmpowerdown.py \
vmstatus.py \
+ utils.py \
$(NULL)
diff --git a/vdsm/virt/utils.py b/vdsm/virt/utils.py
new file mode 100644
index 0000000..f771606
--- /dev/null
+++ b/vdsm/virt/utils.py
@@ -0,0 +1,35 @@
+#
+# 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
+#
+
+"""
+shared utilities and common code for the virt package
+"""
+
+
+def isVdsmImage(drive):
+ """
+ Tell if drive looks like a vdsm image
+
+ :param drive: drive to check
+ :type drive: dict or vm.Drive
+ :return: bool
+ """
+ required = ('domainID', 'imageID', 'poolID', 'volumeID')
+ return all(k in drive for k in required)
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index d7355dd..2b41ab3 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -65,6 +65,7 @@
from . import vmexitreason
from . import vmstatus
+from .utils import isVdsmImage
from vmpowerdown import VmShutdown, VmReboot
_VMCHANNEL_DEVICE_NAME = 'com.redhat.rhevm.vdsm'
@@ -97,18 +98,6 @@
# A libvirt constant for undefined cpu period
_NO_CPU_PERIOD = 0
-
-
-def isVdsmImage(drive):
- """
- Tell if drive looks like a vdsm image
-
- :param drive: drive to check
- :type drive: dict or vm.Drive
- :return: bool
- """
- required = ('domainID', 'imageID', 'poolID', 'volumeID')
- return all(k in drive for k in required)
def _filterSnappableDiskDevices(diskDeviceXmlElements):
--
To view, visit http://gerrit.ovirt.org/30099
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9cb288eef41b567da36849e00f848e1ba20a62af
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>