vdsm-patches September 2015

vdsm-patches@lists.fedorahosted.org

46 participants
667 discussions

Change in vdsm[master]: migration: move progress update into an helper
by fromani＠redhat.com 16 Oct '15

16 Oct '15

Francesco Romani has uploaded a new change for review. Change subject: migration: move progress update into an helper ...................................................................... migration: move progress update into an helper Streamline migration.SourceThread.getStat() by moving responsabilities in helper functions, in order to make the code easier to follow. Let's start with the progress reporting and tracking. Change-Id: Ie9707fcc492a394f8d7aebd57482c44b7b5a703d Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M vdsm/virt/migration.py 1 file changed, 6 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/42797/1 diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index 415cdc1..6f7631a 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -114,9 +114,7 @@ else: res = utils.picklecopy(self._last_error) - if self._monitorThread is not None: - # fetch migration status from the monitor thread - self._progress = self._monitorThread.progress + self._updateStatus() res['progress'] = self._progress @@ -126,6 +124,11 @@ return res + def _updateStatus(self): + if self._monitorThread is not None: + # we need to report monotonic progress + self._progress = self._monitorThread.progress + def _createClient(self, port): sslctx = sslutils.create_ssl_context() client_socket = utils.create_connected_socket( -- To view, visit https://gerrit.ovirt.org/42797 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie9707fcc492a394f8d7aebd57482c44b7b5a703d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

2 7

Change in vdsm[master]: migration: de-entangle migration stat reporting
by fromani＠redhat.com 16 Oct '15

16 Oct '15

