Yaniv Bronhaim has posted comments on this change.

Change subject: ssl: configurable implementation
......................................................................


Patch Set 10: Code-Review-1

(8 comments)

https://gerrit.ovirt.org/#/c/44494/10//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-09-15 14:47:04 +0200
Line 6: 
Line 7: ssl: configurable implementation
Line 8: 
Line 9: We want to choose ssl implementation during build process.
both lines can merge to - adding ssl implementation config value. and the 
commit message should explain the two options and why we need it (which you 
explained in the following patch)
Line 10: 
Line 11: Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823


https://gerrit.ovirt.org/#/c/44494/10/debian/vdsm-python.install
File debian/vdsm-python.install:

Line 16: ./
you still have this m2cutils.py if im not wrong :/


https://gerrit.ovirt.org/#/c/44494/10/lib/vdsm/Makefile.am
File lib/vdsm/Makefile.am:

Line 32:        executor.py \
Line 33:        ipwrapper.py \
Line 34:        jsonrpcvdscli.py \
Line 35:        libvirtconnection.py \
Line 36:        m2cutils.py \
you left it here.. err how did I miss the m2c shortcut . with 
m2cryptoUtils\m2cryptoWrapper you can at least guess what it is
Line 37:        netconfpersistence.py \
Line 38:        netinfo.py \
Line 39:        password.py \
Line 40:        ppc64HardwareInfo.py \


https://gerrit.ovirt.org/#/c/44494/10/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:

Line 1: #
Line 2: # Copyright 2014 Red Hat, Inc.
2015

did you change something in this file?
Line 3: #
Line 4: # This program is free software; you can redistribute it and/or modify
Line 5: # it under the terms of the GNU General Public License as published by
Line 6: # the Free Software Foundation; either version 2 of the License, or


https://gerrit.ovirt.org/#/c/44494/10/tests/integration/m2chelper.py
File tests/integration/m2chelper.py:

Line 43: 
Line 44: class TestServer():
Line 45: 
Line 46:     def __init__(self, host, service):
Line 47:         self.server = SimpleXMLRPCServer.SimpleXMLRPCServer((host, 0),
why not jsonrpc?
Line 48:                                                             
logRequests=False)
Line 49:         self.server.socket = SSLServerSocket(raw=self.server.socket,
Line 50:                                              keyfile=KEY_FILE,
Line 51:                                              certfile=CRT_FILE,


https://gerrit.ovirt.org/#/c/44494/10/tests/sslTests.py
File tests/sslTests.py:

Line 42:     from vdsm.sslutils import VerifyingSafeTransport
Line 43:     from integration.sslhelper import TestServer, \
Line 44:         get_server_socket, KEY_FILE, \
Line 45:         CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE
Line 46:     _m2cEnabled = False
don't you want to test both?
Line 47: 
Line 48: 
Line 49: HOST = '127.0.0.1'
Line 50: 


https://gerrit.ovirt.org/#/c/44494/10/tests/vdscliTests.py
File tests/vdscliTests.py:

Line 34:     from vdsm import m2cutils as sslutils
Line 35:     from integration.m2chelper import get_server_socket
Line 36: except ImportError:
Line 37:     from vdsm import sslutils
Line 38:     from integration.sslhelper import get_server_socket
don't you want to test both?
Line 39: 
Line 40: 
Line 41: HOST = '127.0.0.1'
Line 42: 


https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in
File vdsm.spec.in:

Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py*
Line 1073: %if %{with_m2c}
Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py*
Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py*
Line 1076: %else
why do you need to exclude it?
Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py*
Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py*
Line 1079: %endif
Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Simone Tiraboschi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to