From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: tool: clear networks configured by VDSM
......................................................................
Patch Set 4:
(2 comments)
https://gerrit.ovirt.org/#/c/79495/4//COMMIT_MSG
Commit Message:
PS4, Line 10: but default route
> So if the management is not really with the default route, then we loose th
ack for a compulsory argument. When it is not supplied by the user, the error message prints the default route network as a suggestion.
https://gerrit.ovirt.org/#/c/79495/4/lib/vdsm/tool/network.py
File lib/vdsm/tool/network.py:
PS4, Line 104: net_api.network_caps()
> Why can't you use just the caps? This will reduce the need to depend on the
we'd like to keep the management network, but remove its bridge in case it exists; if we do not use KernelConfig, we'd have to reimplement some of it here.
--
To view, visit https://gerrit.ovirt.org/79495
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I63702da4dd1c5a0328797f524734813ef251bdb6
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horacek <phoracek(a)redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: password: Move to vdsm.common
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.ovirt.org/79700
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If3e996c02acc76e3bd15c3aa656f7341c82c1e6f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: hooking: move to lib
......................................................................
Patch Set 7:
(2 comments)
https://gerrit.ovirt.org/#/c/79638/7/tests/run_tests_local.sh.in
File tests/run_tests_local.sh.in:
Line 23
Line 24
Line 25
Line 26
Line 27
why is `client` related and removed?
https://gerrit.ovirt.org/#/c/79638/7/vdsm.spec.in
File vdsm.spec.in:
PS7, Line 732: PYLIBDIR
> I do not like it but if we want to have this module in lib/vdsm I do not se
I'd keep it completely out of the constants.py junkyard. Howe about taking
os.path.dirname(vdsm.hook.__file__)
whereever the path is needed? (frankly, I think that there's only a single user, in hooks.py)
--
To view, visit https://gerrit.ovirt.org/79638
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If2f1581d75cfb33868af41d1b116f56af89801a5
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: m2c: uses different protocol constant names
......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/79664/2//COMMIT_MSG
Commit Message:
Line 9: M2crypto uses the same name scheme as openssl which differs from ssl
Line 10: module. SSL_OP_NO_TLSv1_1 is not available on some of m2c versions.
Line 11:
Line 12:
Line 13: Label: ovirt-4.1-only
> m2c was removed from master
i know. i would write it here
Line 14: Bug-Url: https://bugzilla.redhat.com/1473344
Line 15: Change-Id: I6c05bddfba94d85e2c48122b6fd56d8f336d01f5
--
To view, visit https://gerrit.ovirt.org/79664
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c05bddfba94d85e2c48122b6fd56d8f336d01f5
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.1
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: handle handshake errors
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/79668/1/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:
Line 187: self._handshake(dispatcher)
Line 188: except socket.error:
Line 189: self.log.error("Handshake failed")
Line 190: dispatcher.socket.close()
Line 191: return
seems like _handshake itself takes care of exceptions.. if you need to handle socker.error somewhere its there and not here
Line 192: else:
Line 193: if config.getboolean('vars', 'verify_client_cert'):
Line 194: if not self._verify_host(dispatcher.socket.getpeercert(),
Line 195: dispatcher.socket.getpeername()[0]):
--
To view, visit https://gerrit.ovirt.org/79668
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I99cfa35e608f429640455c35495be1783854e3da
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: m2c: uses different protocol constant names
......................................................................
Patch Set 2: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/79664/2//COMMIT_MSG
Commit Message:
Line 9: M2crypto uses the same name scheme as openssl which differs from ssl
Line 10: module. SSL_OP_NO_TLSv1_1 is not available on some of m2c versions.
Line 11:
Line 12:
Line 13: Label: ovirt-4.1-only
why is it 4.1 only?
Line 14: Bug-Url: https://bugzilla.redhat.com/1473344
Line 15: Change-Id: I6c05bddfba94d85e2c48122b6fd56d8f336d01f5
--
To view, visit https://gerrit.ovirt.org/79664
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c05bddfba94d85e2c48122b6fd56d8f336d01f5
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.1
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has posted comments on this change.
Change subject: m2c: uses different protocol constant names
......................................................................
Patch Set 2:
(2 comments)
https://gerrit.ovirt.org/#/c/79664/2/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:
Line 237: ('ssl_excludes', '',
Line 238: 'Comma separated protocol values as defined in '
Line 239: 'https://docs.python.org/2/library/ssl.html. '
Line 240: 'e.g. for ssl OP_NO_TLSv1, OP_NO_TLSv1_1 '
Line 241: ' for m2crypto SSL_OP_NO_TLSv1, SSL_OP_NO_TLSv1_1 '
funny.. just except both and do the conversion yourself based on the implementation you use. it will make the user's life easier
Line 242: 'By default tlv1, tlsv1.1 and tlsv1.2 are '
Line 243: 'enabled.'),
Line 244:
Line 245: ('connection_stats_timeout', '3600',
https://gerrit.ovirt.org/#/c/79664/2/lib/vdsm/m2cutils.py
File lib/vdsm/m2cutils.py:
Line 336: # not all m2crypto provide this constant
Line 337: excludes |= 268435456
Line 338: else:
Line 339: excludes |= getattr(m2, no_protocol.strip())
Line 340:
you are calling no_protocol.strip() in the condition, so the attr probably exists.. what constant m2crypto is not provided?
--
To view, visit https://gerrit.ovirt.org/79664
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c05bddfba94d85e2c48122b6fd56d8f336d01f5
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.1
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes