Change in vdsm[master]: ssl: runtime config to choose implementation

piotr.kliczewski at gmail.com piotr.kliczewski at gmail.com
Sun Sep 20 05:32:28 UTC 2015


Piotr Kliczewski has posted comments on this change.

Change subject: ssl: runtime config to choose implementation
......................................................................


Patch Set 9:

(5 comments)

https://gerrit.ovirt.org/#/c/44689/9/lib/vdsm/sslcompat.py
File lib/vdsm/sslcompat.py:

Line 27: if config.get('vars', 'ssl_implementation') == 'm2c':
Line 28:     try:
Line 29:         from . import m2cutils as sslutils
Line 30:         from .m2cutils import SSLSocket
Line 31:         from .m2cutils import SSLHandshakeDispatcher
> Please keep the same order of names everywhere.
Done
Line 32:         _USED_M2C = True
Line 33:     except ImportError:
Line 34:         _log.warning('Configured m2crypto but not available falling back'
Line 35:                      ' to ssl')


Line 28:     try:
Line 29:         from . import m2cutils as sslutils
Line 30:         from .m2cutils import SSLSocket
Line 31:         from .m2cutils import SSLHandshakeDispatcher
Line 32:         _USED_M2C = True
> Not needed if we fail here.
We need it as described below.
Line 33:     except ImportError:
Line 34:         _log.warning('Configured m2crypto but not available falling back'
Line 35:                      ' to ssl')
Line 36:     else:


Line 29:         from . import m2cutils as sslutils
Line 30:         from .m2cutils import SSLSocket
Line 31:         from .m2cutils import SSLHandshakeDispatcher
Line 32:         _USED_M2C = True
Line 33:     except ImportError:
> If you configure m2c, and m2c is not available, vdsm should fail to start, 
There can be difference in how we built vdsm and how we configured.

When we built without m2c and still have this configuration here we would fail. Whereas we should gracefully handle misconfiguration.
Line 34:         _log.warning('Configured m2crypto but not available falling back'
Line 35:                      ' to ssl')
Line 36:     else:
Line 37:         # we need it to satisfy pyflakes


Line 36:     else:
Line 37:         # we need it to satisfy pyflakes
Line 38:         sslutils
Line 39:         SSLHandshakeDispatcher
Line 40:         SSLSocket
> Can we satisfy pyflakes by mentioning these once at the end?
I tried but it does not like it.
Line 41: 
Line 42: if not _USED_M2C:
Line 43:     from . import sslutils
Line 44:     from .sslutils import SSLHandshakeDispatcher


Line 38:         sslutils
Line 39:         SSLHandshakeDispatcher
Line 40:         SSLSocket
Line 41: 
Line 42: if not _USED_M2C:
> Can be just else:
We need it as described above
Line 43:     from . import sslutils
Line 44:     from .sslutils import SSLHandshakeDispatcher
Line 45:     from ssl import SSLSocket
Line 46: 


-- 
To view, visit https://gerrit.ovirt.org/44689
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9881d11e30ced9c34bfe602bba3d968f57e0fe15
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stirabos at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list