vdsm-patches October 2016

vdsm-patches@lists.fedorahosted.org

33 participants
468 discussions

Change in vdsm[master]: migration: use system thread names
by fromani＠redhat.com 03 Nov '16

03 Nov '16

Francesco Romani has uploaded a new change for review. Change subject: migration: use system thread names ...................................................................... migration: use system thread names Add migration thread names. Unfortunately, we need to shorten 'migration' - which is going to eat 10 out of 15 ASCII chars available for the name. https://bugzilla.redhat.com/1141422 Change-Id: Ie03f19f75cb9430cd3a267817df269df776fa6a3 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M vdsm/virt/migration.py 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/65503/1 diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index ea43174..8a254c8 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -125,7 +125,7 @@ 'code': 0, 'message': 'Migration in progress'}} self._progress = 0 - self._thread = concurrent.thread(self.run) + self._thread = concurrent.thread(self.run, name='migr/source') self._preparingMigrationEvt = True self._migrationCanceledEvt = threading.Event() self._monitorThread = None @@ -556,7 +556,7 @@ # we need the first value to support set_initial_downtime self._initial_downtime = next(self._downtimes) - self._thread = concurrent.thread(self.run) + self._thread = concurrent.thread(self.run, name='migr/downtime') def start(self): self._thread.start() @@ -622,7 +622,7 @@ self._conv_schedule = conv_schedule self._use_conv_schedule = use_conv_schedule self.downtime_thread = _FakeThreadInterface() - self._thread = concurrent.thread(self.run) + self._thread = concurrent.thread(self.run, name='migr/monitor') self._log = vm.get_logger(self._raw_log) def start(self): -- To view, visit https://gerrit.ovirt.org/65503 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie03f19f75cb9430cd3a267817df269df776fa6a3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

4 8

Change in vdsm[master]: tests: Ensure nose 1.3.7 is used for tests
by Nir Soffer 02 Nov '16

02 Nov '16

Nir Soffer has uploaded a new change for review. Change subject: tests: Ensure nose 1.3.7 is used for tests ...................................................................... tests: Ensure nose 1.3.7 is used for tests Run the tests via tox, installing the correct version of nose in a virtual environment. Since correct version is nose is always available, the warning about nose version is not needed now. Change-Id: I245a171940a5e869fd719a6410024ee77e8ad86c Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M Makefile.am M automation/check-patch.sh M tox.ini 3 files changed, 9 insertions(+), 8 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/65404/1 diff --git a/Makefile.am b/Makefile.am index 354c6b3..3b3e11c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -131,12 +131,7 @@ .PHONY: tests tests: - @version=`nosetests --version | cut -d' ' -f3`; \ - if build-aux/vercmp $$version "1.3.7"; then \ - echo "Error: NOSE is too old, please install NOSE 1.3.7 or later"; \ - exit 1; \ - fi - @$(MAKE) -C tests/ check + tox -e tests all-local: \ vdsm.spec diff --git a/automation/check-patch.sh b/automation/check-patch.sh index 3284e2b..0877046 100755 --- a/automation/check-patch.sh +++ b/automation/check-patch.sh @@ -8,7 +8,7 @@ export VDSM_AUTOMATION=1 easy_install pip -pip install -U nose==1.3.7 tox==2.1.1 +pip install -U tox==2.1.1 ./autogen.sh --system --enable-hooks --enable-vhostmd diff --git a/tox.ini b/tox.ini index 8686eb2..fb86cb7 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,13 @@ [tox] -envlist = flake8 +envlist = flake8,tests skipsdist = true +[testenv:tests] +deps = + nose==1.3.7 +commands = + make -C tests check + [testenv:flake8] deps = flake8 -- To view, visit https://gerrit.ovirt.org/65404 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I245a171940a5e869fd719a6410024ee77e8ad86c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

8 22

Change in vdsm[ovirt-4.0]: net: remove default route before adding a new one
by phoracek＠redhat.com 02 Nov '16

02 Nov '16

Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/65056 to review the following change. Change subject: net: remove default route before adding a new one ...................................................................... net: remove default route before adding a new one Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd Signed-off-by: Petr Horáček <phoracek(a)redhat.com> Reviewed-on: https://gerrit.ovirt.org/63220 Continuous-Integration: Jenkins CI Reviewed-by: Edward Haas <edwardh(a)redhat.com> Reviewed-by: Dan Kenigsberg <danken(a)redhat.com> --- M lib/vdsm/network/ip/address.py 1 file changed, 22 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/65056/1 diff --git a/lib/vdsm/network/ip/address.py b/lib/vdsm/network/ip/address.py index 634925a..04aec93 100644 --- a/lib/vdsm/network/ip/address.py +++ b/lib/vdsm/network/ip/address.py @@ -18,6 +18,8 @@ # from __future__ import absolute_import +from contextlib import contextmanager +import logging import socket import struct @@ -173,28 +175,42 @@ def add(iface, ipv4, ipv6): if ipv4.address: - ipwrapper.addrAdd(iface, ipv4.address, ipv4.netmask) - if ipv4.gateway and ipv4.defaultRoute: - ipwrapper.routeAdd(['default', 'via', ipv4.gateway]) + _add_ipv4_address(iface, ipv4) if ipv6: _add_ipv6_address(iface, ipv6) elif ipv6_supported(): sysctl.disable_ipv6(iface) +def _add_ipv4_address(iface, ipv4): + ipwrapper.addrAdd(iface, ipv4.address, ipv4.netmask) + if ipv4.gateway and ipv4.defaultRoute: + _set_default_route(ipv4.gateway, family=4) + + def _add_ipv6_address(iface, ipv6): if ipv6.address: ipv6addr, ipv6netmask = ipv6.address.split('/') ipwrapper.addrAdd(iface, ipv6addr, ipv6netmask, family=6) - if ipv6.gateway: - ipwrapper.routeAdd(['default', 'via', ipv6.gateway], - dev=iface, family=6) + if ipv6.gateway and ipv6.defaultRoute: + _set_default_route(ipv6.gateway, family=6, dev=iface) if ipv6.ipv6autoconf is not None: with open('/proc/sys/net/ipv6/conf/%s/autoconf' % iface, 'w') as ipv6_autoconf: ipv6_autoconf.write('1' if ipv6.ipv6autoconf else '0') +@contextmanager +def _set_default_route(gateway, family, dev=None): + try: + ipwrapper.routeAdd(['default', 'via', gateway], family=family, dev=dev) + except ipwrapper.IPRoute2Error: # there already is a default route + logging.warning( + 'Existing default route will be removed so a new one can be set.') + ipwrapper.routeDel('default', family=family) + ipwrapper.routeAdd(['default', 'via', gateway], family=family, dev=dev) + + def flush(iface, family='both'): ipwrapper.addrFlush(iface, family) -- To view, visit https://gerrit.ovirt.org/65056 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-4.0 Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>

4 7

Change in vdsm[ovirt-4.0]: net: canonicalize IPv4 prefix to netmask
by phoracek＠redhat.com 02 Nov '16

02 Nov '16

Petr Horáček has uploaded a new change for review. Change subject: net: canonicalize IPv4 prefix to netmask ...................................................................... net: canonicalize IPv4 prefix to netmask In order to fix OVS switch, this patch moves canonicalization of passed IPv4 prefix into netmask from legacy_switch.py to canonicalize.py. It also adds missing 'prefix' to setupNetworks documentation. Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b Bug-Url: https://bugzilla.redhat.com/1374194 Signed-off-by: Petr Horáček <phoracek(a)redhat.com> --- M lib/vdsm/network/api.py M lib/vdsm/network/canonicalize.py M lib/vdsm/network/legacy_switch.py M tests/network/func_static_ip_test.py M tests/network/netfunctestlib.py 5 files changed, 30 insertions(+), 14 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/64507/1 diff --git a/lib/vdsm/network/api.py b/lib/vdsm/network/api.py index 799686f..1b5ab48 100644 --- a/lib/vdsm/network/api.py +++ b/lib/vdsm/network/api.py @@ -209,7 +209,7 @@ bonding="<name>" | nic="<name>" (bonding and nics are mutually exclusive) ipaddr="<ipv4>" - netmask="<ipv4>" + netmask="<ipv4>" | prefix=<prefixlen> gateway="<ipv4>" bootproto="..." ipv6addr="<ipv6>[/<prefixlen>]" diff --git a/lib/vdsm/network/canonicalize.py b/lib/vdsm/network/canonicalize.py index b0bc983..3468796 100644 --- a/lib/vdsm/network/canonicalize.py +++ b/lib/vdsm/network/canonicalize.py @@ -22,6 +22,7 @@ import six from .netinfo import bridges, mtus, bonding, dns +from .netinfo.addresses import prefix2netmask from vdsm import utils from .errors import ConfigNetworkError @@ -47,6 +48,7 @@ _canonicalize_switch_type_net(attrs) _canonicalize_ip_default_route(attrs) _canonicalize_nameservers(attrs) + _canonicalize_prefix(attrs) def canonicalize_bondings(bonds): @@ -138,3 +140,15 @@ data['nameservers'] = dns.get_host_nameservers() else: data['nameservers'] = [] + + +def _canonicalize_prefix(data): + prefix = data.pop('prefix', None) + if prefix: + if 'netmask' in data: + raise ConfigNetworkError( + ne.ERR_BAD_PARAMS, 'Both PREFIX and NETMASK supplied') + try: + data['netmask'] = prefix2netmask(int(prefix)) + except ValueError as ve: + raise ConfigNetworkError(ne.ERR_BAD_ADDR, 'Bad prefix: %s' % ve) diff --git a/lib/vdsm/network/legacy_switch.py b/lib/vdsm/network/legacy_switch.py index 05a62c2..8d426ec 100644 --- a/lib/vdsm/network/legacy_switch.py +++ b/lib/vdsm/network/legacy_switch.py @@ -30,7 +30,6 @@ from vdsm.network import kernelconfig from vdsm.network import libvirt from vdsm.network.netinfo import NET_PATH -from vdsm.network.netinfo import addresses from vdsm.network.netinfo import bridges from vdsm.network.netinfo import mtus from vdsm.network.netinfo import nics as netinfo_nics @@ -187,7 +186,7 @@ @_alter_running_config def _add_network(network, configurator, nameservers, vlan=None, bonding=None, nic=None, ipaddr=None, - netmask=None, prefix=None, mtu=None, gateway=None, + netmask=None, mtu=None, gateway=None, dhcpv6=None, ipv6addr=None, ipv6gateway=None, ipv6autoconf=None, bridged=True, _netinfo=None, hostQos=None, defaultRoute=None, blockingdhcp=False, **options): @@ -201,16 +200,6 @@ if network == '': raise ConfigNetworkError(ne.ERR_BAD_BRIDGE, 'Empty network names are not valid') - if prefix: - if netmask: - raise ConfigNetworkError(ne.ERR_BAD_PARAMS, - 'Both PREFIX and NETMASK supplied') - else: - try: - netmask = addresses.prefix2netmask(int(prefix)) - except ValueError as ve: - raise ConfigNetworkError(ne.ERR_BAD_ADDR, 'Bad prefix: %s' % - ve) logging.debug('Validating network...') if network in _netinfo.networks: diff --git a/tests/network/func_static_ip_test.py b/tests/network/func_static_ip_test.py index 136a44c..e9bfdb2 100644 --- a/tests/network/func_static_ip_test.py +++ b/tests/network/func_static_ip_test.py @@ -108,6 +108,17 @@ with self.setupNetworks(netcreate, bondcreate, NOCHK): self.assertNetworkIp(NETWORK_NAME, netcreate[NETWORK_NAME]) + def test_add_net_with_prefix(self): + with dummy_device() as nic: + network_attrs = {'nic': nic, + 'ipaddr': IPv4_ADDRESS, + 'prefix': IPv4_PREFIX_LEN, + 'switch': self.switch} + netcreate = {NETWORK_NAME: network_attrs} + + with self.setupNetworks(netcreate, {}, NOCHK): + self.assertNetworkIp(NETWORK_NAME, netcreate[NETWORK_NAME]) + @attr(type='functional', switch='legacy') class NetworkStaticIpBasicLegacyTest(NetworkStaticIpBasicTemplate): diff --git a/tests/network/netfunctestlib.py b/tests/network/netfunctestlib.py index 7b59fa8..9e9b14f 100644 --- a/tests/network/netfunctestlib.py +++ b/tests/network/netfunctestlib.py @@ -30,6 +30,7 @@ from vdsm.network import kernelconfig from vdsm.network.ip import dhclient from vdsm.network.ip.address import ipv6_supported +from vdsm.network.netinfo.addresses import prefix2netmask from vdsm.network.netinfo.nics import operstate from testlib import VdsmTestCase @@ -283,7 +284,8 @@ def assertStaticIPv4(self, netattrs, ipinfo): requires_ipaddress() address = netattrs['ipaddr'] - netmask = netattrs['netmask'] + netmask = (netattrs.get('netmask') or + prefix2netmask(int(netattrs.get('prefix')))) self.assertEqual(address, ipinfo['addr']) self.assertEqual(netmask, ipinfo['netmask']) ipv4 = ipaddress.IPv4Interface( -- To view, visit https://gerrit.ovirt.org/64507 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-4.0 Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>

