Dan Kenigsberg has uploaded a new change for review.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
faqemu: move hook logic out of vdsm
Vdsm-proper has been aware of the faqemu hook. Now that we have an after_get_caps hook point, this is no longer necessary. This patch move the faking logic, so that it does not need to be shipped on production systems and clutter the code.
Change-Id: I46d0079491f707b0abd3fbbe2d47f63697dac9c5 Signed-off-by: Dan Kenigsberg danken@redhat.com --- M vdsm.spec.in M vdsm/caps.py M vdsm_hooks/faqemu/Makefile.am A vdsm_hooks/faqemu/after_get_caps.py 4 files changed, 110 insertions(+), 42 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/05/27705/1
diff --git a/vdsm.spec.in b/vdsm.spec.in index c620174..e6db372 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -1464,6 +1464,7 @@ %defattr(-, root, root, -) %doc COPYING %{_libexecdir}/%{vdsm_name}/hooks/before_vm_start/10_faqemu +%{_libexecdir}/%{vdsm_name}/hooks/after_get_caps/10_faqemu
%if 0%{?with_gluster} %files gluster diff --git a/vdsm/caps.py b/vdsm/caps.py index 46094f3..7c1e520 100644 --- a/vdsm/caps.py +++ b/vdsm/caps.py @@ -1,5 +1,5 @@ # -# Copyright 2011 Red Hat, Inc. +# Copyright 2011-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 @@ -197,7 +197,7 @@
@utils.memoized -def _getLiveSnapshotSupport(arch, capabilities=None): +def getLiveSnapshotSupport(arch, capabilities=None): if capabilities is None: capabilities = _getCapsXMLStr() caps = minidom.parseString(capabilities) @@ -295,7 +295,7 @@
@utils.memoized -def _getEmulatedMachines(arch, capabilities=None): +def getEmulatedMachines(arch, capabilities=None): if capabilities is None: capabilities = _getCapsXMLStr() caps = minidom.parseString(capabilities) @@ -444,13 +444,6 @@ return dict(release=release, version=version, name=osname)
-def getTargetArch(): - if config.getboolean('vars', 'fake_kvm_support'): - return config.get('vars', 'fake_kvm_architecture') - else: - return platform.machine() - - def _getSELinuxEnforceMode(): """ Returns the SELinux mode as reported by kernel. @@ -480,13 +473,11 @@
def get(): - targetArch = getTargetArch() + targetArch = platform.machine()
caps = {}
- caps['kvmEnabled'] = \ - str(config.getboolean('vars', 'fake_kvm_support') or - os.path.exists('/dev/kvm')).lower() + caps['kvmEnabled'] = str(os.path.exists('/dev/kvm')).lower()
cpuInfo = CpuInfo() cpuTopology = CpuTopology() @@ -498,30 +489,9 @@ caps['cpuThreads'] = str(cpuTopology.threads()) caps['cpuSockets'] = str(cpuTopology.sockets()) caps['cpuSpeed'] = cpuInfo.mhz() - if config.getboolean('vars', 'fake_kvm_support'): - if targetArch == Architecture.X86_64: - caps['cpuModel'] = 'Intel(Fake) CPU' - - flagList = ['vmx', 'sse2', 'nx'] - - if targetArch == platform.machine(): - flagList += cpuInfo.flags() - - flags = set(flagList) - - caps['cpuFlags'] = ','.join(flags) + ',model_486,model_pentium,' \ - 'model_pentium2,model_pentium3,model_pentiumpro,' \ - 'model_qemu32,model_coreduo,model_core2duo,model_n270,' \ - 'model_Conroe,model_Penryn,model_Nehalem,model_Opteron_G1' - elif targetArch == Architecture.PPC64: - caps['cpuModel'] = 'POWER 7 (fake)' - caps['cpuFlags'] = 'powernv,model_POWER7_v2.3' - else: - raise RuntimeError('Unsupported architecture: %s' % targetArch) - else: - caps['cpuModel'] = cpuInfo.model() - caps['cpuFlags'] = ','.join(cpuInfo.flags() + - _getCompatibleCpuModels()) + caps['cpuModel'] = cpuInfo.model() + caps['cpuFlags'] = ','.join(cpuInfo.flags() + + _getCompatibleCpuModels())
caps.update(_getVersionInfo()) caps.update(netinfo.get()) @@ -534,7 +504,7 @@ caps['operatingSystem'] = osversion() caps['uuid'] = utils.getHostUUID() caps['packages2'] = _getKeyPackages() - caps['emulatedMachines'] = _getEmulatedMachines(targetArch) + caps['emulatedMachines'] = getEmulatedMachines(targetArch) caps['ISCSIInitiatorName'] = _getIscsiIniName() caps['HBAInventory'] = storage.hba.HBAInventory() caps['vmTypes'] = ['kvm'] @@ -550,7 +520,7 @@
caps['selinux'] = _getSELinux()
- liveSnapSupported = _getLiveSnapshotSupport(targetArch) + liveSnapSupported = getLiveSnapshotSupport(targetArch) if liveSnapSupported is not None: caps['liveSnapshot'] = str(liveSnapSupported).lower()
diff --git a/vdsm_hooks/faqemu/Makefile.am b/vdsm_hooks/faqemu/Makefile.am index 3e1b0c0..e93c943 100644 --- a/vdsm_hooks/faqemu/Makefile.am +++ b/vdsm_hooks/faqemu/Makefile.am @@ -1,5 +1,5 @@ # -# Copyright 2011-2012 Red Hat, Inc. +# Copyright 2011-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 @@ -24,12 +24,18 @@ config.log
EXTRA_DIST = \ - before_vm_start.py + before_vm_start.py \ + after_get_caps.py \ + $(NULL)
install-data-local: $(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_vm_start + $(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/after_get_caps $(INSTALL_SCRIPT) $(srcdir)/before_vm_start.py \ $(DESTDIR)$(vdsmhooksdir)/before_vm_start/10_faqemu + $(INSTALL_SCRIPT) $(srcdir)/after_get_caps.py \ + $(DESTDIR)$(vdsmhooksdir)/after_get_caps/10_faqemu
uninstall-local: $(RM) $(DESTDIR)$(vdsmhooksdir)/before_vm_start/10_faqemu + $(RM) $(DESTDIR)$(vdsmhooksdir)/after_get_caps/10_faqemu diff --git a/vdsm_hooks/faqemu/after_get_caps.py b/vdsm_hooks/faqemu/after_get_caps.py new file mode 100644 index 0000000..a4a008b --- /dev/null +++ b/vdsm_hooks/faqemu/after_get_caps.py @@ -0,0 +1,91 @@ +#!/usr/bin/python +# +# 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 platform +import sys +import traceback + +# using vdsm internals in a hook is dangerous and frowned upon, +# as internal APIs may change without notice. +# However, faqemu is a bit different, as it is not expected to be ever used in +# production. +sys.path.append('/usr/share/vdsm') +import caps +import hooking +from vdsm.config import config + + +def fake_caps(c, target_arch): + c['kvmEnabled'] = 'True' + cpu_info = caps.CpuInfo() + if target_arch == caps.Architecture.X86_64: + c['cpuModel'] = 'Intel(Fake) CPU' + + flagList = ['vmx', 'sse2', 'nx'] + + if target_arch == platform.machine(): + flagList += cpu_info.flags() + + flags = set(flagList) + + c['cpuFlags'] = ','.join(flags) + ',model_486,model_pentium,' \ + 'model_pentium2,model_pentium3,model_pentiumpro,' \ + 'model_qemu32,model_coreduo,model_core2duo,model_n270,' \ + 'model_Conroe,model_Penryn,model_Nehalem,model_Opteron_G1' + elif target_arch == caps.Architecture.PPC64: + c['cpuModel'] = 'POWER 7 (fake)' + c['cpuFlags'] = 'powernv,model_POWER7_v2.3' + else: + raise RuntimeError('Unsupported architecture: %s' % target_arch) + + if target_arch != platform.machine(): + c['emulatedMachines'] = caps.getEmulatedMachines(target_arch) + + liveSnapSupported = caps.getLiveSnapshotSupport(target_arch) + if liveSnapSupported is not None: + caps['liveSnapshot'] = str(liveSnapSupported).lower() + + return c + + +def main(): + if config.getboolean('vars', 'fake_kvm_support'): + try: + c = hooking.read_json() + target_arch = config.get('vars', 'fake_kvm_architecture') + c = fake_caps(c, target_arch) + hooking.write_json(c) + except: + hooking.exit_hook('faqemu: %s\n' + % traceback.format_exc()) + sys.exit(2) + + +def test(): + from pprint import pprint + pprint(fake_caps({}, 'ppc64')) + + +if __name__ == '__main__': + if '--test' in sys.argv: + test() + else: + main()
oVirt Jenkins CI Server has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8847/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8983/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8057/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/598/ : SUCCESS
Francesco Romani has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 1: Code-Review-1
(2 comments)
I'm concerned about the proper way to get the vdsm root (what is usually, but not necessarily, /usr/share/vdsm). -1 for visibility.
http://gerrit.ovirt.org/#/c/27705/1/vdsm_hooks/faqemu/after_get_caps.py File vdsm_hooks/faqemu/after_get_caps.py:
Line 26: # using vdsm internals in a hook is dangerous and frowned upon, Line 27: # as internal APIs may change without notice. Line 28: # However, faqemu is a bit different, as it is not expected to be ever used in Line 29: # production. Line 30: sys.path.append('/usr/share/vdsm') I think this should be filled by make somehow. Or maybe expose the vdsm root from hooking? Line 31: import caps Line 32: import hooking Line 33: from vdsm.config import config Line 34:
Line 32: import hooking Line 33: from vdsm.config import config Line 34: Line 35: Line 36: def fake_caps(c, target_arch): a single-letter identifier is probably too terse :) Line 37: c['kvmEnabled'] = 'True' Line 38: cpu_info = caps.CpuInfo() Line 39: if target_arch == caps.Architecture.X86_64: Line 40: c['cpuModel'] = 'Intel(Fake) CPU'
Antoni Segura Puimedon has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 1: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/27705/1//COMMIT_MSG Commit Message:
Line 6: Line 7: faqemu: move hook logic out of vdsm Line 8: Line 9: Vdsm-proper has been aware of the faqemu hook. Now that we have an Line 10: after_get_caps hook point, this is no longer necessary. This patch move s/move/moves/ Line 11: the faking logic, so that it does not need to be shipped on production Line 12: systems and clutter the code. Line 13: Line 14: Change-Id: I46d0079491f707b0abd3fbbe2d47f63697dac9c5
http://gerrit.ovirt.org/#/c/27705/1/vdsm/caps.py File vdsm/caps.py:
Line 294: return AutoNumaBalancingStatus.UNKNOWN Line 295: Line 296: Line 297: @utils.memoized Line 298: def getEmulatedMachines(arch, capabilities=None): A more picky man would say that these two name changes are not material to this changeset :P. It's fine and more consistent though. Line 299: if capabilities is None: Line 300: capabilities = _getCapsXMLStr() Line 301: caps = minidom.parseString(capabilities) Line 302:
http://gerrit.ovirt.org/#/c/27705/1/vdsm_hooks/faqemu/after_get_caps.py File vdsm_hooks/faqemu/after_get_caps.py:
Line 26: # using vdsm internals in a hook is dangerous and frowned upon, Line 27: # as internal APIs may change without notice. Line 28: # However, faqemu is a bit different, as it is not expected to be ever used in Line 29: # production. Line 30: sys.path.append('/usr/share/vdsm')
I think this should be filled by make somehow.
It should be exposed through hooking. As in:
from hooking import caps Line 31: import caps Line 32: import hooking Line 33: from vdsm.config import config Line 34:
Vinzenz Feenstra has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/27705/1/vdsm_hooks/faqemu/after_get_caps.py File vdsm_hooks/faqemu/after_get_caps.py:
Line 26: # using vdsm internals in a hook is dangerous and frowned upon, Line 27: # as internal APIs may change without notice. Line 28: # However, faqemu is a bit different, as it is not expected to be ever used in Line 29: # production. Line 30: sys.path.append('/usr/share/vdsm')
It should be exposed through hooking. As in:
even better would be if we could move it into the vdsm pkg, then no such hack would be required. Line 31: import caps Line 32: import hooking Line 33: from vdsm.config import config Line 34:
Dan Kenigsberg has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 1:
(2 comments)
http://gerrit.ovirt.org/#/c/27705/1/vdsm/caps.py File vdsm/caps.py:
Line 294: return AutoNumaBalancingStatus.UNKNOWN Line 295: Line 296: Line 297: @utils.memoized Line 298: def getEmulatedMachines(arch, capabilities=None):
A more picky man would say that these two name changes are not material to
The only motivation of making this public, is the hook. If the hook is rejected, I'd like to keep this function private. Hence it belongs to this commit. Line 299: if capabilities is None: Line 300: capabilities = _getCapsXMLStr() Line 301: caps = minidom.parseString(capabilities) Line 302:
http://gerrit.ovirt.org/#/c/27705/1/vdsm_hooks/faqemu/after_get_caps.py File vdsm_hooks/faqemu/after_get_caps.py:
Line 26: # using vdsm internals in a hook is dangerous and frowned upon, Line 27: # as internal APIs may change without notice. Line 28: # However, faqemu is a bit different, as it is not expected to be ever used in Line 29: # production. Line 30: sys.path.append('/usr/share/vdsm')
even better would be if we could move it into the vdsm pkg, then no such ha
But I do not WANT to make vdsm internal available to hooks. It's dangerous, since if a hook is deployed and vdsm upgrades, there is no promise that the internals would be maintained.
I explained in my comment why I think that the case of faqemu is different. And in any case, I must add a requirement for the specific vdsm version.
If you think that "Special cases aren't special enough to break the rules", we'll have to think about a different approach (I do not know which it would be). Line 31: import caps Line 32: import hooking Line 33: from vdsm.config import config Line 34:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 2: Code-Review-1 Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8898/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9034/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8108/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/601/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 3:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9133/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9276/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8345/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/624/ : SUCCESS
Francesco Romani has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
Looks good except for the hardcoded path, which I still find worrysome. -1 for visibility.
http://gerrit.ovirt.org/#/c/27705/3/vdsm_hooks/faqemu/after_get_caps.py File vdsm_hooks/faqemu/after_get_caps.py:
Line 26: # using vdsm internals in a hook is dangerous and frowned upon, Line 27: # as internal APIs may change without notice. Line 28: # However, faqemu is a bit different, as it is not expected to be ever used in Line 29: # production. Line 30: sys.path.append('/usr/share/vdsm') I'm still not convinced about this hardcoded path. Won't this break as soon as anyone uses something different than --prefix=/usr in ./configure? Line 31: import caps Line 32: import hooking Line 33: from vdsm.config import config Line 34:
Vinzenz Feenstra has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 3: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/27705/3/vdsm_hooks/faqemu/after_get_caps.py File vdsm_hooks/faqemu/after_get_caps.py:
Line 26: # using vdsm internals in a hook is dangerous and frowned upon, Line 27: # as internal APIs may change without notice. Line 28: # However, faqemu is a bit different, as it is not expected to be ever used in Line 29: # production. Line 30: sys.path.append('/usr/share/vdsm')
I'm still not convinced about this hardcoded path.
This should be a @@ variable and this file should be a .in file. Line 31: import caps Line 32: import hooking Line 33: from vdsm.config import config Line 34:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 4:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9700/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8767/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/678/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9553/ : SUCCESS
Antoni Segura Puimedon has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 4: Code-Review-1
(1 comment)
Only a very minor nit. Otherwise looks good.
http://gerrit.ovirt.org/#/c/27705/4/vdsm_hooks/faqemu/after_get_caps.py File vdsm_hooks/faqemu/after_get_caps.py:
Line 1: #!/usr/bin/python /usr/bin/env python2 Line 2: # Line 3: # Copyright 2014 Red Hat, Inc. Line 4: # Line 5: # This program is free software; you can redistribute it and/or modify
Itamar Heim has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 4:
ping
Dan Kenigsberg has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 4: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/27705/4/vdsm/caps.py File vdsm/caps.py:
Line 443 Line 444 Line 445 Line 446 Line 447 this is used by vm.py, and cannot be dropped out of hand.
Jenkins CI RO has abandoned this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Abandoned
Abandoned due to no activity - please restore if still relevant
Jenkins CI RO has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 4:
Abandoned due to no activity - please restore if still relevant
automation@ovirt.org has posted comments on this change.
Change subject: faqemu: move hook logic out of vdsm ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found
vdsm-patches@lists.fedorahosted.org