Change in vdsm[master]: hugepages: add support for shared pages
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: hugepages: add support for shared pages
......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/77808/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:
PS2, Line 420: conv.tobool
> We should not use this evil tobool, if the input is not a bool, we should e
the value of custom properties is always a string, Engine can never send a bool. Maybe you suggest that we are stricter here
if custom.get('hugepages_shared') == 'True':
hugepages_shared = True
elif ('False', None)
hugepages_hsared = False
else: raise ValueError
I find this pile of code a bit ugly, and don't mind being a bit more flexible when it comes to human-typed values.
--
To view, visit https://gerrit.ovirt.org/77808
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0eb7b20a16e3e4a0d57da02ff7490136071dd8f
Gerrit-PatchSet: 2
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 Polednik <mpolednik(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 31:
(1 comment)
https://gerrit.ovirt.org/#/c/72284/31/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:
Line 271:
Line 272: def protocol_name_to_int():
Line 273: excludes = 0
Line 274:
Line 275: for no_protocol in config.get('vars', 'ssl_protocol').split(','):
I still do not get why you ignore someone who has set ssl_protocol to something like "sslv23" or another formerly-allowed value.
Line 276: if no_protocol != '':
Line 277: excludes |= getattr(ssl, no_protocol.strip())
Line 278:
--
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: 31
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]: concurrent: Move concurrent module to common
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: concurrent: Move concurrent module to common
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.ovirt.org/77993
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I920834214ef6e730a17aeac5ac9b16fea68cd7b3
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(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: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years
Change in vdsm[master]: spec: Handle VDSM services restart on upgrade to posttrans
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: spec: Handle VDSM services restart on upgrade to posttrans
......................................................................
spec: Handle VDSM services restart on upgrade to posttrans
VDSM services restart has been handled upon upgrade at the postun stage,
which is triggering the scriptlets of the source/old version.
In order to support upgrades with new logic, the scriptlets should run
based on the new version.
Therefore, the handling has been moved to the posttrans step, which
belongs and runs as part of the new version at the end of the
transaction.
Side effect: Upgrades from versions that predates 4.2 will have the
service restart occuring twice, once at the old version postun step and
once in the new posttrans step.
Change-Id: I16793ff6ca1fcf596f57b459fa4ed348a879b104
Bug-Url: https://bugzilla.redhat.com/1457889
Signed-off-by: Edward Haas <edwardh(a)redhat.com>
---
M vdsm.spec.in
1 file changed, 3 insertions(+), 2 deletions(-)
Approvals:
Piotr Kliczewski: Looks good to me, approved
Yaniv Bronhaim: Verified; Looks good to me, but someone else must approve
Jenkins CI: Passed CI tests
Edward Haas: Verified
--
To view, visit https://gerrit.ovirt.org/77785
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I16793ff6ca1fcf596f57b459fa4ed348a879b104
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(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: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
7 years
Change in vdsm[master]: Removing syslog print from vdsm-tool when module load fails
by Code Review
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has uploaded a new change for review.
Change subject: Removing syslog print from vdsm-tool when module load fails
......................................................................
Removing syslog print from vdsm-tool when module load fails
As part of e3b628fe we moved dump-bonding-options to network.py module -
this causes error prints in syslog while upgrading from old version. The
py file seems to leave leftovers which vdsm-tool tries to load while
running vdsm-tool commands during installation.
notes - leftovers won't stay after upgrade is complete - this case can
exist only during upgrade.
Change-Id: Ida68ee4f8284ee9bb4163487b4b9b462068ffa4b
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
---
M static/usr/bin/vdsm-tool
1 file changed, 3 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/77754/1
diff --git a/static/usr/bin/vdsm-tool b/static/usr/bin/vdsm-tool
index 9080230..4f8e6be 100755
--- a/static/usr/bin/vdsm-tool
+++ b/static/usr/bin/vdsm-tool
@@ -24,7 +24,6 @@
import logging
import os
import sys
-import syslog
import textwrap
import traceback
@@ -90,9 +89,9 @@
module = imp.load_module(package_name + '.' + mod_name, mod_fobj,
mod_absp, mod_desc)
except Exception:
- # py module was failed to load. report to syslog and continue
- syslog.syslog("module %s could not load to vdsm-tool: %s" %
- (mod_name, traceback.format_exc()))
+ # in case of a failure we want to continue. we saw cases such as
+ # leftovers pycs in the tool folder which cause too many prints.
+ # therefore, the logic is only to skip the exception
continue
mod_cmds = []
--
To view, visit https://gerrit.ovirt.org/77754
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ida68ee4f8284ee9bb4163487b4b9b462068ffa4b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
7 years
Change in vdsm[master]: Adding libvirt_sasl_isconfigured test
by Code Review
From Yaniv Bronhaim <ybronhei(a)redhat.com>:
Yaniv Bronhaim has uploaded a new change for review.
Change subject: Adding libvirt_sasl_isconfigured test
......................................................................
Adding libvirt_sasl_isconfigured test
This test checks if isconfigured works as expected. for latest libvirt
configure, the isconfigured should check for specific string in the
sasl2 conf file. The test validates that.
Change-Id: Ifb7aa1dfd579e2e0bf6493c06c164b6a42626d3f
Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
---
M tests/tool_test.py
1 file changed, 17 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/06/77506/1
diff --git a/tests/tool_test.py b/tests/tool_test.py
index 901b17b..a9ce0ef 100644
--- a/tests/tool_test.py
+++ b/tests/tool_test.py
@@ -23,6 +23,7 @@
from vdsm.tool.configfile import ConfigFile, ParserWrapper
from vdsm.tool.configurators import abrt
from vdsm.tool.configurators import libvirt
+from vdsm.tool.configurators import passwd
from vdsm.tool import UsageError
from vdsm.tool import upgrade
import monkeypatch
@@ -95,6 +96,22 @@
dict((m.name, m) for m in mockConfigurers))
+class PasswdConfiguratorTest(VdsmTestCase):
+ def testCheckIsConfigured(self):
+ first = tempfile.mktemp()
+ second = tempfile.mktemp()
+ with open(first, 'w') as f:
+ f.write("\n")
+ f.write("\n")
+ f.write("mech_list: gssapi\n")
+ with open(second, 'w') as f:
+ f.write("\n")
+ passwd._SASL2_CONF = first
+ self.assertEqual(passwd.libvirt_sasl_isconfigured(), NO)
+ passwd._SASL2_CONF = second
+ self.assertEqual(passwd.libvirt_sasl_isconfigured(), MAYBE)
+
+
@expandPermutations
class PatchConfiguratorsTests(VdsmTestCase):
--
To view, visit https://gerrit.ovirt.org/77506
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb7aa1dfd579e2e0bf6493c06c164b6a42626d3f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>
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 30:
(1 comment)
https://gerrit.ovirt.org/#/c/72284/30/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:
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()))
> PROTOCOL_TLS is not available on all platforms we use that is why we decide
PROTOCOL_TLS and PROTOCOL_SSLv23 are two differnt names for the same value (2). I believe that we don't want SSLContext with anything other that protocol=2. But maybe I'm missing something
Line 277:
--
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: nmdbus internal renaming of attributes
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: net: nmdbus internal renaming of attributes
......................................................................
net: nmdbus internal renaming of attributes
Renaming attributes for consistency with dbus naming.
Change-Id: I0f71ec7b2431fa12691ffa4d2826ae0638965953
Signed-off-by: Edward Haas <edwardh(a)redhat.com>
---
M lib/vdsm/network/nm/nmdbus/__init__.py
M lib/vdsm/network/nm/nmdbus/active.py
M lib/vdsm/network/nm/nmdbus/device.py
M lib/vdsm/network/nm/nmdbus/settings.py
4 files changed, 42 insertions(+), 36 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Petr Horáček: Looks good to me, but someone else must approve
Dan Kenigsberg: Looks good to me, approved
Edward Haas: Verified
--
To view, visit https://gerrit.ovirt.org/77903
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0f71ec7b2431fa12691ffa4d2826ae0638965953
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(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 Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
7 years
Change in vdsm[master]: net: nmdbus internal renaming of attributes
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: net: nmdbus internal renaming of attributes
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/77903
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f71ec7b2431fa12691ffa4d2826ae0638965953
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(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 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 30:
(1 comment)
https://gerrit.ovirt.org/#/c/72284/30/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:
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()))
> Correct, now we have protocol and excludes.
Internally, I don't think that we really need protocol - we should always use PROTOCOL_TLS, and always pepper options on top of it.
this function should translate whatever a user requested hitorically into the OP_NO_* integer which we should be using to give them what they wanted.
Line 277:
--
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