4 8

Change in vdsm[master]: build: Remove build requires needed for the tests
by Nir Soffer 01 Nov '16

01 Nov '16

Nir Soffer has uploaded a new change for review. Change subject: build: Remove build requires needed for the tests ...................................................................... build: Remove build requires needed for the tests We are not running the tests during the build, so we don't need to pull these packages. These packages are installed in the CI using automation/check-patch.packages. Developers should also use the same file for setting up a development environment. Change-Id: I3c385b0abeb2657a26215ce5b96976cc290734bb Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm.spec.in 1 file changed, 1 insertion(+), 55 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/65500/1 diff --git a/vdsm.spec.in b/vdsm.spec.in index 9252530..e4806fe 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -57,54 +57,10 @@ %{!?_licensedir:%global license %%doc} -BuildRequires: cyrus-sasl-lib BuildRequires: python BuildRequires: python2-devel -BuildRequires: python-mock -BuildRequires: python-netaddr -BuildRequires: python-six >= 1.9.0 BuildRequires: rpm-build - -# BuildRequires needed by the tests during the build -BuildRequires: dosfstools -BuildRequires: genisoimage -BuildRequires: libnl3 -BuildRequires: libselinux-python -BuildRequires: libvirt-python -BuildRequires: m2crypto -BuildRequires: mom >= 0.5.5 -BuildRequires: openssl -BuildRequires: policycoreutils-python -BuildRequires: psmisc -BuildRequires: PyYAML -BuildRequires: python-cpopen >= 1.4-1 -BuildRequires: python-inotify -BuildRequires: python-ioprocess >= 0.16.1-1 -BuildRequires: python-pthreading -BuildRequires: qemu-img -BuildRequires: rpm-python -%if 0%{?with_gluster_mgmt} -BuildRequires: python-blivet -%endif -BuildRequires: sanlock-python - -%if 0%{?with_openvswitch} -BuildRequires: openvswitch >= 2.0.0 -%endif - -%if 0%{?with_python3} -%if 0%{?rhel} -BuildRequires: python34-nose -BuildRequires: python34-six -%else # fedora -BuildRequires: python3-nose -BuildRequires: python3-six -BuildRequires: python3-netaddr -BuildRequires: python3-yaml -BuildRequires: libselinux-python3 -BuildRequires: libvirt-python3 -%endif # rhel -%endif # support python3 +BuildRequires: systemd-units # Autotools BuildRequires %if 0%{?enable_autotools} @@ -114,16 +70,6 @@ BuildRequires: libtool %endif -BuildRequires: systemd-units - -%if 0%{?rhel} -BuildRequires: python-decorator -%endif # rhel - -%if 0%{?fedora} -BuildRequires: python2-decorator -BuildRequires: python3-decorator -%endif # Numactl is not available on s390[x] and ARM %ifnarch s390 s390x %{arm} -- To view, visit https://gerrit.ovirt.org/65500 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3c385b0abeb2657a26215ce5b96976cc290734bb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

5 7

Change in vdsm[master]: build: Disable tests during build
by Nir Soffer 01 Nov '16

01 Nov '16

Nir Soffer has uploaded a new change for review. Change subject: build: Disable tests during build ...................................................................... build: Disable tests during build Tests are needed for development, not for building a package. This allows us to use latest and greatest development tools, which are not available in brew or koji. Since we install nose using pip, remove the build requires - we don't need now nose to build rpms. Change-Id: I9e3589c365166f934f117b53c65cea4b90db3516 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm.spec.in 1 file changed, 0 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/63966/1 diff --git a/vdsm.spec.in b/vdsm.spec.in index f234b09..c7f6dde 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -62,7 +62,6 @@ BuildRequires: python2-devel BuildRequires: python-mock BuildRequires: python-netaddr -BuildRequires: python-nose BuildRequires: python-six >= 1.9.0 BuildRequires: rpm-build @@ -95,10 +94,8 @@ %if 0%{?with_python3} %if 0%{?rhel} -BuildRequires: python34-nose BuildRequires: python34-six %else # fedora -BuildRequires: python3-nose BuildRequires: python3-six BuildRequires: python3-netaddr BuildRequires: python3-yaml @@ -781,9 +778,6 @@ # Install the libvirt hook for cleaning up the XML install -Dm 0755 vdsm/virt/libvirt-hook.sh \ %{buildroot}%{_sysconfdir}/libvirt/hooks/qemu - -%check -make tests %pre # Force standard locale behavior (English) -- To view, visit https://gerrit.ovirt.org/63966 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9e3589c365166f934f117b53c65cea4b90db3516 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

