Change in vdsm[master]: ssl: enable tlsv1.2 support
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: ssl: enable tlsv1.2 support
......................................................................
Patch Set 30: Code-Review-1
(3 comments)
https://gerrit.ovirt.org/#/c/72284/30/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:
PS30, Line 271: int
I was expecting this to return an int.
you chose a list (/me does not know why) but the name of the function does not match.
Line 272: protocols = []
Line 273:
Line 274: for no_protocol in config.get('vars', 'ssl_protocol').split(','):
Line 275: if no_protocol != '':
Line 276: protocols.append(getattr(ssl, no_protocol.strip()))
in commit 181bca90e15e44ec you introduced ssl_protocol. if someone had used it, we should not let them explode.
Line 277:
https://gerrit.ovirt.org/#/c/72284/30/lib/yajsonrpc/stompreactor.py
File lib/yajsonrpc/stompreactor.py:
Line 629: from vdsm.sslutils import SSLContext
Line 630: sslctx = SSLContext(key_file=constants.KEY_FILE,
Line 631: cert_file=constants.CERT_FILE,
Line 632: ca_certs=constants.CA_FILE,
Line 633: protocol=CLIENT_PROTOCOL)
you've added this as a default, so why bother setting all the users?
Line 634: return StandAloneRpcClient(host, port, "jms.topic.vdsm_requests",
Line 635: str(uuid4()), sslctx, lazy_start=False)
Line 636:
Line 637:
--
To view, visit https://gerrit.ovirt.org/72284
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc83d8cc08349a0ca47ed7d021bb3fc6d1408a6
Gerrit-PatchSet: 30
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: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Peřina <mperina(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
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
7 years
Change in vdsm[master]: net: dump bonding options on vdsmd start
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: net: dump bonding options on vdsmd start
......................................................................
Patch Set 1: Code-Review-1
We must stop shipping with the defaults file.
Add motivation to commit message
--
To view, visit https://gerrit.ovirt.org/78006
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I98f7c691a2a86d152ebcb81e1d96f9b0d83fa698
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years
Change in vdsm[master]: vm: create a vnic with vhostuser backend
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: vm: create a vnic with vhostuser backend
......................................................................
Patch Set 31:
(1 comment)
https://gerrit.ovirt.org/#/c/74327/31/lib/vdsm/virt/vmdevices/network.py
File lib/vdsm/virt/vmdevices/network.py:
Line 303: detach_detachable(self.hostdev)
Line 304: else:
Line 305: bridge_info = supervdsm.getProxy().ovs_bridge(self.network)
Line 306: if bridge_info and bridge_info['dpdk_enabled']:
Line 307: self._is_vhostuser = True
> This becomes too complicated.
It's sad that we need to store data just in order not to bother supervdsm during __init__. But yes, it's my option 1.
I kinda likeMartin's suggestion. in vm.py we can have
if not self.recovering:
self._setup_devices()
else:
self._recover_devices() # <== add this
which should call a device.recover() which would speak with supervdsm.
Line 308: self._create_vhost_port(bridge_info['name'])
Line 309:
Line 310: def teardown(self):
Line 311: if self.is_hostdevice:
--
To view, visit https://gerrit.ovirt.org/74327
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie81cf52f17ffe01713dd6502d68c4b17c64e5335
Gerrit-PatchSet: 31
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(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: Martin Peřina <mperina(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years
Change in vdsm[master]: network: decode command output to native string
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: network: decode command output to native string
......................................................................
Patch Set 25:
(1 comment)
https://gerrit.ovirt.org/#/c/77724/25/lib/vdsm/network/cmd.py
File lib/vdsm/network/cmd.py:
PS25, Line 62: 'utf-8'
> aha i see now. i think we should provide kwarg for exec_sync that would ena
I think that adding an argument that changes the type of the output is a sad idea, which was one of the reasons we ran aware from vdsm's execCmd. I prefer adding another function for raw bytes, or as I said - add a TODO, a failing test, and ignore the issue until we really go py3.
--
To view, visit https://gerrit.ovirt.org/77724
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I39ed13161487cff4f531f513a25a1c74787cf2df
Gerrit-PatchSet: 25
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
7 years
Change in vdsm[master]: network: use driverloader in vdsm.network.ovs.driver
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: network: use driverloader in vdsm.network.ovs.driver
......................................................................
network: use driverloader in vdsm.network.ovs.driver
Change-Id: I62afd52f7f15625108c4a880695abdf403882750
Signed-off-by: Petr Horacek <phoracek(a)redhat.com>
---
M lib/vdsm/network/ovs/driver/__init__.py
M lib/vdsm/network/ovs/driver/vsctl.py
M tests/network/ovs_driver_test.py
M tests/network/ovs_switch_test.py
4 files changed, 16 insertions(+), 24 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Petr Horáček: Verified
Edward Haas: Looks good to me, approved
--
To view, visit https://gerrit.ovirt.org/77046
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I62afd52f7f15625108c4a880695abdf403882750
Gerrit-PatchSet: 11
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>
7 years
Change in vdsm[master]: network: provide __repr__ for route and rule
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: network: provide __repr__ for route and rule
......................................................................
network: provide __repr__ for route and rule
IPRuleData and IPRouteData now provides __repr__ method in a more
Pythonic way.
Change-Id: I68bed024a52e8f0de9369be0b2f6b2d0153165b1
Signed-off-by: Petr Horacek <phoracek(a)redhat.com>
---
M lib/vdsm/network/ip/route/__init__.py
M lib/vdsm/network/ip/rule/__init__.py
2 files changed, 6 insertions(+), 4 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Petr Horáček: Verified
Dan Kenigsberg: Looks good to me, approved
--
To view, visit https://gerrit.ovirt.org/77415
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I68bed024a52e8f0de9369be0b2f6b2d0153165b1
Gerrit-PatchSet: 6
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>
7 years
Change in vdsm[master]: network: provide __repr__ for route and rule
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: network: provide __repr__ for route and rule
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/77415
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I68bed024a52e8f0de9369be0b2f6b2d0153165b1
Gerrit-PatchSet: 6
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: No
7 years
Change in vdsm[master]: ssl: enable tlsv1.2 support
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: ssl: enable tlsv1.2 support
......................................................................
Patch Set 29: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/72284/29/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:
Line 262:
Line 263: def create_ssl_context():
Line 264: sslctx = None
Line 265: if config.getboolean('vars', 'ssl'):
Line 266: protocol = config.get('vars', 'ssl_protocol')
I think that it is wrong to let this user-defined string leak so deep into the module. It is safer to have a conversion function, from ssl_protocol to context_options. the like of
def protocol_name_to_int()
would handle the strings that we must support due to backward compatibility (which are these? sertainly sslv23). The commit message should express clearly what is expected to happen if a user misspells the protocol version (I'm for a violent death to vdsm, with proper log).
Line 267: sslctx = SSLContext(key_file=constants.KEY_FILE,
Line 268: cert_file=constants.CERT_FILE,
Line 269: ca_certs=constants.CA_FILE, protocol=protocol)
--
To view, visit https://gerrit.ovirt.org/72284
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc83d8cc08349a0ca47ed7d021bb3fc6d1408a6
Gerrit-PatchSet: 29
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: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Peřina <mperina(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
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
7 years
Change in vdsm[master]: ssl: enable tlsv1.2 support
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: ssl: enable tlsv1.2 support
......................................................................
Patch Set 28:
Can you add an example to config.py? SSL.html is quite long.
Would it be possible to handle the el7 problem separately? The commit message, and probably the patch, is confusing due to its bundling into here.
--
To view, visit https://gerrit.ovirt.org/72284
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8cc83d8cc08349a0ca47ed7d021bb3fc6d1408a6
Gerrit-PatchSet: 28
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: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Peřina <mperina(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
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: No
7 years
Change in vdsm[ovirt-3.6]: Configure sasl2 libvirt to use scram-sha-1
by Code Review
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has uploaded a new change for review.
Change subject: Configure sasl2 libvirt to use scram-sha-1
......................................................................
Configure sasl2 libvirt to use scram-sha-1
The default of mech_list was changed in libvirt 3.2 to gssapi. This
patch adds to isconfigured a check if the new value exists. If so,
configure will replace the content to mech_list: scram-sha-1
Change-Id: I589e7de6df46ebb2f971701cf99a313b3c4a2f8e
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1458856
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
---
M lib/vdsm/tool/configurators/passwd.py
M vdsm.spec.in
2 files changed, 38 insertions(+), 12 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/05/78005/1
diff --git a/lib/vdsm/tool/configurators/passwd.py b/lib/vdsm/tool/configurators/passwd.py
index fc0cfc0..84ad4ff 100644
--- a/lib/vdsm/tool/configurators/passwd.py
+++ b/lib/vdsm/tool/configurators/passwd.py
@@ -22,15 +22,14 @@
from ... import utils
-from .import \
- YES, \
- NO
+from .import YES, NO, MAYBE
_SASLDBLISTUSERS2 = utils.CommandPath("sasldblistusers2",
"/usr/sbin/sasldblistusers2",
)
_LIBVIRT_SASLDB = "/etc/libvirt/passwd.db"
+_SASL2_CONF = "/etc/sasl2/libvirt.conf"
_SASLPASSWD2 = utils.CommandPath("saslpasswd2",
"/usr/sbin/saslpasswd2",
)
@@ -39,6 +38,22 @@
def isconfigured():
+ ret = passwd_isconfigured()
+ if ret == NO:
+ return ret
+ return libvirt_sasl_isconfigured()
+
+
+def libvirt_sasl_isconfigured():
+ with open(_SASL2_CONF, 'r') as f:
+ lines = f.readlines()
+ # check for new default configuration - since libvirt 3.2
+ if 'mech_list: gssapi\n' in lines:
+ return NO
+ return MAYBE
+
+
+def passwd_isconfigured():
script = (str(_SASLDBLISTUSERS2), '-f', _LIBVIRT_SASLDB)
_, out, _ = utils.execCmd(script)
for user in out:
@@ -48,18 +63,12 @@
def configure():
- script = (str(_SASLPASSWD2), '-p', '-a', 'libvirt', SASL_USERNAME)
- rc, _, err = utils.execCmd(script, data=libvirt_password())
- if rc != 0:
- raise RuntimeError("Set password failed: %s" % (err,))
- if utils.isOvirtNode():
- # It seems that all /etc/libvirt folder is persisted in node,
- # but better to persist the db file explicitly
- utils.persist(_LIBVIRT_SASLDB)
+ configure_libvirt_sasl()
+ configure_passwd()
def removeConf():
- if isconfigured() == YES:
+ if passwd_isconfigured() == YES:
rc, out, err = utils.execCmd(
(
str(_SASLPASSWD2),
@@ -72,6 +81,22 @@
raise RuntimeError("Remove password failed: %s" % (err,))
+def configure_libvirt_sasl():
+ with open(_SASL2_CONF, 'w') as f:
+ f.writelines(['## start vdsm-4.20.0 configuration\n',
+ 'mech_list: scram-sha-1\n',
+ 'sasldb_path: %s\n' % (_LIBVIRT_SASLDB),
+ '## end vdsm configuration']
+ )
+
+
+def configure_passwd():
+ script = (str(_SASLPASSWD2), '-p', '-a', 'libvirt', SASL_USERNAME)
+ rc, _, err = utils.execCmd(script, data=libvirt_password())
+ if rc != 0:
+ raise RuntimeError("Set password failed: %s" % (err,))
+
+
@utils.memoized
def libvirt_password():
with open(LIBVIRT_PASSWORD_PATH) as passwd_file:
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 28188ac..590da75 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -111,6 +111,7 @@
BuildRequires: systemd-units
+Requires: cyrus-sasl-scram
Requires: ethtool
Requires: which
Requires: sudo >= 1.7.3
--
To view, visit https://gerrit.ovirt.org/78005
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I589e7de6df46ebb2f971701cf99a313b3c4a2f8e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
7 years