Francesco Romani has uploaded a new change for review.
Change subject: vm: add optimizations for hyperv ......................................................................
vm: add optimizations for hyperv
This patch adds support for hyperv optimizations. The optimizations are both for stability and for performance. Engine, or any other client, can enable them by supplying a new optional parameter 'machineType' in the Vm parameters at creation time.
As default, the new settings are disabled for backward compatibility.
The parameters are hardcoded and not externally configurable because they are not supposed to be changed very often, if changed at all; moreover, this patch already implements the optimal recommended settings.
Change-Id: I28ea1d5adeda07798255484209e1a1d92c2c2bc5 Bug-Url: https://bugzilla.redhat.com/1083529 Signed-off-by: Francesco Romani fromani@redhat.com --- M tests/vmTests.py M vdsm/virt/vm.py M vdsm_api/vdsmapi-schema.json 3 files changed, 70 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/19/27619/1
diff --git a/tests/vmTests.py b/tests/vmTests.py index 1dd75ae..b64f041 100644 --- a/tests/vmTests.py +++ b/tests/vmTests.py @@ -264,6 +264,23 @@ domxml.appendFeatures() self.assertXML(domxml.dom, featuresXML, 'features')
+ def testFeaturesHyperVXML(self): + featuresXML = """ + <features> + <acpi/> + <hyperv> + <relaxed state="on"/> + <vapic state="on"/> + <spinlocks retries="8191" state="on"/> + </hyperv> + </features>""" + conf = {'machineType': 'hyperv'} + conf.update(self.conf) + domxml = vm._DomXML(conf, self.log, + caps.Architecture.X86_64) + domxml.appendFeatures() + self.assertXML(domxml.dom, featuresXML, 'features') + def testSysinfoXML(self): sysinfoXML = """ <sysinfo type="smbios"> diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index f1c36f8..5f6b557 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -647,11 +647,37 @@ <features> <acpi/> <features/> + + for hyperv: + <features> + <acpi/> + <hyperv> + <relaxed state='on'/> + <vapic state='on'/> + <spinlocks state='on' retries='8191'/> + </hyperv> + <features/> """
- if utils.tobool(self.conf.get('acpiEnable', 'true')): + if (utils.tobool(self.conf.get('acpiEnable', 'true')) or + self.conf.get('machineType') == MachineType.HYPERV): features = self.dom.appendChildWithArgs('features') + + if utils.tobool(self.conf.get('acpiEnable', 'true')): features.appendChildWithArgs('acpi') + + if self.conf.get('machineType') == MachineType.HYPERV: + hyperv = XMLElement('hyperv') + features.appendChild(hyperv) + + hyperv.appendChildWithArgs('relaxed', state='on') + # turns off an internal Windows watchdog, and by doing so avoids + # some high load BSODs. + hyperv.appendChildWithArgs('vapic', state='on') + hyperv.appendChildWithArgs( + 'spinlocks', state='on', retries='8191') + # performance optimization flags, that can improve the performance + # by 10% to much more (in extreme cases of resources overcommit).
def appendCpu(self): """ @@ -1526,6 +1552,10 @@ pass
+class MachineType: + HYPERV = "hyperv" + + class Vm(object): """ Used for abstracting communication between various parts of the diff --git a/vdsm_api/vdsmapi-schema.json b/vdsm_api/vdsmapi-schema.json index d134174..86883ff 100644 --- a/vdsm_api/vdsmapi-schema.json +++ b/vdsm_api/vdsmapi-schema.json @@ -898,6 +898,18 @@ {'enum': 'VmType', 'data': ['kvm']}
## +# @MachineType: +# +# Enumeration of supported machine types, with +# predefined optimized settings. +# +# @hyperv: Machine optimized for hyperv +# +# Since: 4.15.0 +## +{'enum': 'MachineType', 'data': ['hyperv']} + +## # @NetInfoNetworkMap: # # A mapping of vdsm Network information indexed by network name. @@ -3131,6 +3143,9 @@ # # @kvmEnable: Indicates if KVM hardware acceleration is enabled # +# @machineType: #optional Indicates the machine profile for the VM. +# (new in version 4.15.0) +# # @maxVCpus: #optional Maximum number of CPU available for the guest # It is the upper boundry for hot plug CPU action # @@ -3180,7 +3195,8 @@ 'display': 'VmDisplayType', 'displayIp': 'str', 'displayPort': 'int', 'displaySecurePort': 'int', '*emulatedMachine': 'str', '*keyboardLayout': 'str', - 'kvmEnable': 'bool', '*maxVCpus': 'uint', 'memSize': 'uint', + 'kvmEnable': 'bool', '*machineType': 'MachineType', + '*maxVCpus': 'uint', 'memSize': 'uint', 'memGuaranteedSize': 'uint', 'nicModel': 'str', 'nice': 'int', '*pauseCode': 'str', 'pid': 'uint', 'smp': 'uint', '*smpCoresPerSocket': 'uint', '*smpThreadsPerCore': 'uint', 'status': 'VmStatus', @@ -3209,6 +3225,9 @@ # @display: The type of display # # @kvmEnable: Indicates if KVM hardware acceleration is enabled +# +# @machineType: #optional Indicates the machine profile for the VM. +# (new in version 4.15.0) # # @memSize: The amount of memory assigned to the VM in MB # @@ -3242,7 +3261,8 @@ {'type': 'VmParameters', 'data': {'acpiEnable': 'bool', '*bootMenuEnable': 'bool', '*cpuShares': 'str', '*custom': 'StringMap', '*devices': ['VmDevice'], - 'display': 'VmDisplayType', 'kvmEnable': 'bool', 'memSize': 'uint', + 'display': 'VmDisplayType', 'kvmEnable': 'bool', + '*machineType': 'MachineType', 'memSize': 'uint', 'nice': 'int', 'smp': 'uint', '*smpCoresPerSocket': 'uint', '*smpThreadsPerCore': 'uint', 'timeOffset': 'uint', 'transparentHugePages': 'bool', 'vmId': 'UUID', 'vmName': 'str',
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8778/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8914/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7988/ : SUCCESS
Roy Golan has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.ovirt.org/#/c/27619/1/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 671: features.appendChild(hyperv) Line 672: Line 673: hyperv.appendChildWithArgs('relaxed', state='on') Line 674: # turns off an internal Windows watchdog, and by doing so avoids Line 675: # some high load BSODs. :) funny that BSODs is still around Line 676: hyperv.appendChildWithArgs('vapic', state='on') Line 677: hyperv.appendChildWithArgs( Line 678: 'spinlocks', state='on', retries='8191') Line 679: # performance optimization flags, that can improve the performance
Michal Skrivanek has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 1:
do we know exact host versions where all those are supported?
Francesco Romani has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 1:
Will add dependency requirements
Francesco Romani has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 1:
Apparently these options were introduced first in libvirt 1.1.0 (I'd have a preciated a reference to that in the libvirt docs. however). RHEL 6.5 has partial support (only to the 'relaxed' attribute)
Francesco Romani has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 1: Code-Review-1
-1 because for u/s libvirt the dependencies requirements are simple enough, but d/s RHEL requirements are not yet clear.
Francesco Romani has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 2:
changes: implemented the clocksource fix.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 2: Code-Review-1 Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9614/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8682/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9468/ : UNSTABLE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 3:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9615/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8683/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9469/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 4:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9416/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10200/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10356/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5282/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3440/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/916/ : SUCCESS
Michal Skrivanek has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 4:
we may need to differentiate based on VM's machine type as there is only a partial support in EL 6.5 machine type (in addition to cluster level, which we assume engine will take care of)
Dan Kenigsberg has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 4:
(1 comment)
http://gerrit.ovirt.org/#/c/27619/4/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 1780: class MissingLibvirtDomainError(Exception): Line 1781: pass Line 1782: Line 1783: Line 1784: class MachineType: When there's "type" with only one element, bells of over-engineering ring. Everywhere else in VM creation, the notion of "instance type" is kept at Engine. Only specific knobs are available to Vdsm.
I suggest to keep this here, and expose a simple enableHyperV boolean. Line 1785: HYPERV = "hyperv" Line 1786: Line 1787: Line 1788: class Vm(object):
Francesco Romani has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 5: Code-Review-1
dropped the machineType setting and implemented a much simpler 'hypervEnable' boolean.
self -1 because not sure yet how to deal with partial RHEL6.5 support.
Francesco Romani has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 5:
(1 comment)
explain the reason for self inflicted -1
http://gerrit.ovirt.org/#/c/27619/5/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 716: """ Line 717: Line 718: if (self.arch == caps.Architecture.X86_64 and Line 719: utils.tobool(self.conf.get('hypervEnable', 'false'))): Line 720: clockName = 'hypervclock' this is not available until libvirt 1.2.2. - and it is not in RHEL 6.5. Probably the best way forward is to add another boolean just for clock. Line 721: else: Line 722: clockName = 'rtc' Line 723: Line 724: m = XMLElement('clock', offset='variable',
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: add optimizations for hyperv ......................................................................
Patch Set 5:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9641/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10426/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10583/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5507/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3665/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1009/ : There was an infra issue, please contact infra@ovirt.org
Michal Skrivanek has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 5:
(1 comment)
http://gerrit.ovirt.org/#/c/27619/5/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 716: """ Line 717: Line 718: if (self.arch == caps.Architecture.X86_64 and Line 719: utils.tobool(self.conf.get('hypervEnable', 'false'))): Line 720: clockName = 'hypervclock'
this is not available until libvirt 1.2.2. - and it is not in RHEL 6.5. Pro
can't we check the version instead? The complexity of various level of support of hyperv can be done inside vdsm Line 721: else: Line 722: clockName = 'rtc' Line 723: Line 724: m = XMLElement('clock', offset='variable',
Francesco Romani has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 6: Verified+1
Due to troublesome hyperv enlightenment support, I split the original patch in a more granular series.
The problems are: - all these hyperv enhancements are passed in through XML; thus, there is no easy way to tell in advance which version support what, unless, of course, put constraint on libvirt version, but: - RHEL6 only partially supports hyperv features and - RHEL6 libvirt, surprisingly enough, bails out with error if receives an unsupported hyperv enlightenment feature instead of silently ignore it as I would expect.
Verification: * on RHEL6.5 (the more hostile platform), forced hypervEnable=true * verified the produced XML is compliant * VM booted OK
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9655/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10440/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1010/ : There was an infra issue, please contact infra@ovirt.org
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10597/ : SUCCESS
Michal Skrivanek has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 6:
(1 comment)
http://gerrit.ovirt.org/#/c/27619/6/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 705: Line 706: self._devices = XMLElement('devices') Line 707: self.dom.appendChild(self._devices) Line 708: Line 709: def _isHyperV(self): do we need the logic regarding arch here? why not rely on engine? (and get rid of this extra function) Line 710: return (self.arch == caps.Architecture.X86_64 and Line 711: utils.tobool(self.conf.get('hypervEnable', 'false'))) Line 712: Line 713: def appendClock(self):
Francesco Romani has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 6:
(1 comment)
http://gerrit.ovirt.org/#/c/27619/6/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 705: Line 706: self._devices = XMLElement('devices') Line 707: self.dom.appendChild(self._devices) Line 708: Line 709: def _isHyperV(self):
do we need the logic regarding arch here? why not rely on engine? (and get
I just want to make sure to not break PPC64. If we choose to fully trust engine and do not add safety nets I'll change the code in near zero time. Line 710: return (self.arch == caps.Architecture.X86_64 and Line 711: utils.tobool(self.conf.get('hypervEnable', 'false'))) Line 712: Line 713: def appendClock(self):
Michal Skrivanek has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 6:
(1 comment)
http://gerrit.ovirt.org/#/c/27619/6/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 705: Line 706: self._devices = XMLElement('devices') Line 707: self.dom.appendChild(self._devices) Line 708: Line 709: def _isHyperV(self):
I just want to make sure to not break PPC64.
well, I'd prefer not to spread arch checks like plague and try to trust engine. We believe it is saying the right thing for all other devices, etc., and as long as the vdsm doesn't break and the outcome is only a failure in VM creation…. Line 710: return (self.arch == caps.Architecture.X86_64 and Line 711: utils.tobool(self.conf.get('hypervEnable', 'false'))) Line 712: Line 713: def appendClock(self):
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 7:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9658/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10443/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1013/ : There was an infra issue, please contact infra@ovirt.org
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10600/ : SUCCESS
Francesco Romani has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 6:
(1 comment)
http://gerrit.ovirt.org/#/c/27619/6/vdsm/virt/vm.py File vdsm/virt/vm.py:
Line 705: Line 706: self._devices = XMLElement('devices') Line 707: self.dom.appendChild(self._devices) Line 708: Line 709: def _isHyperV(self):
well, I'd prefer not to spread arch checks like plague and try to trust eng
Will drop the architecture check and the helper method. Line 710: return (self.arch == caps.Architecture.X86_64 and Line 711: utils.tobool(self.conf.get('hypervEnable', 'false'))) Line 712: Line 713: def appendClock(self):
Francesco Romani has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 8: Verified+1
removed arch check and utility method. Copied score because I think the verification is still valid.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 8:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9661/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10445/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1015/ : There was an infra issue, please contact infra@ovirt.org
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/10602/ : SUCCESS
Vinzenz Feenstra has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 8: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 8: Code-Review+2
Convinced.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
vm: hyperv: initial windows hyperv support
This patch adds explicit support for hyperv optimizations. The optimizations are both for stability and for performance. Engine, or any other client, can enable them by supplying a new optional parameter 'hyperVenable' in the Vm parameters at creation time.
As default, the new settings are disabled for backward compatibility.
The parameters are hardcoded and not externally configurable because they are not supposed to be changed very often, if changed at all; moreover, we implement the optimal recommended settings.
Change-Id: I28ea1d5adeda07798255484209e1a1d92c2c2bc5 Bug-Url: https://bugzilla.redhat.com/1110305 Signed-off-by: Francesco Romani fromani@redhat.com Reviewed-on: http://gerrit.ovirt.org/27619 Reviewed-by: Vinzenz Feenstra vfeenstr@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M tests/vmTests.py M vdsm/rpc/vdsmapi-schema.json M vdsm/virt/vm.py 3 files changed, 46 insertions(+), 4 deletions(-)
Approvals: Vinzenz Feenstra: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: hyperv: initial windows hyperv support ......................................................................
Patch Set 9:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1534/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5519/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3677/ : SUCCESS
vdsm-patches@lists.fedorahosted.org