8 32

Change in vdsm[master]: utils: Wait for terminated process
by Nir Soffer 01 Nov '16

01 Nov '16

Nir Soffer has uploaded a new change for review. Change subject: utils: Wait for terminated process ...................................................................... utils: Wait for terminated process utils.terminating was not waiting for a terminated process, passing the process to zombiereaper. This behavior may be disastrous storage and mostly unwanted for other code. When we run a child process touching shared storage, we must wait for the child process. If we kill the child process without waiting, engine may start the same operation on another host, while the original child process is still running, possibly in interruptible state, trying to write to storage. To avoid this issue, we always invoke wait() after killing the process. Special code that do not want to wait for the child process should not use this context manager. Change-Id: Ida04e2c7ba092efdc2927ed9f460b0098ba2ad94 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M lib/vdsm/utils.py M tests/utilsTests.py 2 files changed, 7 insertions(+), 34 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/65324/1 diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index a428c8e..e5c28b8 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -738,8 +738,7 @@ if proc.poll() is None: logging.debug('Terminating process pid=%d' % proc.pid) proc.kill() - if proc.poll() is None: - zombiereaper.autoReapPID(proc.pid) + proc.wait() except Exception: logging.exception('Failed to kill process %d' % proc.pid) diff --git a/tests/utilsTests.py b/tests/utilsTests.py index 388a65d..6ff85d4 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -42,9 +42,8 @@ from vdsm import cmdutils from vdsm import commands from vdsm import panic -from vdsm.common import zombiereaper -from monkeypatch import MonkeyPatch, MonkeyPatchScope +from monkeypatch import MonkeyPatch from vmTestsData import VM_STATUS_DUMP from monkeypatch import Patch from testlib import forked, online_cpus, namedTemporaryDir @@ -83,22 +82,11 @@ self.patch.revert() -def wait_for_removal(path, timeout, wait=0.1): - deadline = utils.monotonic_time() + timeout - while True: - if not os.path.exists(path): - return True - if utils.monotonic_time() > deadline: - return False - time.sleep(wait) - - class TerminatingTests(TestCaseBase): def setUp(self): self.proc = commands.execCmd([EXT_SLEEP, "2"], sync=False) self.kill_proc = self.proc.kill - self.reaped = set() def tearDown(self): if self.proc.poll() is None: @@ -108,8 +96,7 @@ def test_terminating(self): with utils.terminating(self.proc): self.assertIsNone(self.proc.poll()) - proc_path = "/proc/%d" % self.proc.pid - self.assertTrue(wait_for_removal(proc_path, timeout=1)) + self.assertEqual(self.proc.returncode, -signal.SIGKILL) def test_terminating_with_kill_exception(self): class FakeKillError(Exception): @@ -118,24 +105,11 @@ def fake_kill(): raise FakeKillError("fake kill exception") - with MonkeyPatchScope([(zombiereaper, - 'autoReapPID', - self.reaped.add - )]): - self.proc.kill = fake_kill - with utils.terminating(self.proc): - self.assertIsNone(self.proc.poll()) - self.assertTrue(self.proc.pid not in self.reaped) + self.proc.kill = fake_kill + with utils.terminating(self.proc): + self.assertIsNone(self.proc.poll()) - def test_terminating_with_infected_kill(self): - with MonkeyPatchScope([(zombiereaper, - 'autoReapPID', - self.reaped.add - )]): - self.proc.kill = lambda: None - with utils.terminating(self.proc): - self.assertIsNone(self.proc.poll()) - self.assertTrue(self.proc.pid in self.reaped) + self.assertIsNone(self.proc.returncode) class RetryTests(TestCaseBase): -- To view, visit https://gerrit.ovirt.org/65324 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ida04e2c7ba092efdc2927ed9f460b0098ba2ad94 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

7 14

Change in vdsm[master]: readme: Fix build status url
by Nir Soffer 01 Nov '16

01 Nov '16