Francesco Romani has uploaded a new change for review. Change subject: migration: de-entangle migration stat reporting ...................................................................... migration: de-entangle migration stat reporting migration.SourceThread has very entangled and messy status reporting. One single field is in charge to track migration status, including progress, errors and so forth. This leads to code hard to follow and to change. As first step to clean up the mess, we split the migration status tracking (e.g. progress while running, stats when done) from last error tracking. Change-Id: I6d745bacddf80c54354a2f7ec2d290dfd3b12d03 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M vdsm/virt/migration.py 1 file changed, 14 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/96/42796/1 diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index 99af017..415cdc1 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -89,6 +89,7 @@ config.get('vars', 'migration_downtime') self._autoConverge = autoConverge self._compressed = compressed + self._last_error = None self._last_status = { 'status': { 'code': 0, @@ -107,16 +108,23 @@ """ Get the status of the migration. """ + + if self._last_error is None: + res = utils.picklecopy(self._last_status) + else: + res = utils.picklecopy(self._last_error) + if self._monitorThread is not None: # fetch migration status from the monitor thread self._progress = self._monitorThread.progress - self._last_status['progress'] = self._progress + + res['progress'] = self._progress stat = self._vm._dom.jobStats(libvirt.VIR_DOMAIN_JOB_STATS_COMPLETED) if 'downtime_net' in stat: - self._last_status['downtime'] = stat['downtime_net'] + res['downtime'] = stat['downtime_net'] - return self._last_status + return res def _createClient(self, port): sslctx = sslutils.create_ssl_context() @@ -197,8 +205,7 @@ self._vm.lastStatus = vmstatus.MIGRATION_SOURCE def _recover(self, message): - if not response.is_failure(self._last_status): - self._last_status = response.error('migrateErr') + self._last_error = response.error('migrateErr') self.log.error(message) if not self.hibernating: try: @@ -288,7 +295,7 @@ self._finishSuccessfully() except libvirt.libvirtError as e: if e.get_error_code() == libvirt.VIR_ERR_OPERATION_ABORTED: - self._last_status = response.error( + self._last_error = response.error( 'migCancelErr', message='Migration canceled') raise @@ -326,7 +333,7 @@ destCreationTime) if response.is_failure(result): - self._last_status = result + self._last_error = result raise RuntimeError('migration destination error: ' + result['status']['message']) if config.getboolean('vars', 'ssl'): -- To view, visit https://gerrit.ovirt.org/42796 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6d745bacddf80c54354a2f7ec2d290dfd3b12d03 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

2 7

Change in vdsm[master]: migration: return plain response on error
by fromani＠redhat.com 16 Oct '15

16 Oct '15

Francesco Romani has uploaded a new change for review. Change subject: migration: return plain response on error ...................................................................... migration: return plain response on error We should add extra status fields only when migration is OK (either doing OK or completed OK). It makes no sense to do that on error, as the fields are meaningless. TODO: make sure there aren't legacy constraints and Engine unexpectedly relies on those fields being present on error. In that case I'll mark them REQUIRED_FOR in the error path and leave them alone until 4.0 Change-Id: Ifafa5de4db317ed0e2f735eca6d944042e19c4e1 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M vdsm/virt/migration.py 1 file changed, 5 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/42799/1 diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index 6ea4357..8cdfc10 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -111,12 +111,12 @@ if self._last_error is None: res = utils.picklecopy(self._last_status) + + self._updateStatus() + + return self._addMigrationFields(res) else: - res = utils.picklecopy(self._last_error) - - self._updateStatus() - - return self._addMigrationFields(res) + return utils.picklecopy(self._last_error) def _updateStatus(self): if self._monitorThread is not None: -- To view, visit https://gerrit.ovirt.org/42799 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ifafa5de4db317ed0e2f735eca6d944042e19c4e1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

2 7

Change in vdsm[master]: migration: add helper to add status field
by fromani＠redhat.com 16 Oct '15

16 Oct '15

Francesco Romani has uploaded a new change for review. Change subject: migration: add helper to add status field ...................................................................... migration: add helper to add status field In the case the migration is correctly progressing, or if it succesfully completed, we need to report extra status fields to Engine in a succesfull response. This patch extracts the code which does that from getStat to an helper method, to make the code more self-describing. Change-Id: I047705e2f45221ad3a7527ca6b018dc6d0f3368e Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M vdsm/virt/migration.py 1 file changed, 12 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/98/42798/1 diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index 6f7631a..6ea4357 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -116,6 +116,18 @@ self._updateStatus() + return self._addMigrationFields(res) + + def _updateStatus(self): + if self._monitorThread is not None: + # we need to report monotonic progress + self._progress = self._monitorThread.progress + + def _addMigrationFields(self, res): + """ + Adds migration-status specific fields, required + by schema, to a plain response object. + """ res['progress'] = self._progress stat = self._vm._dom.jobStats(libvirt.VIR_DOMAIN_JOB_STATS_COMPLETED) @@ -123,11 +135,6 @@ res['downtime'] = stat['downtime_net'] return res - - def _updateStatus(self): - if self._monitorThread is not None: - # we need to report monotonic progress - self._progress = self._monitorThread.progress def _createClient(self, port): sslctx = sslutils.create_ssl_context() -- To view, visit https://gerrit.ovirt.org/42798 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I047705e2f45221ad3a7527ca6b018dc6d0f3368e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

2 7

Change in vdsm[master]: migration: build new reason on success
by fromani＠redhat.com 16 Oct '15

16 Oct '15

Francesco Romani has uploaded a new change for review. Change subject: migration: build new reason on success ...................................................................... migration: build new reason on success There is no need to cache successfull responses, it is cleaner and safer to build them on the fly when requested. We should only cache errors, and only because SourceThread is orchestrating DowntimeThread and MonitorThread. Change-Id: Ic42b70be6f5a5406e23916e16a678dd8caf0f500 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M vdsm/virt/migration.py 1 file changed, 30 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/42800/1 diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index 8cdfc10..0e6a024 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -53,6 +53,20 @@ VIR_MIGRATE_PARAM_GRAPHICS_URI = 'graphics_uri' +class MigrationStatus(object): + IN_PROGRESS = 'Migration in progress' + COMPLETED = 'Migration done' + SAVESTATE_DONE = 'SaveState done' + CANCELED = 'Migration process cancelled' + + +class MigrationAbortReason(object): + UNKNOWN = 0, # no further detail available. + CLIENT = 1 + MAX_TIME = 2 + STALLED = 3 + + class SourceThread(threading.Thread): """ A thread that takes care of migration on the source vdsm. @@ -90,10 +104,7 @@ self._autoConverge = autoConverge self._compressed = compressed self._last_error = None - self._last_status = { - 'status': { - 'code': 0, - 'message': 'Migration in progress'}} + self._last_status = MigrationStatus.IN_PROGRESS self._progress = 0 threading.Thread.__init__(self) self._preparingMigrationEvt = True @@ -107,13 +118,23 @@ def getStat(self): """ Get the status of the migration. + + TODO: this method is troublesome. We are *lying*, we + are *not* compliant to the schema and we know it. + The problem is that we cannot change the return value + to conform to schema and to cleanup things, for + legacy constraings. """ if self._last_error is None: - res = utils.picklecopy(self._last_status) + res = response.success(message=self._last_status) self._updateStatus() + # this uglyness is REQUIRED_FOR: Engine 3.x + # TODO when we can break compatibility: + # refactor and cleanup. Schema is already + # much better, too bad we cannot comply. return self._addMigrationFields(res) else: return utils.picklecopy(self._last_error) @@ -232,7 +253,7 @@ self._progress = 100 if not self.hibernating: self._vm.setDownStatus(NORMAL, vmexitreason.MIGRATION_SUCCEEDED) - self._last_status['status']['message'] = 'Migration done' + self._last_status = MigrationStatus.COMPLETED else: # don't pickle transient params for ignoreParam in ('displayIp', 'display', 'pid'): @@ -247,7 +268,7 @@ self._vm.cif.teardownVolumePath(self._dstparams) self._vm.setDownStatus(NORMAL, vmexitreason.SAVE_STATE_SUCCEEDED) - self._last_status['status']['message'] = 'SaveState done' + self._last_status = MigrationStatus.SAVESTATE_DONE def _patchConfigForLegacy(self): """ @@ -421,9 +442,8 @@ if not self._preparingMigrationEvt: raise else: - self._last_status['status']['message'] = \ - 'Migration process cancelled' - return self._last_status + self._last_status = MigrationStatus.CANCELED + return response.success(message=self._last_status) def exponential_downtime(downtime, steps): -- To view, visit https://gerrit.ovirt.org/42800 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic42b70be6f5a5406e23916e16a678dd8caf0f500 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

2 7

Change in vdsm[master]: storage: Hidden files filtered form listings of storage domains
by jniederm＠redhat.com 15 Oct '15

15 Oct '15

Jakub Niedermertl has uploaded a new change for review. Change subject: storage: Hidden files filtered form listings of storage domains ...................................................................... storage: Hidden files filtered form listings of storage domains Listing of files in storage domain (FileStorageDomain#getFileList) now returns only non-hidden files (not starting with '.'). It allows to avoid various auxiliary hidden files like '.keep'. It is especially useful in context of ISO doman that stores files for various purposes. Change-Id: I0eebd8b9fe98c41df686330858b3292fa37ba245 Bug-Url: https://bugzilla.redhat.com/1259279 Signed-off-by: Jakub Niedermertl <jniederm(a)redhat.com> --- M vdsm/storage/fileSD.py M vdsm/storage/hsm.py 2 files changed, 11 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/45645/1 diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py index 5c9aa6d..85140ac 100644 --- a/vdsm/storage/fileSD.py +++ b/vdsm/storage/fileSD.py @@ -421,12 +421,15 @@ def getFileList(self, pattern, caseSensitive): """ - Returns a list of all files in the domain filtered according to - extension. + Returns a list of all visible (not starting with '.') files in the + domain filtered according to extension. """ basedir = self.getIsoDomainImagesDir() filesList = self.oop.simpleWalk(basedir) + nonHiddenFilesRE = '.*/[^.][^/]*$' + filesList = [x for x in filesList if re.match(nonHiddenFilesRE, x)] + if pattern != '*': if caseSensitive: filesList = fnmatch.filter(filesList, pattern) diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 82f4426..7674209 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -2305,8 +2305,8 @@ def getFileStats(self, sdUUID, pattern='*', caseSensitive=False, options=None): """ - Returns statistics of all files in the domain filtered according to - pattern. + Returns statistics of all visible (not starting with '.') files in + the domain filtered according to pattern. :param sdUUID: The UUID of the storage domain you want to query. :type sdUUID: UUID @@ -2333,7 +2333,8 @@ @public def getIsoList(self, spUUID, extension='iso', options=None): """ - Gets a list of all ISO/Floppy volumes in a storage pool. + Gets a list of all visible (not starting with '.') ISO/Floppy volumes + in a storage pool. :param spUUID: The UUID of the storage pool you want to query. :type spUUID: UUID @@ -2361,7 +2362,8 @@ @public def getFloppyList(self, spUUID, options=None): """ - Gets a list of all Floppy volumes if a storage pool. + Gets a list of all visible (not starting with '.') Floppy volumes of + a storage pool. :param spUUID: The UUID of the storage pool you want to query. :type spUUID: UUID -- To view, visit https://gerrit.ovirt.org/45645 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0eebd8b9fe98c41df686330858b3292fa37ba245 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Jakub Niedermertl <jniederm(a)redhat.com>

5 14

Change in vdsm[master]: ssl: configurable implementation
by Piotr Kliczewski 15 Oct '15

15 Oct '15

Piotr Kliczewski has uploaded a new change for review. Change subject: ssl: configurable implementation ...................................................................... ssl: configurable implementation We want to choose ssl implementation during build process. Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823 Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M debian/vdsm-python.install M lib/vdsm/Makefile.am M lib/vdsm/jsonrpcvdscli.py A lib/vdsm/sslutils.py M lib/vdsm/utils.py M lib/vdsm/vdscli.py M lib/yajsonrpc/stompreactor.py M tests/crossImportsTests.py.in M tests/integration/Makefile.am M tests/integration/jsonRpcHelper.py M tests/integration/m2chelper.py A tests/integration/sslhelper.py M tests/protocoldetectorTests.py M tests/sslTests.py M tests/stompTests.py M tests/vdscliTests.py M vdsm.spec.in M vdsm/clientIF.py M vdsm/kaxmlrpclib.py M vdsm/protocoldetector.py M vdsm/virt/migration.py 21 files changed, 562 insertions(+), 88 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/44494/1 diff --git a/debian/vdsm-python.install b/debian/vdsm-python.install index 36c0d9f..d9402c3 100644 --- a/debian/vdsm-python.install +++ b/debian/vdsm-python.install @@ -12,7 +12,6 @@ ./usr/lib/python2.7/dist-packages/vdsm/ipwrapper.py ./usr/lib/python2.7/dist-packages/vdsm/jsonrpcvdscli.py ./usr/lib/python2.7/dist-packages/vdsm/libvirtconnection.py -./usr/lib/python2.7/dist-packages/vdsm/m2cutils.py ./usr/lib/python2.7/dist-packages/vdsm/netconfpersistence.py ./usr/lib/python2.7/dist-packages/vdsm/netinfo.py ./usr/lib/python2.7/dist-packages/vdsm/netlink/__init__.py @@ -30,6 +29,7 @@ ./usr/lib/python2.7/dist-packages/vdsm/qemuimg.py ./usr/lib/python2.7/dist-packages/vdsm/response.py ./usr/lib/python2.7/dist-packages/vdsm/schedule.py +./usr/lib/python2.7/dist-packages/vdsm/sslutils.py ./usr/lib/python2.7/dist-packages/vdsm/tool/__init__.py ./usr/lib/python2.7/dist-packages/vdsm/tool/dummybr.py ./usr/lib/python2.7/dist-packages/vdsm/tool/dump_bonding_defaults.py diff --git a/lib/vdsm/Makefile.am b/lib/vdsm/Makefile.am index ec1a140..4a2bcb1 100644 --- a/lib/vdsm/Makefile.am +++ b/lib/vdsm/Makefile.am @@ -40,6 +40,7 @@ qemuimg.py \ response.py \ schedule.py \ + sslutils.py \ sysctl.py \ udevadm.py \ utils.py \ diff --git a/lib/vdsm/jsonrpcvdscli.py b/lib/vdsm/jsonrpcvdscli.py index 1687b6f..391d79e 100644 --- a/lib/vdsm/jsonrpcvdscli.py +++ b/lib/vdsm/jsonrpcvdscli.py @@ -22,7 +22,6 @@ from uuid import uuid4 import socket -from . import m2cutils from yajsonrpc import stompreactor from yajsonrpc import \ JsonRpcRequest, \ @@ -30,6 +29,10 @@ from vdsm import response from .config import config +try: + from . import m2cutils as sslutils +except ImportError: + from . import sslutils _COMMAND_CONVERTER = { diff --git a/lib/vdsm/sslutils.py b/lib/vdsm/sslutils.py new file mode 100644 index 0000000..63fc9cc --- /dev/null +++ b/lib/vdsm/sslutils.py @@ -0,0 +1,302 @@ +# +# 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 +# +from __future__ import absolute_import +import httplib +import logging +import os +import socket +import ssl +import xmlrpclib + +from vdsm.utils import ( + monotonic_time, +) +from .config import config + + +DEFAULT_ACCEPT_TIMEOUT = 5 +SOCKET_DEFAULT_TIMEOUT = socket._GLOBAL_DEFAULT_TIMEOUT + + +class SSLSocket(object): + def __init__(self, sock): + self.sock = sock + self._data = '' + + # ssl do not accept flag other than 0 + def read(self, size=4096, flag=None): + result = None + if flag == socket.MSG_PEEK: + bytes_left = size - len(self._data) + if bytes_left > 0: + self._data += self.sock.read(bytes_left) + result = self._data + else: + if self._data: + result = self._data + self._data = '' + else: + result = self.sock.read(size) + return result + recv = read + + def pending(self): + pending = self.sock.pending() + if self._data: + pending = pending + len(self._data) + return pending + + def __getattr__(self, name): + return getattr(self.sock, name) + + def makefile(self, mode='rb', bufsize=-1): + if mode == 'rb': + return socket._fileobject(self, mode, bufsize) + else: + return self.sock.makefile(mode, bufsize) + + +class SSLContext(object): + def __init__(self, cert_file, key_file, ca_certs=None, + protocol=ssl.PROTOCOL_TLSv1): + self.cert_file = cert_file + self.key_file = key_file + self.ca_certs = ca_certs + self.protocol = protocol + + +class VerifyingHTTPSConnection(httplib.HTTPSConnection): + def __init__(self, host, port=None, key_file=None, cert_file=None, + strict=None, timeout=SOCKET_DEFAULT_TIMEOUT, + ca_certs=None, cert_reqs=ssl.CERT_REQUIRED): + httplib.HTTPSConnection.__init__(self, host, port, key_file, cert_file, + strict, timeout) + self.ca_certs = ca_certs + self.cert_reqs = cert_reqs + + def connect(self): + "Connect to a host on a given (SSL) port." + + sock = socket.create_connection((self.host, self.port), self.timeout) + if self._tunnel_host: + self.sock = sock + self._tunnel() + # DK added: pass ca_cert to sslsocket + self.sock = ssl.wrap_socket(sock, self.key_file, self.cert_file, + ca_certs=self.ca_certs, server_side=False, + cert_reqs=self.cert_reqs) + + +class VerifyingSafeTransport(xmlrpclib.SafeTransport): + def __init__(self, use_datetime=0, key_file=None, cert_file=None, + ca_certs=None, cert_reqs=ssl.CERT_REQUIRED, + timeout=SOCKET_DEFAULT_TIMEOUT): + xmlrpclib.SafeTransport.__init__(self, use_datetime) + self.key_file = key_file + self.cert_file = cert_file + self.ca_certs = ca_certs + self.cert_reqs = cert_reqs + self._timeout = timeout + + def make_connection(self, host): + """Return VerifyingHTTPS object that is aware of ca_certs, and will + create VerifyingHTTPSConnection. + In Python 2.7, return VerifyingHTTPSConnection object + """ + chost, self._extra_headers, x509 = self.get_host_info(host) + if hasattr(xmlrpclib.SafeTransport, "single_request"): # Python 2.7 + return VerifyingHTTPSConnection( + chost, None, key_file=self.key_file, strict=None, + cert_file=self.cert_file, timeout=self._timeout, + ca_certs=self.ca_certs, + cert_reqs=self.cert_reqs) + else: + return VerifyingHTTPS( + chost, None, key_file=self.key_file, + cert_file=self.cert_file, timeout=self._timeout, + ca_certs=self.ca_certs, + cert_reqs=self.cert_reqs) + + +class VerifyingHTTPS(httplib.HTTPS): + _connection_class = VerifyingHTTPSConnection + + def __init__(self, host='', port=None, key_file=None, cert_file=None, + strict=None, timeout=SOCKET_DEFAULT_TIMEOUT, + ca_certs=None, cert_reqs=ssl.CERT_REQUIRED): + """A ca_cert-aware HTTPS object, + that creates a VerifyingHTTPSConnection + """ + # provide a default host, pass the X509 cert info + + # urf. compensate for bad input. + if port == 0: + port = None + self._setup(self._connection_class(host=host, + port=port, + key_file=key_file, + cert_file=cert_file, + strict=strict, + timeout=timeout, + ca_certs=ca_certs, + cert_reqs=cert_reqs)) + + # we never actually use these for anything, but we keep them + # here for compatibility with post-1.5.2 CVS. + self.key_file = key_file + self.cert_file = cert_file + + +class SSLHandshakeDispatcher(object): + """ + SSLHandshakeDispatcher is dispatcher implementation to process ssl + handshake in asynchronous way. Once we are done with handshaking we + we need to swap our dispatcher implementation with message processing + dispatcher. We use handshake_finished_handler function to perform + swapping. The handler implementation need to invoke + + dispatcher.switch_implementation() + + where we provide message processing dispatcher as parameter. + """ + log = logging.getLogger("ProtocolDetector.SSLHandshakeDispatcher") + SSL_HANDSHAKE_TIMEOUT = 10 + + def __init__( + self, + sslinfo, + handshake_finished_handler, + handshake_timeout=SSL_HANDSHAKE_TIMEOUT, + ): + self._give_up_at = monotonic_time() + handshake_timeout + self._has_been_set_up = False + self._is_handshaking = True + self.want_read = True + self.want_write = True + self._sslinfo = sslinfo + self._handshake_finished_handler = handshake_finished_handler + + def _set_up_socket(self, dispatcher): + client_socket = dispatcher.socket + client_socket = SSLSocket( + ssl.wrap_socket(client_socket, + keyfile=self._sslinfo.key_file, + certfile=self._sslinfo.cert_file, + server_side=True, + cert_reqs=ssl.CERT_REQUIRED, + ssl_version=self._sslinfo.protocol, + ca_certs=self._sslinfo.ca_certs, + do_handshake_on_connect=False)) + + dispatcher.socket = client_socket + self._has_been_set_up = True + + def next_check_interval(self): + return max(self._give_up_at - monotonic_time(), 0) + + def readable(self, dispatcher): + if self.has_expired(): + dispatcher.close() + return False + + return self.want_read + + def writable(self, dispatcher): + if self.has_expired(): + dispatcher.close() + return False + + return self.want_write + + def has_expired(self): + return monotonic_time() >= self._give_up_at + + def handle_read(self, dispatcher): + self._handle_io(dispatcher) + + def handle_write(self, dispatcher): + self._handle_io(dispatcher) + + def _handle_io(self, dispatcher): + if not self._has_been_set_up: + self._set_up_socket(dispatcher) + + if self._is_handshaking: + self._handshake(dispatcher) + else: + if not self._verify_host(dispatcher.socket.getpeercert(), + dispatcher.socket.getpeername()[0]): + self.log.error("peer certificate does not match host name") + dispatcher.socket.close() + return + + self._handshake_finished_handler(dispatcher) + + def _verify_host(self, peercert, addr): + if not peercert: + return False + + for sub in peercert.get("subject", ()): + for key, value in sub: + if key == "commonName": + return self._compare_names(addr, value) + + return False + + def _compare_names(self, addr, name): + if addr == name or addr == '127.0.0.1': + return True + else: + return name.lower() == socket.gethostbyaddr(addr)[0].lower() + + def _handshake(self, dispatcher): + try: + dispatcher.socket.do_handshake() + except ssl.SSLError as err: + self.want_read = self.want_write = False + if err.args[0] == ssl.SSL_ERROR_WANT_READ: + self.want_read = True + elif err.args[0] == ssl.SSL_ERROR_WANT_WRITE: + self.want_write = True + else: + dispatcher.close() + raise + else: + self.want_read = self.want_write = True + self._is_handshaking = False + + +def create_ssl_context(): + sslinfo = None + if config.getboolean('vars', 'ssl'): + truststore_path = config.get('vars', 'trust_store_path') + protocol = ( + ssl.PROTOCOL_SSLv23 + if config.get('vars', 'ssl_protocol') == 'sslv23' + else ssl.PROTOCOL_TLSv1 + ) + sslinfo = SSLContext( + key_file=os.path.join(truststore_path, 'keys', 'vdsmkey.pem'), + cert_file=os.path.join(truststore_path, 'certs', + 'vdsmcert.pem'), + ca_certs=os.path.join(truststore_path, 'certs', 'cacert.pem'), + protocol=protocol + ) + return sslinfo diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index d847a23..b7818eb 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -54,11 +54,16 @@ import threading import time import vdsm.infra.zombiereaper as zombiereaper -from M2Crypto import SSL from cpopen import CPopen from . import cmdutils from . import constants +try: + from M2Crypto import SSL + _m2cEnabled = True +except ImportError: + import ssl + _m2cEnabled = False try: # If failing to import old code, then try importing the legacy code @@ -1251,11 +1256,18 @@ def create_connected_socket(host, port, sslctx=None, timeout=None): + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) if sslctx: - sock = SSL.Connection(sslctx.context) - else: - sock = socket.socket(socket.AF_INET, - socket.SOCK_STREAM) + if _m2cEnabled: + sock = SSL.Connection(sslctx.context) + else: + sock = ssl.wrap_socket(sock, + keyfile=sslctx.key_file, + certfile=sslctx.cert_file, + server_side=False, + cert_reqs=ssl.CERT_REQUIRED, + ssl_version=sslctx.protocol, + ca_certs=sslctx.ca_certs) sock.settimeout(timeout) sock.connect((host, port)) return sock diff --git a/lib/vdsm/vdscli.py b/lib/vdsm/vdscli.py index a78a67a..cf083e3 100644 --- a/lib/vdsm/vdscli.py +++ b/lib/vdsm/vdscli.py @@ -26,7 +26,10 @@ import re import sys from xml.parsers.expat import ExpatError -from . import m2cutils +try: + from . import m2cutils as sslutils +except ImportError: + from . import sslutils _USE_SSL = False @@ -60,7 +63,7 @@ self.timeout = kwargs['timeout'] del kwargs['timeout'] else: - self.timeout = m2cutils.SOCKET_DEFAULT_TIMEOUT + self.timeout = sslutils.SOCKET_DEFAULT_TIMEOUT xmlrpclib.Transport.__init__(self, *args, **kwargs) @@ -107,8 +110,8 @@ def connect(hostPort=None, useSSL=None, tsPath=None, - TransportClass=m2cutils.VerifyingSafeTransport, - timeout=m2cutils.SOCKET_DEFAULT_TIMEOUT): + TransportClass=sslutils.VerifyingSafeTransport, + timeout=sslutils.SOCKET_DEFAULT_TIMEOUT): hostPort = cannonizeHostPort(hostPort) if useSSL is None: diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py index ee710f5..5dfa805 100644 --- a/lib/yajsonrpc/stompreactor.py +++ b/lib/yajsonrpc/stompreactor.py @@ -23,10 +23,14 @@ from vdsm import utils from vdsm.config import config from vdsm.compat import json -from vdsm.m2cutils import SSLSocket from . import JsonRpcClient, JsonRpcServer from . import stomp from .betterAsyncore import Dispatcher, Reactor +try: + from vdsm.m2cutils import SSLSocket +except ImportError: + from ssl import SSLSocket + _STATE_LEN = "Waiting for message length" _STATE_MSG = "Waiting for message" diff --git a/tests/crossImportsTests.py.in b/tests/crossImportsTests.py.in index a3e1d81..a997211 100644 --- a/tests/crossImportsTests.py.in +++ b/tests/crossImportsTests.py.in @@ -51,4 +51,7 @@ else: mods = get_mods(os.path.join(get_python_lib(), pkg_name)) + # ignore M2Crypto + mods.remove('m2cutils') + __import__(pkg_name, fromlist=mods) diff --git a/tests/integration/Makefile.am b/tests/integration/Makefile.am index 525ddd8..7fd327d 100644 --- a/tests/integration/Makefile.am +++ b/tests/integration/Makefile.am @@ -25,4 +25,5 @@ jsonRpcHelper.py \ jsonRpcTests.py \ m2chelper.py \ + sslhelper.py \ $(NULL) diff --git a/tests/integration/jsonRpcHelper.py b/tests/integration/jsonRpcHelper.py index 65ec5e3..fbb74c1 100644 --- a/tests/integration/jsonRpcHelper.py +++ b/tests/integration/jsonRpcHelper.py @@ -37,7 +37,10 @@ from protocoldetector import MultiProtocolAcceptor from rpc.bindingjsonrpc import BindingJsonRpc from vdsm import utils -from m2chelper import DEAFAULT_SSL_CONTEXT +try: + from integration.m2chelper import DEAFAULT_SSL_CONTEXT +except ImportError: + from integration.sslhelper import DEAFAULT_SSL_CONTEXT PERMUTATIONS = tuple(product((True, False), ("xml", "stomp"))) diff --git a/tests/integration/m2chelper.py b/tests/integration/m2chelper.py index 77a2767..7ece453 100644 --- a/tests/integration/m2chelper.py +++ b/tests/integration/m2chelper.py @@ -17,9 +17,11 @@ # # Refer to the README and COPYING files for full details of the license # - import os -from vdsm.m2cutils import SSLContext +import SimpleXMLRPCServer +import threading +from M2Crypto import SSL +from vdsm.m2cutils import SSLContext, SSLServerSocket CERT_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), '..') CRT_FILE = os.path.join(CERT_DIR, "server.crt") @@ -29,3 +31,43 @@ DEAFAULT_SSL_CONTEXT = SSLContext( CRT_FILE, KEY_FILE, session_id="server-tests") + + +def get_server_socket(key_file, cert_file, socket): + return SSLServerSocket(raw=socket, + keyfile=key_file, + certfile=cert_file, + ca_certs=cert_file) + + +class TestServer(): + + def __init__(self, host, service): + self.server = SimpleXMLRPCServer.SimpleXMLRPCServer((host, 0), + logRequests=False) + self.server.socket = SSLServerSocket(raw=self.server.socket, + keyfile=KEY_FILE, + certfile=CRT_FILE, + ca_certs=CRT_FILE) + _, self.port = self.server.socket.getsockname() + self.server.register_instance(service) + + def start(self): + self.thread = threading.Thread(target=self.serve_forever) + self.thread.daemon = True + self.thread.start() + + def serve_forever(self): + try: + self.server.serve_forever() + except SSL.SSLError: + # expected sslerror is thrown in server side during test_invalid + # method we do not want to pollute test console output + pass + + def stop(self): + self.server.shutdown() + + def get_timeout(self): + self.server.socket.accept_timeout = 1 + return self.server.socket.accept_timeout + 1 diff --git a/tests/integration/sslhelper.py b/tests/integration/sslhelper.py new file mode 100644 index 0000000..acc2516 --- /dev/null +++ b/tests/integration/sslhelper.py @@ -0,0 +1,82 @@ +# 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 os +import SimpleXMLRPCServer +import ssl +import threading +from vdsm.sslutils import SSLContext + +CERT_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), '..') +CRT_FILE = os.path.join(CERT_DIR, "server.crt") +KEY_FILE = os.path.join(CERT_DIR, "server.key") +OTHER_CRT_FILE = os.path.join(CERT_DIR, "other.crt") +OTHER_KEY_FILE = os.path.join(CERT_DIR, "other.key") + +DEAFAULT_SSL_CONTEXT = SSLContext(cert_file=CRT_FILE, key_file=KEY_FILE, + ca_certs=CRT_FILE) + + +def get_server_socket(key_file, cert_file, socket): + return ssl.wrap_socket(socket, + keyfile=key_file, + certfile=cert_file, + server_side=False, + cert_reqs=ssl.CERT_REQUIRED, + ssl_version=ssl.PROTOCOL_TLSv1, + ca_certs=cert_file) + + +class TestServer(SimpleXMLRPCServer.SimpleXMLRPCServer): + + def __init__(self, host, service): + SimpleXMLRPCServer.SimpleXMLRPCServer.__init__(self, (host, 0), + logRequests=False, + bind_and_activate=False) + + self.socket = ssl.wrap_socket(self.socket, + keyfile=KEY_FILE, + certfile=CRT_FILE, + server_side=True, + cert_reqs=ssl.CERT_REQUIRED, + ssl_version=ssl.PROTOCOL_TLSv1, + ca_certs=CRT_FILE, + do_handshake_on_connect=False) + + self.server_bind() + self.server_activate() + + _, self.port = self.socket.getsockname() + self.register_instance(service) + + def finish_request(self, request, client_address): + if self.timeout is not None: + request.settimeout(self.timeout) + + request.do_handshake() + + return SimpleXMLRPCServer.SimpleXMLRPCServer.finish_request( + self, + request, + client_address) + + def handle_error(self, request, client_address): + # ignored due to expected sslerrors when perorming plain connection + pass + + def start(self): + self.thread = threading.Thread(target=self.serve_forever) + self.thread.daemon = True + self.thread.start() + + def stop(self): + self.shutdown() + + def get_timeout(self): + self.timeout = 1 + return self.timeout + 1 diff --git a/tests/protocoldetectorTests.py b/tests/protocoldetectorTests.py index f90eb43..927b44a 100644 --- a/tests/protocoldetectorTests.py +++ b/tests/protocoldetectorTests.py @@ -26,10 +26,12 @@ from contextlib import contextmanager from yajsonrpc.betterAsyncore import Reactor -from vdsm import m2cutils from protocoldetector import MultiProtocolAcceptor -from integration.m2chelper import KEY_FILE, CRT_FILE from testlib import VdsmTestCase, expandPermutations, permutations +try: + from integration.m2chelper import KEY_FILE, CRT_FILE, DEAFAULT_SSL_CONTEXT +except: + from integration.sslhelper import KEY_FILE, CRT_FILE, DEAFAULT_SSL_CONTEXT class Detector(object): @@ -97,7 +99,7 @@ GRACETIME = 0.5 CONCURRENCY = 5 PERMUTATIONS = ((False,), (True,)) - SSLCTX = m2cutils.SSLContext(CRT_FILE, KEY_FILE, ca_cert=CRT_FILE) + SSLCTX = DEAFAULT_SSL_CONTEXT BUFSIZE = 512 def setUp(self): diff --git a/tests/sslTests.py b/tests/sslTests.py index b40f263..b7a4bfb 100644 --- a/tests/sslTests.py +++ b/tests/sslTests.py @@ -22,7 +22,6 @@ import errno import os import re -import SimpleXMLRPCServer import socket import ssl import subprocess @@ -31,12 +30,21 @@ import xmlrpclib from contextlib import contextmanager, closing -from M2Crypto import SSL -from integration.m2chelper import KEY_FILE, \ - CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE from testlib import VdsmTestCase as TestCaseBase -from vdsm.m2cutils import SSLServerSocket -from vdsm.m2cutils import VerifyingSafeTransport +from nose.plugins.skip import SkipTest +try: + from vdsm.m2cutils import VerifyingSafeTransport + from integration.m2chelper import TestServer, \ + get_server_socket, KEY_FILE, \ + CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE + _m2cEnabled = True +except ImportError: + from vdsm.sslutils import VerifyingSafeTransport + from integration.sslhelper import TestServer, \ + get_server_socket, KEY_FILE, \ + CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE + _m2cEnabled = False + HOST = '127.0.0.1' @@ -45,35 +53,6 @@ def add(self, x, y): return x + y - - -class TestServer(): - - def __init__(self): - self.server = SimpleXMLRPCServer.SimpleXMLRPCServer((HOST, 0), - logRequests=False) - self.server.socket = SSLServerSocket(raw=self.server.socket, - keyfile=KEY_FILE, - certfile=CRT_FILE, - ca_certs=CRT_FILE) - _, self.port = self.server.socket.getsockname() - self.server.register_instance(MathService()) - - def start(self): - self.thread = threading.Thread(target=self.serve_forever) - self.thread.daemon = True - self.thread.start() - - def serve_forever(self): - try: - self.server.serve_forever() - except SSL.SSLError: - # expected sslerror is thrown in server side during test_invalid - # method we do not want to pollute test console output - pass - - def stop(self): - self.server.shutdown() class VerifyingClient(): @@ -97,7 +76,7 @@ @contextmanager def verifyingclient(key_file, crt_file): - server = TestServer() + server = TestServer(HOST, MathService()) server.start() try: client = VerifyingClient(server.port, key_file, crt_file) @@ -112,9 +91,8 @@ class SocketTests(TestCaseBase): def test_block_socket(self): - server = TestServer() - server.server.socket.accept_timeout = 1 - timeout = server.server.socket.accept_timeout + 1 + server = TestServer(HOST, MathService()) + timeout = server.get_timeout() server.start() try: with closing(socket.socket(socket.AF_INET, @@ -222,11 +200,8 @@ # Create the server socket: self.server = socket.socket() - self.server = SSLServerSocket( - raw=self.server, - keyfile=self.keyfile, - certfile=self.certfile, - ca_certs=self.certfile) + self.server = get_server_socket(self.keyfile, self.certfile, + self.server) self.address = self.tryBind(ADDRESS) self.server.listen(5) @@ -357,6 +332,11 @@ Verify that SSL the session identifier is preserved when connecting two times without stopping the server. """ + if not _m2cEnabled: + raise SkipTest( + 'support for SSL sessions discontinued and may be reintroduced' + ' when we move to python 3' + ) # Create a temporary file to store the session details: sessionDetailsFile = tempfile.NamedTemporaryFile(delete=False) diff --git a/tests/stompTests.py b/tests/stompTests.py index 3b38c95..3eabbb7 100644 --- a/tests/stompTests.py +++ b/tests/stompTests.py @@ -26,9 +26,12 @@ dummyTextGenerator from integration.jsonRpcHelper import constructAcceptor -from integration.m2chelper import DEAFAULT_SSL_CONTEXT from yajsonrpc.stompreactor import StandAloneRpcClient -from vdsm.utils import running +from vdsm import utils +try: + from integration.m2chelper import DEAFAULT_SSL_CONTEXT +except ImportError: + from integration.sslhelper import DEAFAULT_SSL_CONTEXT CALL_TIMEOUT = 15 @@ -61,9 +64,11 @@ with constructAcceptor(self.log, use_ssl, _SampleBridge()) as acceptor: sslctx = DEAFAULT_SSL_CONTEXT if use_ssl else None - with running(StandAloneRpcClient(acceptor._host, acceptor._port, - 'jms.topic.vdsm_requests', - str(uuid4()), sslctx)) as client: + with utils.running(StandAloneRpcClient(acceptor._host, + acceptor._port, + 'jms.topic.vdsm_requests', + str(uuid4()), + sslctx)) as client: self.assertEquals(client.callMethod('echo', (data,), str(uuid4())), data) @@ -75,7 +80,6 @@ with constructAcceptor(self.log, use_ssl, _SampleBridge(), 'jms.queue.events') as acceptor: sslctx = DEAFAULT_SSL_CONTEXT if use_ssl else None - client = StandAloneRpcClient(acceptor._host, acceptor._port, 'jms.topic.vdsm_requests', 'jms.queue.events', sslctx, False) diff --git a/tests/vdscliTests.py b/tests/vdscliTests.py index ba2cf36..bfcbddf 100644 --- a/tests/vdscliTests.py +++ b/tests/vdscliTests.py @@ -29,9 +29,14 @@ from testlib import VdsmTestCase as TestCaseBase from testValidation import ValidateRunningAsRoot - -from vdsm import m2cutils from vdsm import vdscli +try: + from vdsm import m2cutils as sslutils + from integration.m2chelper import get_server_socket +except ImportError: + from vdsm import sslutils + from integration.sslhelper import get_server_socket + HOST = '127.0.0.1' @@ -51,11 +56,8 @@ if useSSL: KEY_FILE = os.path.join(path, 'keys/vdsmkey.pem') CRT_FILE = os.path.join(path, 'certs/vdsmcert.pem') - self.server.socket = m2cutils.SSLServerSocket( - raw=self.server.socket, - keyfile=KEY_FILE, - certfile=CRT_FILE, - ca_certs=CRT_FILE) + self.server.socket = get_server_socket(KEY_FILE, CRT_FILE, + self.server.socket) _, self.port = self.server.socket.getsockname() self.server.register_instance(TestingService()) @@ -74,7 +76,7 @@ @contextmanager def setupclient(useSSL, tsPath, - timeout=m2cutils.SOCKET_DEFAULT_TIMEOUT): + timeout=sslutils.SOCKET_DEFAULT_TIMEOUT): server = TestServer(useSSL, tsPath) server.start() hostPort = '0:' + str(server.port) diff --git a/vdsm.spec.in b/vdsm.spec.in index ddb3184..7bcea09 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -42,6 +42,9 @@ %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d %endif +# enable m2crypto by default +%global with_m2c 1 + # Gluster should not be shipped with RHEV %if ! 0%{?rhev_build} %global with_gluster 1 @@ -83,7 +86,9 @@ BuildRequires: libnl3 BuildRequires: libselinux-python BuildRequires: libvirt-python +%if %{with_m2c} BuildRequires: m2crypto +%endif BuildRequires: mom >= 0.4.5 BuildRequires: openssl BuildRequires: policycoreutils-python @@ -129,7 +134,9 @@ Requires: %{name}-infra = %{version}-%{release} Requires: rpm-python Requires: nfs-utils +%if %{with_m2c} Requires: m2crypto +%endif Requires: libnl3 Requires: curl Requires: %{name}-xmlrpc = %{version}-%{release} @@ -295,7 +302,9 @@ %package reg Summary: VDSM registration package Requires: %{name} = %{version}-%{release} +%if %{with_m2c} Requires: m2crypto +%endif Requires: openssl Conflicts: ovirt-node < 3.0.4 @@ -307,7 +316,9 @@ Summary: VDSM python libraries Requires: %{name}-infra = %{version}-%{release} Requires: python-cpopen >= 1.2.3-5 +%if %{with_m2c} Requires: m2crypto +%endif Requires: python-ioprocess >= 0.14 %description python @@ -1049,7 +1060,6 @@ %{python_sitelib}/%{vdsm_name}/ipwrapper.py* %{python_sitelib}/%{vdsm_name}/jsonrpcvdscli.py* %{python_sitelib}/%{vdsm_name}/libvirtconnection.py* -%{python_sitelib}/%{vdsm_name}/m2cutils.py* %{python_sitelib}/%{vdsm_name}/netinfo.py* %{python_sitelib}/%{vdsm_name}/netlink/__init__.py* %{python_sitelib}/%{vdsm_name}/netlink/addr.py* @@ -1067,6 +1077,13 @@ %{python_sitelib}/%{vdsm_name}/response.py* %{python_sitelib}/%{vdsm_name}/netconfpersistence.py* %{python_sitelib}/%{vdsm_name}/schedule.py* +%if %{with_m2c} +%{python_sitelib}/%{vdsm_name}/m2cutils.py* +%exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* +%else +%exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* +%{python_sitelib}/%{vdsm_name}/sslutils.py* +%endif %{python_sitelib}/%{vdsm_name}/sysctl.py* %{python_sitelib}/%{vdsm_name}/udevadm.py* %{python_sitelib}/%{vdsm_name}/utils.py* diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py index 5590db0..578b349 100644 --- a/vdsm/clientIF.py +++ b/vdsm/clientIF.py @@ -37,7 +37,6 @@ from vdsm.compat import pickle from vdsm.define import doneCode, errCode import libvirt -from vdsm import m2cutils from vdsm import libvirtconnection from vdsm import constants from vdsm import utils @@ -55,6 +54,10 @@ from virt.vmchannels import Listener from virt.vmdevices import hwclass from virt.utils import isVdsmImage +try: + from vdsm import m2cutils as sslutils +except ImportError: + from vdsm import sslutils try: import gluster.api as gapi _glusterEnabled = True @@ -193,7 +196,7 @@ return cls._instance def _createAcceptor(self, host, port): - sslctx = m2cutils.create_ssl_context() + sslctx = sslutils.create_ssl_context() self._reactor = Reactor() self._acceptor = MultiProtocolAcceptor(self._reactor, host, @@ -205,7 +208,7 @@ broker_port = config.getint('addresses', 'broker_port') request_queues = config.get('addresses', 'request_queues') - sslctx = m2cutils.create_ssl_context() + sslctx = sslutils.create_ssl_context() sock = socket.socket() sock.connect((broker_address, broker_port)) if sslctx: diff --git a/vdsm/kaxmlrpclib.py b/vdsm/kaxmlrpclib.py index b38031e..b0640e8 100644 --- a/vdsm/kaxmlrpclib.py +++ b/vdsm/kaxmlrpclib.py @@ -110,7 +110,10 @@ ################### # the same, for ssl -from vdsm import m2cutils +try: + from vdsm import m2cutils as sslutils +except ImportError: + from vdsm import sslutils import ssl @@ -122,7 +125,7 @@ SslServerProxy = SslServer -class TcpkeepSafeTransport(m2cutils.VerifyingSafeTransport): +class TcpkeepSafeTransport(sslutils.VerifyingSafeTransport): def make_connection(self, host): chost, self._extra_headers, x509 = self.get_host_info(host) @@ -139,17 +142,17 @@ cert_reqs=self.cert_reqs) -class TcpkeepHTTPSConnection(m2cutils.VerifyingHTTPSConnection): +class TcpkeepHTTPSConnection(sslutils.VerifyingHTTPSConnection): def __init__(self, host, port=None, key_file=None, cert_file=None, strict=None, timeout=CONNECTTIMEOUT, ca_certs=None, cert_reqs=ssl.CERT_REQUIRED): - m2cutils.VerifyingHTTPSConnection.__init__( + sslutils.VerifyingHTTPSConnection.__init__( self, host, port=port, key_file=key_file, cert_file=cert_file, strict=strict, timeout=timeout, ca_certs=ca_certs, cert_reqs=cert_reqs) def connect(self): - m2cutils.VerifyingHTTPSConnection.connect(self) + sslutils.VerifyingHTTPSConnection.connect(self) # after TCP_KEEPIDLE seconds of silence, TCP_KEEPCNT probes would be # sent, TCP_KEEPINTVL seconds apart of each other. If all of them fail, @@ -160,5 +163,5 @@ self.sock.setsockopt(socket.SOL_TCP, socket.TCP_KEEPCNT, KEEPCNT) -class TcpkeepHTTPS(m2cutils.VerifyingHTTPS): +class TcpkeepHTTPS(sslutils.VerifyingHTTPS): _connection_class = TcpkeepHTTPSConnection diff --git a/vdsm/protocoldetector.py b/vdsm/protocoldetector.py index 6d01a1c..dbdc730 100644 --- a/vdsm/protocoldetector.py +++ b/vdsm/protocoldetector.py @@ -24,7 +24,10 @@ import vdsm.infra.filecontrol as filecontrol from vdsm.utils import monotonic_time -from vdsm.m2cutils import SSLHandshakeDispatcher +try: + from vdsm.m2cutils import SSLHandshakeDispatcher +except ImportError: + from vdsm.sslutils import SSLHandshakeDispatcher def _create_socket(host, port): diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index 32caccb..6c41114 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -28,7 +28,6 @@ from vdsm import utils from vdsm import vdscli from vdsm import jsonrpcvdscli -from vdsm import m2cutils from vdsm.compat import pickle from vdsm.config import config from vdsm.define import NORMAL, Mbytes @@ -40,6 +39,11 @@ from . import vmexitreason from . import vmstatus +try: + from vdsm import m2cutils as sslutils +except ImportError: + from vdsm import sslutils + MODE_REMOTE = 'remote' MODE_FILE = 'file' -- To view, visit https://gerrit.ovirt.org/44494 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

8 82

Change in vdsm[ovirt-3.5]: lvm: Check if device in VG before extend
by frolland＠redhat.com 15 Oct '15

15 Oct '15

Hello Fred Rolland, Allon Mureinik, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/46651 to review the following change. Change subject: lvm: Check if device in VG before extend ...................................................................... lvm: Check if device in VG before extend Before extending a VG, VDSM needs to check that none of the additional devices are not already part of the VG. Without this protection, LVM will create a new PV with the same device. After extending VG, its metadata will updated with new PV. As a result, the VG will be partial. pvs output after creating new PV with same device: # pvs WARNING: Device for PV 2fDjgH-TeKA-1Vy2-NMMv-GB9n-op0w-g2Nyrd not found or rejected by a filter. PV /dev/mapper/360014054bf6dd7d14fd40c9ac3b5dbe8 unknown device Change-Id: Ifec2503094d6a0ecdd32e5e62f9c01a1469145f3 Backport-To: 3.6 Bug-Url: https://bugzilla.redhat.com/1265907 Signed-off-by: Fred Rolland <frolland(a)redhat.com> Reviewed-on: https://gerrit.ovirt.org/45946 Continuous-Integration: Jenkins CI Reviewed-by: Allon Mureinik <amureini(a)redhat.com> Reviewed-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/storage/lvm.py 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/46651/1 diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py index aa3c04b..7f2adf1 100644 --- a/vdsm/storage/lvm.py +++ b/vdsm/storage/lvm.py @@ -962,6 +962,13 @@ pvs = [_fqpvname(pdev) for pdev in _normalizeargs(devices)] _checkpvsblksize(pvs, getVGBlockSizes(vgName)) vg = _lvminfo.getVg(vgName) + + member_pvs = set(vg.pv_name).intersection(pvs) + if member_pvs: + log.error("Cannot extend vg %s: pvs already belong to vg %s", + vg.name, member_pvs) + raise se.VolumeGroupExtendError(vgName, pvs) + # Format extension PVs as all the other already in the VG _initpvs(pvs, int(vg.vg_mda_size) / 2 ** 20, force) -- To view, visit https://gerrit.ovirt.org/46651 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ifec2503094d6a0ecdd32e5e62f9c01a1469145f3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com> Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com> Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>

5 12

Change in vdsm[ovirt-3.5]: net: bonding: validate options based on pre-dumped ones
by phoracek＠redhat.com 15 Oct '15

15 Oct '15

Hello Ondřej Svoboda, Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/43148 to review the following change. Change subject: net: bonding: validate options based on pre-dumped ones ...................................................................... net: bonding: validate options based on pre-dumped ones The removed code created a bond device only to check if the provided options are recongnized by the kernel. This was buggy, as the options depends on the bonding mode. It was also redundant, as we already depend on pre-dumped maps of bond options per bond mode, so we can (and should) use it here, too. Change-Id: I03fce903280d5eda817e713a51c8079dab9d7649 Signed-off-by: Dan Kenigsberg <danken(a)redhat.com> Reviewed-on: https://gerrit.ovirt.org/41616 Continuous-Integration: Jenkins CI Reviewed-by: Ondřej Svoboda <osvoboda(a)redhat.com> Reviewed-by: Petr Horáček <phoracek(a)redhat.com> Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1234867 --- M tests/netmodelsTests.py M vdsm/network/api.py M vdsm/network/models.py 3 files changed, 58 insertions(+), 41 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/43148/1 diff --git a/tests/netmodelsTests.py b/tests/netmodelsTests.py index a2a3ee3..548f01b 100644 --- a/tests/netmodelsTests.py +++ b/tests/netmodelsTests.py @@ -28,8 +28,6 @@ from network.models import _nicSort from testrunner import VdsmTestCase as TestCaseBase -from testValidation import ValidateRunningAsRoot -from nose.plugins.skip import SkipTest from monkeypatch import MonkeyPatch @@ -80,18 +78,17 @@ self.assertEqual(Bond.validateName('bond11'), None) self.assertEqual(Bond.validateName('bond11128421982'), None) - @ValidateRunningAsRoot + @MonkeyPatch(netinfo, 'BONDING_DEFAULTS', netinfo.BONDING_DEFAULTS + if os.path.exists(netinfo.BONDING_DEFAULTS) + else '../vdsm/bonding-defaults.json') def testValidateBondingOptions(self): - if not os.path.exists(netinfo.BONDING_MASTERS): - raise SkipTest("bonding kernel module could not be found.") - opts = 'mode=802.3ad miimon=150' badOpts = 'foo=bar badopt=one' with self.assertRaises(errors.ConfigNetworkError) as cne: - Bond.validateOptions('bond0', badOpts) + Bond.validateOptions(badOpts) self.assertEqual(cne.exception.errCode, errors.ERR_BAD_BONDING) - self.assertEqual(Bond.validateOptions('bond0', opts), None) + self.assertEqual(Bond.validateOptions(opts), None) def testIsIpValid(self): addresses = ('10.18.1.254', '10.50.25.177', '250.0.0.1', @@ -122,7 +119,6 @@ self.assertEqual(IPv4.validateNetmask(mask), None) @MonkeyPatch(netinfo, 'getMtu', lambda *x: 1500) - @MonkeyPatch(Bond, 'validateOptions', lambda *x: 0) def testTextualRepr(self): _netinfo = {'networks': {}, 'vlans': {}, 'nics': ['testnic1', 'testnic2'], diff --git a/vdsm/network/api.py b/vdsm/network/api.py index f829732..52bbf45 100755 --- a/vdsm/network/api.py +++ b/vdsm/network/api.py @@ -470,7 +470,7 @@ for bonding, bondingAttrs in bondings.iteritems(): Bond.validateName(bonding) if 'options' in bondingAttrs: - Bond.validateOptions(bonding, bondingAttrs['options']) + Bond.validateOptions(bondingAttrs['options']) if bondingAttrs.get('remove', False): continue diff --git a/vdsm/network/models.py b/vdsm/network/models.py index 8e33950..9608937 100644 --- a/vdsm/network/models.py +++ b/vdsm/network/models.py @@ -17,9 +17,7 @@ # Refer to the README and COPYING files for full details of the license # from collections import namedtuple -from contextlib import contextmanager import logging -import os import re import socket import struct @@ -179,8 +177,11 @@ for slave in slaves: slave.master = self self.slaves = slaves - self.options = options or '' - self.validateOptions(name, self.options) + if options is None: + self.options = 'mode=802.3ad miimon=150' + else: + self.validateOptions(options) + self.options = self._reorderOptions(options) self.destroyOnMasterRemoval = destroyOnMasterRemoval super(Bond, self).__init__(name, configurator, ipconfig, mtu) @@ -270,36 +271,56 @@ name) @classmethod - def validateOptions(cls, bonding, bondingOptions): + def validateOptions(cls, bondingOptions): 'Example: BONDING_OPTS="mode=802.3ad miimon=150"' - with cls._validationBond(bonding) as bond: - try: - for option in bondingOptions.split(): - key, _ = option.split('=') - if not os.path.exists('/sys/class/net/%s/bonding/%s' % - (bond, key)): - raise ConfigNetworkError(ne.ERR_BAD_BONDING, '%r is ' - 'not a valid bonding option' % - key) - except ValueError: - raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Error parsing ' - 'bonding options: %r' % - bondingOptions) + mode = 'balance-rr' + try: + for option in bondingOptions.split(): + key, value = option.split('=', 1) + if key == 'mode': + mode = value + except ValueError: + raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Error parsing ' + 'bonding options: %r' % bondingOptions) + + MODE_NAME_TO_NUMBER = { + 'balance-rr': '0', + 'active-backup': '1', + 'balance-xor': '2', + 'broadcast': '3', + '802.3ad': '4', + 'balance-tlb': '5', + 'balance-alb': '6', + } + + if mode in MODE_NAME_TO_NUMBER: + mode = MODE_NAME_TO_NUMBER[mode] + defaults = netinfo.getDefaultBondingOptions(mode) + + for option in bondingOptions.split(): + key, _ = option.split('=', 1) + if key not in defaults: + raise ConfigNetworkError(ne.ERR_BAD_BONDING, '%r is not a ' + 'valid bonding option' % key) @staticmethod - @contextmanager - def _validationBond(bonding): - bond_created = False - try: - bonding = open(netinfo.BONDING_MASTERS, 'r').read().split()[0] - except IndexError: - open(netinfo.BONDING_MASTERS, 'w').write('+%s\n' % bonding) - bond_created = True - try: - yield bonding - finally: - if bond_created: - open(netinfo.BONDING_MASTERS, 'w').write('-%s\n' % bonding) + def _reorderOptions(options): + """Order the mode first and the rest of options alphabetically.""" + if not options.strip(): + return '' + + opts = dict((option.split('=', 1) for option in options.split())) + + mode = opts.pop('mode', None) + opts = sorted(opts.iteritems()) + if mode: + opts.insert(0, ('mode', mode)) + + return ' '.join((opt + '=' + val for (opt, val) in opts)) + + @property + def backing_device(self): + return True class IPv4(object): -- To view, visit https://gerrit.ovirt.org/43148 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I03fce903280d5eda817e713a51c8079dab9d7649 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com>

2 6

Change in vdsm[ovirt-3.5]: network: allow custom bondOption
by phoracek＠redhat.com 15 Oct '15

15 Oct '15

Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/43149 to review the following change. Change subject: network: allow custom bondOption ...................................................................... network: allow custom bondOption Allow 'custom' in bond options. This value will be accepted by Bond.validateOptions method but it will not be passed to configurator. Thanks to this patch, users will be allowed to pass custom properties to setupNetworks hooks. This path was reverted in past because of malfunction, now it should work, there is a new test for sure. Change-Id: I9ace532d959bc3195a8a92b4536bdc0062bc7d1d Signed-off-by: Petr Horáček <phoracek(a)redhat.com> Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1234867 Reviewed-on: https://gerrit.ovirt.org/40882 Reviewed-by: Dan Kenigsberg <danken(a)redhat.com> Continuous-Integration: Dan Kenigsberg <danken(a)redhat.com> --- M lib/vdsm/netinfo.py M tests/functional/networkTests.py M vdsm/network/models.py 3 files changed, 37 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/43149/1 diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index 01f25c7..987a40c 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -73,8 +73,9 @@ _BONDING_FAILOVER_MODES = frozenset(('1', '3')) _BONDING_LOADBALANCE_MODES = frozenset(('0', '2', '4', '5', '6')) _EXCLUDED_BONDING_ENTRIES = frozenset(( - 'slaves', 'active_slave', 'mii_status', 'queue_id', 'ad_aggregator', - 'ad_num_ports', 'ad_actor_key', 'ad_partner_key', 'ad_partner_mac' + 'slaves', 'active_slave', 'mii_status', 'queue_id', 'ad_aggregator', + 'ad_num_ports', 'ad_actor_key', 'ad_partner_key', 'ad_partner_mac', + 'custom' )) _IFCFG_ZERO_SUFFIXED = frozenset( ('IPADDR0', 'GATEWAY0', 'PREFIX0', 'NETMASK0')) diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 37979ba..3a3d6f9 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -2337,3 +2337,22 @@ self.assertNetworkDoesntExist(NETWORK_NAME) self.assertBondDoesntExist(BONDING_NAME, [nic]) self.vdsm_net.save_config() + + @cleanupNet + @ValidateRunningAsRoot + def test_setupNetworks_bond_with_custom_option(self): + with dummyIf(2) as nics: + status, msg = self.setupNetworks( + {}, + {BONDING_NAME: {'nics': nics, + 'options': 'custom=foo mode=balance-rr'}}, + NOCHK, test_kernel_config=False) + self.assertEqual(status, SUCCESS, msg) + self.assertBondExists(BONDING_NAME, nics) + opts = self.vdsm_net.config.bonds.get(BONDING_NAME).get('options') + self.assertEquals(opts, 'mode=balance-rr') + + status, msg = self.setupNetworks( + {}, {BONDING_NAME: {'remove': True}}, NOCHK) + self.assertEqual(status, SUCCESS, msg) + self.assertBondDoesntExist(BONDING_NAME, nics) diff --git a/vdsm/network/models.py b/vdsm/network/models.py index 9608937..aacf462 100644 --- a/vdsm/network/models.py +++ b/vdsm/network/models.py @@ -235,9 +235,23 @@ _netinfo=_netinfo)) return slaves + @staticmethod + def remove_custom_option(options): + """ 'custom' property should not be exposed to configurator, it is + only for purpose of hooks """ + d_opts = dict(opt.split('=', 1) for opt in options.split()) + if d_opts.pop('custom', None) is not None: + options = ' '.join(['%s=%s' % (key, value) + for key, value in d_opts.items()]) + return options + @classmethod def objectivize(cls, name, configurator, options, nics, mtu, _netinfo, destroyOnMasterRemoval=None): + + if options: + options = cls.remove_custom_option(options) + if nics: # New bonding or edit bonding. slaves = cls._objectivizeSlaves(name, configurator, _nicSort(nics), mtu, _netinfo) @@ -299,7 +313,7 @@ for option in bondingOptions.split(): key, _ = option.split('=', 1) - if key not in defaults: + if key not in defaults and key != 'custom': raise ConfigNetworkError(ne.ERR_BAD_BONDING, '%r is not a ' 'valid bonding option' % key) -- To view, visit https://gerrit.ovirt.org/43149 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9ace532d959bc3195a8a92b4536bdc0062bc7d1d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>

3 9

← Newer
1
...
19
20
21
22
23
24
25
...
67
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches September 2015