Nir Soffer has uploaded a new change for review. Change subject: readme: Fix build status url ...................................................................... readme: Fix build status url Previously we pointed to Nir's private fork, now that oVirt's github is triggering builds we should point to the correct builds. Change-Id: If40bda074383ffa1c4626bb7e68a6035cf7039e3 Signed-off-by: Nir Soffer <nirsof(a)gmail.com> --- M README.md 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/65904/1 diff --git a/README.md b/README.md index e253d3a..d3ba4c8 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # Vdsm: Virtual Desktop Server Manager -[![Build Status](https://travis-ci.org/nirs/vdsm.svg?branch=master)] -(https://travis-ci.org/nirs/vdsm) +[![Build Status](https://travis-ci.org/oVirt/vdsm.svg?branch=master)] +(https://travis-ci.org/oVirt/vdsm) The Vdsm service exposes an API for managing virtualization hosts running the KVM hypervisor technology. Vdsm manages and monitors -- To view, visit https://gerrit.ovirt.org/65904 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If40bda074383ffa1c4626bb7e68a6035cf7039e3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

3 4

Change in vdsm[master]: virt: Make DomainDescriptor use XML helpers
by mzamazal＠redhat.com 31 Oct '16

31 Oct '16

Milan Zamazal has uploaded a new change for review. Change subject: virt: Make DomainDescriptor use XML helpers ...................................................................... virt: Make DomainDescriptor use XML helpers We are going to stop using xml.dom.minidom and to use xml.etree instead. XML processing is currently spread across several places and we use both minidom and etree, so we must be careful with the conversion. The overall strategy is as follows: 1. Create XML helpers abstracting common XML operations. 2. Use the helpers everywhere where minidom is currently used. Avoid using minidom anywhere outside the XML helpers. 3. Switch from minidom to etree in the XML helpers. We avoid using XML directly in many places as a side effect. It is debatable whether it is a good or bad thing. If we decide it is a good thing, we may want to replace direct etree base XML processing with the helpers as well in a followup patch series. If we decide it is a bad thing, we can unfold the helpers in a followup patch series (note we need the helpers as an intermediate step and can't avoid them from the beginning). Keeping status quo after this series is also an option. There's also duplicate XML code and processing present in some places, especially in core devices. We keep it duplicated as it is in this series, in order not to mix etree conversion with refactoring. Device XML processing refactoring is planned in a followup patch series. This patch implements the first step of the whole process. It converts DomainDescriptor class from minidom to XML helpers. The reason for this start is that DomainDescriptor class is simple while it requires most of the needed XML helpers. So we can start with a small change of the existent code while becoming equipped with a nice set XML helpers for the following, perhaps more complicated, patches. Change-Id: Ib169735936d19171ff8b8d127666d7258c308f34 Signed-off-by: Milan Zamazal <mzamazal(a)redhat.com> --- M tests/vmXmlTests.py M vdsm/virt/domain_descriptor.py M vdsm/virt/vm.py M vdsm/virt/vmxml.py 4 files changed, 216 insertions(+), 23 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/55769/9 diff --git a/tests/vmXmlTests.py b/tests/vmXmlTests.py index c3e8f05..3051c63 100644 --- a/tests/vmXmlTests.py +++ b/tests/vmXmlTests.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- # # Copyright 2014, 2016 Red Hat, Inc. # @@ -26,7 +27,7 @@ from virt import vmxml from testlib import VdsmTestCase as TestCaseBase -from testlib import permutations, expandPermutations +from testlib import XMLTestCase, permutations, expandPermutations import vmfakelib as fake @@ -59,6 +60,58 @@ @expandPermutations +class TestVmXmlHelpers(XMLTestCase): + + _XML = u'''<?xml version="1.0" ?> + <topelement> + <hello lang="english">hello</hello> + <hello cyrillic="yes" lang="русский">здра́вствуйте</hello> + <bye>good bye<hello lang="čeština">dobrý den</hello></bye> + <empty/> + </topelement> + ''' + + def setUp(self): + self._dom = vmxml.parse_xml(self._XML) + + def test_import_export(self): + xml = vmxml.export_xml(self._dom) + self.assertXMLEqual(xml, self._XML) + + @permutations([[None, 'topelement', 1], + ['topelement', 'topelement', 1], + [None, 'hello', 3], + ['topelement', 'bye', 1], + [None, 'none', 0]]) + def test_find_all(self, start_tag, tag, number): + dom = self._dom + if start_tag is not None: + dom = vmxml.find_first(self._dom, 'topelement') + elements = vmxml.find_all(dom, tag) + matches = [vmxml.tag(e) == tag for e in elements] + self.assertTrue(all(matches)) + self.assertEqual(len(matches), number) + + def test_find_first_not_found(self): + self.assertRaises(vmxml.NotFound, vmxml.find_first, self._dom, 'none') + + @permutations([['hello', 'lang', 'english'], + ['hello', 'none', ''], + ['none', 'lang', ''], + ]) + def test_find_attr(self, tag, attribute, result): + value = vmxml.find_attr(self._dom, tag, attribute) + self.assertEqual(value, result) + + @permutations([['hello', 'hello'], + ['empty', '']]) + def test_text(self, tag, result): + element = vmxml.find_first(self._dom, tag) + text = vmxml.text(element) + self.assertEqual(text, result) + + +@expandPermutations class TestDomainDescriptor(VmXmlTestCase): @permutations([[cpuarch.X86_64], [cpuarch.PPC64]]) diff --git a/vdsm/virt/domain_descriptor.py b/vdsm/virt/domain_descriptor.py index 2c2057f..c6f84cd 100644 --- a/vdsm/virt/domain_descriptor.py +++ b/vdsm/virt/domain_descriptor.py @@ -17,16 +17,16 @@ # # Refer to the README and COPYING files for full details of the license # -import xml.dom.minidom +from . import vmxml class DomainDescriptor(object): def __init__(self, xmlStr): self._xml = xmlStr - self._dom = xml.dom.minidom.parseString(xmlStr) - self._devices = self._first_element_by_tag_name('devices') - self._devices_hash = hash(self._devices.toxml() + self._dom = vmxml.parse_xml(xmlStr) + self._devices = vmxml.find_first(self._dom, 'devices', None) + self._devices_hash = hash(vmxml.export_xml(self._devices) if self._devices is not None else '') @classmethod @@ -42,31 +42,22 @@ return self._devices def get_device_elements(self, tagName): - return self._devices.getElementsByTagName(tagName) + return vmxml.find_all(self._devices, tagName) @property def devices_hash(self): return self._devices_hash def all_channels(self): - for channel in self.get_device_elements('channel'): - try: - name = channel.getElementsByTagName('target')[0].\ - getAttribute('name') - path = channel.getElementsByTagName('source')[0].\ - getAttribute('path') - except IndexError: - continue - else: + for channel in vmxml.find_all(self._devices, 'channel'): + name = vmxml.find_attr(channel, 'target', 'name') + path = vmxml.find_attr(channel, 'source', 'path') + if name and path: yield name, path - - def _first_element_by_tag_name(self, tagName): - elements = self._dom.childNodes[0].getElementsByTagName(tagName) - return elements[0] if elements else None def get_memory_size(self): """ Return the vm memory from xml in MiB """ - memory = self._first_element_by_tag_name("memory") - return int(memory.firstChild.nodeValue) // 1024 if memory else None + memory = vmxml.find_first(self._dom, "memory") + return (int(vmxml.text(memory)) // 1024 if memory else None) diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index e24a90a..2b9e03e 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -3761,8 +3761,8 @@ use updateDevice to select the device. """ try: - graphics = self._domain.get_device_elements('graphics')[0] - except IndexError: + graphics = next(self._domain.get_device_elements('graphics')) + except StopIteration: return response.error('ticketErr', 'no graphics devices configured') return self._setTicketForGraphicDev( diff --git a/vdsm/virt/vmxml.py b/vdsm/virt/vmxml.py index b868a25..02927d9 100644 --- a/vdsm/virt/vmxml.py +++ b/vdsm/virt/vmxml.py @@ -33,6 +33,155 @@ _BOOT_MENU_TIMEOUT = 10000 # milliseconds +_UNSPECIFIED = object() + + +class NotFound(Exception): + """ + Raised when vmxml helpers can't find some requested entity. + """ + pass + + +def parse_xml(xml_string): + """ + Parse given XML string to DOM element and return the element. + + :param xml_string: XML string to parse + :type xml_string: str + :returns: DOM element created by parsing `xml_string` + :rtype: DOM element + + """ + return xml.dom.minidom.parseString(xml_string) + + +def export_xml(element): + """ + Export given DOM element to XML string. + + :param element: DOM element to export + :type element: DOM element + :returns: XML corresponding to `element` content + :rtype: basestring + + """ + return element.toxml(encoding='UTF-8') + + +def find_all(element, tag_): + """ + Return an iterator over all DOM elements with given `tag`. + + `element` may is included in the result if it is of `tag`. + + :param element: DOM element to be searched for given `tag` + :type element: DOM element + :param tag: tag to look for + :type tag: basestring + :returns: all elements with given `tag` + :rtype: sequence of DOM elements + + """ + if isinstance(element, xml.dom.minidom.Element) and \ + tag(element) == tag_: + yield element + for elt in element.getElementsByTagName(tag_): + yield elt + + +def find_first(element, tag, default=_UNSPECIFIED): + """ + Find the first DOM element with the given tag. + + :param element: DOM element to be searched for given `tag` + :type element: DOM element + :param tag: tag to look for + :type tag: basestring + :param default: any object to return when no element with `tag` is found; + if not given then `NotFound` is raised in such a case + :returns: the matching element or default + :raises: NotFound -- when no element with `tag` is found and `default` is + not given + + """ + try: + return next(find_all(element, tag)) + except StopIteration: + if default is _UNSPECIFIED: + raise NotFound((element, tag,)) + else: + return default + + +def find_attr(element, tag, attribute): + """ + Find `attribute` value of the first DOM element with `tag`. + + :param element: DOM element to be searched for given `tag` + :type element: DOM element + :param tag: tag to look for + :type tag: basestring + :param attribute: attribute name to look for + :type attribute: basestring + :returns: the attribute value or an empty string if no element with given + tag is found or the found element doesn't contain `attribute` + :rtype: basestring + + """ + try: + subelement = find_first(element, tag) + except NotFound: + return '' + return attr(subelement, attribute) + + +def tag(element): + """ + Return tag of the given DOM element. + + :param element: element to get the tag of + :type element: DOM element + :returns: tag of the element + :rtype: basestring + + """ + return element.tagName + + +def attr(element, attribute): + """ + Return attribute values of `element`. + + :param element: the element to look the attributes in + :type element: DOM element + :param attribute: attribute name to look for + :type attribute: basestring + :returns: the corresponding attribute value or empty string (if no element + with `tag` or exists or `attribute` is not present) + :rtype: basestring + + """ + # Minidom returns unicodes, except for empty strings. + return element.getAttribute(attribute) + + +def text(element): + """ + Return text of the given DOM element. + + :param element: element to get the text from + :type element: DOM element + :returns: text of the element (empty string if it the element doesn't + contain any text) + :rtype: basestring + + """ + child_node = element.firstChild + if child_node is None: + return '' + return child_node.nodeValue + def has_channel(domXML, name): domObj = etree.fromstring(domXML) -- To view, visit https://gerrit.ovirt.org/55769 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib169735936d19171ff8b8d127666d7258c308f34 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com> Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com> Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>

5 29

Change in vdsm[master]: storage: iscsiadm parses IPv6 iSCSI addresses
by dholler＠redhat.com 31 Oct '16

31 Oct '16

Dominik Holler has uploaded a new change for review. Change subject: storage: iscsiadm parses IPv6 iSCSI addresses ...................................................................... storage: iscsiadm parses IPv6 iSCSI addresses Enables parsing of IPv6 iSCSI addresses during discovery. Change-Id: I0e9fa8d8f5ac34753ad6d40625e64dfeee1a3c02 Bug-Url: https://bugzilla.redhat.com/1308306 Signed-off-by: Dominik Holler <dholler(a)redhat.com> --- M vdsm/storage/iscsiadm.py 1 file changed, 9 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/65707/5 diff --git a/vdsm/storage/iscsiadm.py b/vdsm/storage/iscsiadm.py index 52a0083..2cfe6c5 100644 --- a/vdsm/storage/iscsiadm.py +++ b/vdsm/storage/iscsiadm.py @@ -31,6 +31,7 @@ ISCSI_ERR_SESS_EXISTS = 15 ISCSI_ERR_LOGIN_AUTH_FAILED = 24 ISCSI_ERR_OBJECT_NOT_FOUND = 21 +ISCSI_HOST_IPV6_REGEX = re.compile(r"\[(.*)\]:([0-9]+)") Iface = namedtuple('Iface', 'ifacename transport_name hwaddress ipaddress \ net_ifacename initiatorname') @@ -239,11 +240,16 @@ if rc == 0: res = [] for line in out: - if line.startswith("["): # skip IPv6 targets - continue rest, iqn = line.split() rest, tpgt = rest.split(",") - ip, port = rest.split(":") + if rest.startswith("["): + # IPv6 + match = ISCSI_HOST_IPV6_REGEX.match(rest) + ip = match.group(1) + port = match.group(2) + else: + # IPv4 + ip, port = rest.split(":") res.append((ip, int(port), int(tpgt), iqn)) return res -- To view, visit https://gerrit.ovirt.org/65707 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0e9fa8d8f5ac34753ad6d40625e64dfeee1a3c02 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dominik Holler <dholler(a)redhat.com>

5 19

← Newer
1
...
4
5
6
7
8
9
10
...
47
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches October 2016