Change in vdsm[master]: net: simplify _get_gateway
Ido Barkan has uploaded a new change for review. Change subject: net: simplify _get_gateway .. net: simplify _get_gateway Using the default value of _RT_TABLE_UNSPEC is meaningless since it can be any default value chosen by the writer of this function. None is more concise. It is also worth testing for none first since in most cases the default value is used. Also noted a few untested case for future work. Change-Id: I6867576955a7769dd733e85a0ae3b4e111cdcb0b Signed-off-by: Ido Barkan --- M lib/vdsm/netinfo.py M tests/netinfoTests.py 2 files changed, 6 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/42966/1 diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index 716c21a..81f2619 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -634,8 +634,7 @@ return addrs -def _get_gateway(routes_by_dev, dev, family=4, - table=nl_route._RT_TABLE_UNSPEC): +def _get_gateway(routes_by_dev, dev, family=4, table=None): """ Return the default gateway for a device and an address family :param routes_by_dev: dictionary from device names to a list of routes. @@ -647,8 +646,7 @@ # currently from an IPv4 address) for each device so we have to look for # the gateway in all tables (RT_TABLE_UNSPEC), not just the 'main' one. gateways = [r for r in routes if r['destination'] == 'none' and -(r.get('table') == table or - table == nl_route._RT_TABLE_UNSPEC) and +(table is None or r.get('table') == table) and r['scope'] == 'global' and r['family'] == ('inet6' if family == 6 else 'inet') ] diff --git a/tests/netinfoTests.py b/tests/netinfoTests.py index 94bf924..0824d98 100644 --- a/tests/netinfoTests.py +++ b/tests/netinfoTests.py @@ -308,6 +308,10 @@ bonds.write('-' + bondName) def test_get_gateway(self): +# TODO: test ipv6 gateway +# TODO: test more then one gateway +# TODO: test specific table +# TODO: test multiple gateway per device (error flow) TEST_IFACE = 'test_iface' # different tables but the gateway is the same so it should be reported DUPLICATED_GATEWAY = {TEST_IFACE: [ -- To view, visit https://gerrit.ovirt.org/42966 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6867576955a7769dd733e85a0ae3b4e111cdcb0b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: simplify _get_gateway
Ido Barkan has posted comments on this change. Change subject: net: simplify _get_gateway .. Patch Set 1: Verified+1 -- To view, visit https://gerrit.ovirt.org/42966 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6867576955a7769dd733e85a0ae3b4e111cdcb0b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: simplify _get_gateway
automat...@ovirt.org has posted comments on this change. Change subject: net: simplify _get_gateway .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42966 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6867576955a7769dd733e85a0ae3b4e111cdcb0b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: tests: only test kernel config if unified persistence
automat...@ovirt.org has posted comments on this change. Change subject: net: tests: only test kernel config if unified persistence .. Patch Set 1: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * warn_if_not_merged_to_previous_branch: OK -- To view, visit https://gerrit.ovirt.org/42964 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If7c360deea32f4514919eabee2284c34667b4a4e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: tests: do not use KernelConfig.__eq__ in tests
automat...@ovirt.org has posted comments on this change. Change subject: net: tests: do not use KernelConfig.__eq__ in tests .. Patch Set 1: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42963 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b142d1b62e293bf094f650c3c691d65f684231d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: Drop default bonding options
automat...@ovirt.org has posted comments on this change. Change subject: net: Drop default bonding options .. Patch Set 1: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42959 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e4a8d832891f62d1fcc412606b014fa4f874062 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: ifcfg: ONBOOT=yes on all persisted network files.
automat...@ovirt.org has posted comments on this change. Change subject: net: ifcfg: ONBOOT=yes on all persisted network files. .. Patch Set 1: * Update tracker::#1203422::OK * Check Bug-Url::OK * Check Public Bug::#1203422::OK, public bug * Check Product::#1203422::OK, Correct product oVirt * Check TR::#1203422::OK, correct target release 3.5.4 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42962 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia942827745945f0f2e2a0e1691996824511cc111 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: introducing KernelConfig
automat...@ovirt.org has posted comments on this change. Change subject: net: introducing KernelConfig .. Patch Set 1: * Update tracker::#1203422::OK * Check Bug-Url::OK * Check Public Bug::#1203422::OK, public bug * Check Product::#1203422::OK, Correct product oVirt * Check TR::#1203422::OK, correct target release 3.5.4 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42960 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79f1eb553a42f1398ad12aa1bc33522f8af30c79 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: during boot, restore only networks that were actually c...
automat...@ovirt.org has posted comments on this change. Change subject: net: during boot, restore only networks that were actually changed. .. Patch Set 1: * Update tracker::#1203422::OK * Check Bug-Url::OK * Check Public Bug::#1203422::OK, public bug * Check Product::#1203422::OK, Correct product oVirt * Check TR::#1203422::OK, correct target release 3.5.4 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42961 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I66307a7cf6b0f7269a1ae04aa7c1748dde768144 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: missing Exception formatting value
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/42955 to review the following change. Change subject: net: missing Exception formatting value .. net: missing Exception formatting value Change-Id: I1e3bb7067c5c7ec70c7120515c7fb1231e9cabdc Signed-off-by: Ido Barkan Reviewed-on: https://gerrit.ovirt.org/42195 Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M lib/vdsm/netinfo.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/55/42955/1 diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index 0844251..4cd5b23 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -294,7 +294,7 @@ def prefix2netmask(prefix): if not 0 <= prefix <= 32: raise ValueError('%s is not a valid prefix value. It must be between ' - '0 and 32') + '0 and 32' % prefix) return socket.inet_ntoa( struct.pack("!I", int('1' * prefix + '0' * (32 - prefix), 2))) -- To view, visit https://gerrit.ovirt.org/42955 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1e3bb7067c5c7ec70c7120515c7fb1231e9cabdc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: Move _stpBooleanize to netinfo.
automat...@ovirt.org has posted comments on this change. Change subject: Move _stpBooleanize to netinfo. .. Patch Set 1: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42956 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1b2dc96fb405c2055431209023f1cb5130e5ae5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: Drop default bonding options
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/42959 to review the following change. Change subject: net: Drop default bonding options .. net: Drop default bonding options On old engines, no bonding options were provided to VDSM and it used to configure it's own default options. Nowadays, those options are always provided by engine. If a user will setup a network on an existing bond, VDSM takes ownership of the bond and configures the options on it. The code for the default options was very local and options were set 'too late' to be persisted. Hence this code was incorrect and is not needed anyway. The change in behavior is that from now on, if a bond with no options is explicitly requested, the kernel will be asked to create a bond without any options making the default kernel options effective. Change-Id: I9e4a8d832891f62d1fcc412606b014fa4f874062 Signed-off-by: Ido Barkan Reviewed-on: https://gerrit.ovirt.org/42594 Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M tests/functional/networkTests.py M vdsm/network/models.py 2 files changed, 9 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/59/42959/1 diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 79a2200..6719346 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -1382,8 +1382,7 @@ self.assertEquals(status, SUCCESS, msg) self.assertBondExists(BONDING_NAME, nics=nics[:1]) -self._assert_exact_bond_opts(BONDING_NAME, - ['miimon=150', 'mode=4']) +self._assert_exact_bond_opts(BONDING_NAME, []) # Increase bond size bondings[BONDING_NAME]['nics'] = nics @@ -1392,8 +1391,7 @@ self.assertEquals(status, SUCCESS, msg) self.assertBondExists(BONDING_NAME, nics) -self._assert_exact_bond_opts(BONDING_NAME, - ['miimon=150', 'mode=4']) +self._assert_exact_bond_opts(BONDING_NAME, []) # Reduce bond size REQMODE_BROADCAST = '3' diff --git a/vdsm/network/models.py b/vdsm/network/models.py index 73839cb..f4b698f 100644 --- a/vdsm/network/models.py +++ b/vdsm/network/models.py @@ -188,11 +188,8 @@ for slave in slaves: slave.master = self self.slaves = slaves -if options is None: -self.options = 'mode=802.3ad miimon=150' -else: -self.validateOptions(name, options) -self.options = options +self.options = options or '' +self.validateOptions(name, self.options) self.destroyOnMasterRemoval = destroyOnMasterRemoval super(Bond, self).__init__(name, configurator, ipconfig, mtu) @@ -217,6 +214,11 @@ self.configurator.configureBond(self, **opts) def areOptionsApplied(self): +# TODO: this method returns True iff self.options are a subset of the +# TODO: current bonding options. VDSM should probably compute if the +# TODO: non-default settings are equal to the non-default state. +if not self.options: +return True confOpts = [option.split('=', 1) for option in self.options.split(' ')] activeOpts = netinfo.bondOpts(self.name, (name for name, value in confOpts)) -- To view, visit https://gerrit.ovirt.org/42959 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9e4a8d832891f62d1fcc412606b014fa4f874062 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: tests: wrap setupNetworks calls
automat...@ovirt.org has posted comments on this change. Change subject: net: tests: wrap setupNetworks calls .. Patch Set 1: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42957 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36eb18c4608f66926af8da564e2ba105ea4d8109 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: reset bonding options to defaults.
automat...@ovirt.org has posted comments on this change. Change subject: net: reset bonding options to defaults. .. Patch Set 1: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42958 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe720674e63119e954265735e9f5ccb994c3531a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: tests: do not use KernelConfig.__eq__ in tests
Hello Petr Horáček, Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/42963 to review the following change. Change subject: net: tests: do not use KernelConfig.__eq__ in tests .. net: tests: do not use KernelConfig.__eq__ in tests Until now, functional tests used the __eq__ to assert that a KernelConfig instance is equal to a RunningConfig instance. This hides the normalization process inside __eq__ and makes the life of the tester very hard if something really breaks. We trade some magic here for a better error message. Change-Id: I9b142d1b62e293bf094f650c3c691d65f684231d Signed-off-by: Ido Barkan Reviewed-on: https://gerrit.ovirt.org/42813 Continuous-Integration: Jenkins CI Reviewed-by: Petr Horáček Reviewed-by: Dan Kenigsberg --- M tests/functional/networkTests.py 1 file changed, 5 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/63/42963/1 diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index f9e9339..4c8fb1d 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -351,7 +351,11 @@ netinfo = self.vdsm_net.netinfo kernel_config = KernelConfig(netinfo) running_config = self.vdsm_net.config -self.assertEqual(kernel_config, running_config) +# do not use KernelConfig.__eq__ to get better exception if something +# breaks here +normalized_config = kernel_config.normalize(running_config) +self.assertEqual(normalized_config.networks, kernel_config.networks) +self.assertEqual(normalized_config.bonds, kernel_config.bonds) @cleanupNet @permutations([[True], [False]]) -- To view, visit https://gerrit.ovirt.org/42963 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9b142d1b62e293bf094f650c3c691d65f684231d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Petr Horáček ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: missing Exception formatting value
automat...@ovirt.org has posted comments on this change. Change subject: net: missing Exception formatting value .. Patch Set 1: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42955 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1e3bb7067c5c7ec70c7120515c7fb1231e9cabdc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: reset bonding options to defaults.
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/42958 to review the following change. Change subject: net: reset bonding options to defaults. .. net: reset bonding options to defaults. When an existing bond is edited, replacing bonding options in the ifcfg file and ifdown+ifup is not enough. This is because ifcfg script will only set the options that are indicated. This is a bug since what is persisted is not what is eventually configured on the edited bond. This is solved by resetting the non default options back before ifup. testSetupNetworksResizeBond was changed to point at the bug and prove it is solved. Change-Id: Ibe720674e63119e954265735e9f5ccb994c3531a Signed-off-by: Ido Barkan Reviewed-on: https://gerrit.ovirt.org/42115 Reviewed-by: Dan Kenigsberg Continuous-Integration: Jenkins CI --- M lib/vdsm/netinfo.py M tests/functional/networkTests.py M vdsm/network/configurators/ifcfg.py 3 files changed, 85 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/58/42958/1 diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index fbc737f..15255d6 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -504,7 +504,7 @@ @memoized -def _getDefaultBondingOptions(mode=None): +def getDefaultBondingOptions(mode=None): """ Return default options for the given mode. If it is None, return options for the default mode (usually '0'). @@ -524,7 +524,7 @@ """ opts = realBondOpts(bond) mode = opts['mode'][-1] if 'mode' in opts else None -defaults = _getDefaultBondingOptions(mode) +defaults = getDefaultBondingOptions(mode) return dict(((opt, val[-1]) for (opt, val) in opts.iteritems() if val and val != defaults.get(opt))) diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index c13f471..79a2200 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -232,9 +232,22 @@ self.assertEqual(set(nics), set(config.bonds[bondName].get('nics'))) if options is not None: -active = (opt + '=' + val for (opt, val) - in netinfo.bondings[bondName]['opts'].iteritems()) -self.assertTrue(set(options.split()) <= set(active)) +active_opts = self._get_active_bond_opts(bondName) +self.assertTrue(set(options.split()) <= set(active_opts)) + +def _assert_exact_bond_opts(self, bond, opts): +""":param opts: list of strings e.g. ['miimon=150', 'mode=4']""" +# TODO: we should try and call this logic always during +# TODO: assertBondExists and be stricter. Will probably need to fix a +# TODO: few tests +self.assertEqual(set(self._get_active_bond_opts(bond)), + set(opts)) + +def _get_active_bond_opts(self, bondName): +netinfo = self.vdsm_net.netinfo +active_options = [opt + '=' + val for (opt, val) + in netinfo.bondings[bondName]['opts'].iteritems()] +return active_options def assertBondDoesntExist(self, bondName, nics=None): netinfo = self.vdsm_net.netinfo @@ -1369,6 +1382,8 @@ self.assertEquals(status, SUCCESS, msg) self.assertBondExists(BONDING_NAME, nics=nics[:1]) +self._assert_exact_bond_opts(BONDING_NAME, + ['miimon=150', 'mode=4']) # Increase bond size bondings[BONDING_NAME]['nics'] = nics @@ -1377,6 +1392,8 @@ self.assertEquals(status, SUCCESS, msg) self.assertBondExists(BONDING_NAME, nics) +self._assert_exact_bond_opts(BONDING_NAME, + ['miimon=150', 'mode=4']) # Reduce bond size REQMODE_BROADCAST = '3' @@ -1387,8 +1404,9 @@ self.assertEquals(status, SUCCESS, msg) -self.assertBondExists(BONDING_NAME, nics[:2], - bondings[BONDING_NAME]['options']) +self.assertBondExists(BONDING_NAME, nics[:2]) +self._assert_exact_bond_opts( +BONDING_NAME, [bondings[BONDING_NAME]['options']]) bondings = {BONDING_NAME: dict(remove=True)} status, msg = self.setupNetworks({}, bondings, {}) diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py index d1f65a4..5fd4eb8 100644 --- a/vdsm/network/configurators/ifcfg.py +++ b/vdsm/network/configurators/ifcfg.py @@ -18,6 +18,7 @@ # from __future__ import absolute_import +import errno import glob import logging import os @@ -142,6 +143,7 @@ if bondIfcfgWritten: ifdown(bond.name) +_re
Change in vdsm[ovirt-3.5]: net: during boot, restore only networks that were actually c...
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/42961 to review the following change. Change subject: net: during boot, restore only networks that were actually changed. .. net: during boot, restore only networks that were actually changed. This is an optimization that will shorten boot time when there are many networks. If also, all persisted networks will be persisted with onboot=true, then in most case, all the hard of restorattion will be done by network service and vdsm will only interfere to fix the changed networks and bonds. Change-Id: I66307a7cf6b0f7269a1ae04aa7c1748dde768144 Signed-off-by: Ido Barkan Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1203422 Reviewed-on: https://gerrit.ovirt.org/42355 Reviewed-by: Dan Kenigsberg Continuous-Integration: Jenkins CI --- M lib/vdsm/netconfpersistence.py M tests/functional/networkTests.py M vdsm/network/configurators/__init__.py M vdsm/network/configurators/ifcfg.py M vdsm/network/configurators/libvirt.py M vdsm/vdsm-restore-net-config 6 files changed, 155 insertions(+), 53 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/61/42961/1 diff --git a/lib/vdsm/netconfpersistence.py b/lib/vdsm/netconfpersistence.py index ee5039d..55aef45 100644 --- a/lib/vdsm/netconfpersistence.py +++ b/lib/vdsm/netconfpersistence.py @@ -199,7 +199,7 @@ self.setBonding(bond, bond_attr) def __eq__(self, other): -normalized_other = self._normalize(other) +normalized_other = self.normalize(other) return (self.networks == normalized_other.networks and self.bonds == normalized_other.bonds) @@ -302,7 +302,7 @@ if v != 0) return stripped_qos -def _normalize(self, running_config): +def normalize(self, running_config): # TODO: normalize* methods can become class functions, as they are only # TODO: dependent in self._netinfo, which is only needed to access # TODO: netinfo module level functions, that cannot be imported here diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index e376813..f9e9339 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -28,7 +28,8 @@ expandPermutations, permutations) from testValidation import (brokentest, RequireDummyMod, RequireVethMod, ValidateRunningAsRoot) -from vdsm.netconfpersistence import KernelConfig +from vdsm.ipwrapper import linkDel, linkSet +from vdsm.netconfpersistence import KernelConfig, RunningConfig import dhcp import dummy import firewall @@ -199,7 +200,8 @@ func(*args, **kwargs) return wrapper -def assertNetworkExists(self, networkName, bridged=None, bridgeOpts=None): +def assertNetworkExists(self, networkName, bridged=None, bridgeOpts=None, +assert_in_running_conf=True): netinfo = self.vdsm_net.netinfo config = self.vdsm_net.config self.assertIn(networkName, netinfo.networks) @@ -209,7 +211,7 @@ appliedOpts = netinfo.bridges[networkName]['opts'] for opt, value in bridgeOpts.iteritems(): self.assertEqual(value, appliedOpts[opt]) -if config is not None: +if assert_in_running_conf and config is not None: self.assertIn(networkName, config.networks) if bridged is not None: self.assertEqual(config.networks[networkName].get('bridged'), @@ -220,14 +222,23 @@ if self.vdsm_net.config is not None: self.assertNotIn(networkName, self.vdsm_net.config.networks) -def assertBondExists(self, bondName, nics=None, options=None): +def assertBridgeExists(self, bridgeName): +netinfo = self.vdsm_net.netinfo +self.assertIn(bridgeName, netinfo.bridges) + +def assertBridgeDoesntExist(self, bridgeName): +netinfo = self.vdsm_net.netinfo +self.assertNotIn(bridgeName, netinfo.bridges) + +def assertBondExists(self, bondName, nics=None, options=None, + assert_in_running_conf=True): netinfo = self.vdsm_net.netinfo config = self.vdsm_net.config self.assertIn(bondName, netinfo.bondings) if nics is not None: self.assertEqual(set(nics), set(netinfo.bondings[bondName]['slaves'])) -if config is not None: +if assert_in_running_conf and config is not None: self.assertIn(bondName, config.bonds) self.assertEqual(set(nics), set(config.bonds[bondName].get('nics'))) @@ -1479,6 +1490,97 @@ self.vdsm_net.save_config() @cleanupNet +@RequireDummyMod +@ValidateRunningAsRoot +def testRestoreN
Change in vdsm[ovirt-3.5]: net: tests: only test kernel config if unified persistence
Ido Barkan has uploaded a new change for review. Change subject: net: tests: only test kernel config if unified persistence .. net: tests: only test kernel config if unified persistence Testing KernelConfig does not support ifcfg persistence. Change-Id: If7c360deea32f4514919eabee2284c34667b4a4e Signed-off-by: Ido Barkan --- M tests/functional/networkTests.py 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/42964/1 diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 4c8fb1d..b45653a 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -341,9 +341,10 @@ self.assertIn(b, self.vdsm_net.netinfo.bondings) def setupNetworks(self, *args, **kwargs): -test_kernel_config = kwargs.pop('test_kernel_config', True) status, msg = self.vdsm_net.setupNetworks(*args, **kwargs) -if test_kernel_config: +unified = ( +vdsm.config.config.get('vars', 'net_persistence') == 'unified') +if unified and kwargs.pop('test_kernel_config', True): self._assert_kernel_config_matches_running_config() return status, msg -- To view, visit https://gerrit.ovirt.org/42964 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If7c360deea32f4514919eabee2284c34667b4a4e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: ifcfg: ONBOOT=yes on all persisted network files.
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/42962 to review the following change. Change subject: net: ifcfg: ONBOOT=yes on all persisted network files. .. net: ifcfg: ONBOOT=yes on all persisted network files. This will cause network service to configure those networks long before vdsm starts. after https://gerrit.ovirt.org/#/c/42355/ vdsm will only restore changed networks. Change-Id: Ia942827745945f0f2e2a0e1691996824511cc111 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1203422 Signed-off-by: Ido Barkan Reviewed-on: https://gerrit.ovirt.org/42362 Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M vdsm/network/configurators/ifcfg.py M vdsm/network/models.py 2 files changed, 4 insertions(+), 25 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/62/42962/1 diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py index 60bbc66..cf42256 100644 --- a/vdsm/network/configurators/ifcfg.py +++ b/vdsm/network/configurators/ifcfg.py @@ -609,10 +609,7 @@ if bridge.stp is not None: conf += 'STP=%s\n' % ('on' if bridge.stp else 'off') ipconfig = bridge.ipConfig -if not self.unifiedPersistence or ipconfig.defaultRoute: -conf += 'ONBOOT=%s\n' % 'yes' -else: -conf += 'ONBOOT=%s\n' % 'no' +conf += 'ONBOOT=%s\n' % 'yes' defaultRoute = ConfigWriter._toIfcfgFormat(ipconfig.defaultRoute) ipconfig = ipconfig._replace(defaultRoute=defaultRoute) @@ -626,10 +623,7 @@ opts['hotplug'] = 'no' # So that udev doesn't trigger an ifup if vlan.bridge: conf += 'BRIDGE=%s\n' % pipes.quote(vlan.bridge.name) -if not self.unifiedPersistence or vlan.serving_default_route: -conf += 'ONBOOT=%s\n' % 'yes' -else: -conf += 'ONBOOT=%s\n' % 'no' +conf += 'ONBOOT=%s\n' % 'yes' ipconfig = vlan.ipConfig defaultRoute = ConfigWriter._toIfcfgFormat(ipconfig.defaultRoute) ipconfig = ipconfig._replace(defaultRoute=defaultRoute) @@ -641,10 +635,7 @@ opts['hotplug'] = 'no' # So that udev doesn't trigger an ifup if bond.bridge: conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name) -if not self.unifiedPersistence or bond.serving_default_route: -conf += 'ONBOOT=%s\n' % 'yes' -else: -conf += 'ONBOOT=%s\n' % 'no' +conf += 'ONBOOT=%s\n' % 'yes' ipconfig, mtu = self._getIfaceConfValues(bond) self._createConfFile(conf, bond.name, ipconfig, mtu, **opts) @@ -667,10 +658,7 @@ conf += 'BRIDGE=%s\n' % pipes.quote(nic.bridge.name) if nic.bond: conf += 'MASTER=%s\nSLAVE=yes\n' % pipes.quote(nic.bond.name) -if not self.unifiedPersistence or nic.serving_default_route: -conf += 'ONBOOT=%s\n' % 'yes' -else: -conf += 'ONBOOT=%s\n' % 'no' +conf += 'ONBOOT=%s\n' % 'yes' ethtool_opts = getEthtoolOpts(nic.name) if ethtool_opts: diff --git a/vdsm/network/models.py b/vdsm/network/models.py index f4b698f..8e33950 100644 --- a/vdsm/network/models.py +++ b/vdsm/network/models.py @@ -71,15 +71,6 @@ return self.master return None -@property -def serving_default_route(self): -device = self -while device: -if device.ipConfig.defaultRoute: -return True -device = device.master -return False - class Nic(NetDevice): def __init__(self, name, configurator, ipconfig=None, mtu=None, -- To view, visit https://gerrit.ovirt.org/42962 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia942827745945f0f2e2a0e1691996824511cc111 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: Move _stpBooleanize to netinfo.
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/42956 to review the following change. Change subject: Move _stpBooleanize to netinfo. .. Move _stpBooleanize to netinfo. And rename it. It will be used in the following patched from netconfpersistence.py Change-Id: Ie1b2dc96fb405c2055431209023f1cb5130e5ae5 Signed-off-by: Ido Barkan Reviewed-on: https://gerrit.ovirt.org/41972 Reviewed-by: Dan Kenigsberg Continuous-Integration: Jenkins CI --- M lib/vdsm/netinfo.py M vdsm/network/api.py 2 files changed, 14 insertions(+), 14 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/42956/1 diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index 4cd5b23..fbc737f 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -207,6 +207,19 @@ return 'off' +def stp_booleanize(value): +if value is None: +return False +if type(value) is bool: +return value +if value.lower() in ('true', 'on', 'yes'): +return True +elif value.lower() in ('false', 'off', 'no'): +return False +else: +raise ValueError('Invalid value for bridge stp') + + def isvirtio(dev): return 'virtio' in os.readlink('/sys/class/net/%s/device' % dev) diff --git a/vdsm/network/api.py b/vdsm/network/api.py index 62e3d0d..f829732 100755 --- a/vdsm/network/api.py +++ b/vdsm/network/api.py @@ -127,7 +127,7 @@ elif 'STP' in opts: stp = opts.pop('STP') try: -stp = _stpBooleanize(stp) +stp = netinfo.stp_booleanize(stp) except ValueError: raise ConfigNetworkError(ne.ERR_BAD_PARAMS, '"%s" is not a valid ' 'bridge STP value.' % stp) @@ -143,19 +143,6 @@ blocking=(configurator._inRollback or utils.tobool(blockingdhcp)), ipv6autoconf=ipv6autoconf, dhcpv6=dhcpv6) return topNetDev - - -def _stpBooleanize(value): -if value is None: -return False -if type(value) is bool: -return value -if value.lower() in ('true', 'on', 'yes'): -return True -elif value.lower() in ('false', 'off', 'no'): -return False -else: -raise ValueError('Invalid value for bridge stp') def _validateInterNetworkCompatibility(ni, vlan, iface, bridged): -- To view, visit https://gerrit.ovirt.org/42956 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie1b2dc96fb405c2055431209023f1cb5130e5ae5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: introducing KernelConfig
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/42960 to review the following change. Change subject: net: introducing KernelConfig .. net: introducing KernelConfig KernelConfig can be created from a NetInfo object. It reflects the network kernel state and exposes an API similar to RunningConfig. At this stage, It can compare itself to a RunninConfig instance in order to be tested properly and prove it is 'correct'. Later, it should be able to tell which networks/bonds it obtained from the kernel are different from a given PersistentConfig's networks/bodns. This is useful at boot time in order to allow skipping restoring already configured networks. Since VDSM network api has language differences from netinfo module and also has many default parameters, KernelConfig tries to do 2 things: 1. It tries to be expilict as possible (no defaults) 2. It knows how to 'normalize' a running/persistent config so it would also be explicit. Note the custom properties are never included in KernelConfig, as they only serve as hints to Vdsm and its hooks. Therefore, tests which set custom properties would certainly get into RunningConfig!=KernelConfig. These tests pass test_kernel_config=False so as not to fail. Change-Id: I79f1eb553a42f1398ad12aa1bc33522f8af30c79 Signed-off-by: Ido Barkan Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1203422 Reviewed-on: https://gerrit.ovirt.org/41973 Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M lib/vdsm/netconfpersistence.py M lib/vdsm/netinfo.py M tests/functional/networkTests.py 3 files changed, 266 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/60/42960/1 diff --git a/lib/vdsm/netconfpersistence.py b/lib/vdsm/netconfpersistence.py index 8feedf2..ee5039d 100644 --- a/lib/vdsm/netconfpersistence.py +++ b/lib/vdsm/netconfpersistence.py @@ -18,9 +18,11 @@ # Refer to the README and COPYING files for full details of the license # +import copy import errno import json import logging +import netaddr import os from .config import config @@ -28,12 +30,17 @@ from . import constants from . import utils - CONF_RUN_DIR = constants.P_VDSM_RUN + 'netconf/' # The persistent path is inside of an extra "persistence" dir in order to get # oVirt Node to persist the symbolic links that are necessary for the # atomic storage of running config into persistent config. CONF_PERSIST_DIR = constants.P_VDSM_LIB + 'persistence/netconf/' +_BONDING_MODES = { +# TODO: this dictionary and the reverse mapping are duplicated in code +'0': 'balance-rr', '1': 'active-backup', '2': 'balance-xor', +'3': 'broadcast', '4': '802.3ad', '5': 'balance-tlb', '6': 'balance-alb' +} +_BONDING_MODES_REVERSED = dict((v, k) for k, v in _BONDING_MODES.iteritems()) class BaseConfig(object): @@ -182,6 +189,216 @@ (self, self.networksPath, self.bondingsPath)) +class KernelConfig(BaseConfig): +def __init__(self, netinfo): +super(KernelConfig, self).__init__({}, {}) +self._netinfo = netinfo +for net, net_attr in self._analyze_netinfo_nets(netinfo): +self.setNetwork(net, net_attr) +for bond, bond_attr in self._analyze_netinfo_bonds(netinfo): +self.setBonding(bond, bond_attr) + +def __eq__(self, other): +normalized_other = self._normalize(other) +return (self.networks == normalized_other.networks +and self.bonds == normalized_other.bonds) + +def _analyze_netinfo_nets(self, netinfo): +for net, net_attr in netinfo.networks.iteritems(): +yield net, self._translate_netinfo_net(net, net_attr) + +def _analyze_netinfo_bonds(self, netinfo): +for bond, bond_attr in netinfo.bondings.iteritems(): +yield bond, self._translate_netinfo_bond(bond_attr) + +def _translate_netinfo_net(self, net, net_attr): +nics, vlan, bond = self._netinfo.getNicsVlanAndBondingForNetwork(net) +attributes = {} +self._translate_bridged(attributes, net_attr) +self._translate_mtu(attributes, net_attr) +self._translate_vlan(attributes, vlan) +if bond: +self._translate_bonding(attributes, bond) +elif nics: +self._translate_nics(attributes, nics) +self._translate_ipaddr(attributes, net_attr) +self._translate_hostqos(attributes, net_attr) + +return attributes + +def _translate_hostqos(self, attributes, net_attr): +if net_attr.get('hostQos'): +attributes['hostQos'] = self._remove_zero_values_in_net_qos( +net_attr['hostQos']) + +def _translate_ipaddr(self, attributes, net_attr): +attributes['bootproto'] = net_attr['bootproto4'] +ifcfg = net_attr.get('cfg') +# TODO: we must not depend on 'cfg', which is config
Change in vdsm[ovirt-3.5]: net: bondingOptions should not be a part of runningConfig ne...
automat...@ovirt.org has posted comments on this change. Change subject: net: bondingOptions should not be a part of runningConfig networks .. Patch Set 2: Verified-1 * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::ERROR, At least one bug-url is required for the stable branch * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/42146 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I374239670df77804e397b4189a9405f1479d0259 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: disable truncating diff in assert error messages
Ido Barkan has posted comments on this change. Change subject: tests: disable truncating diff in assert error messages .. Patch Set 2: Verified+1 -- To view, visit https://gerrit.ovirt.org/42902 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70efdc56e7638ebe363453f1df4d3a520ab44f2e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Mooli Tayer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: disable truncating diff in assert error messages
automat...@ovirt.org has posted comments on this change. Change subject: tests: disable truncating diff in assert error messages .. Patch Set 2: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42902 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70efdc56e7638ebe363453f1df4d3a520ab44f2e Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Mooli Tayer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: disable truncating diff in assert error messages
Ido Barkan has posted comments on this change. Change subject: tests: disable truncating diff in assert error messages .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/42902/1/tests/testlib.py File tests/testlib.py: Line 149: class VdsmTestCase(unittest.TestCase): Line 150: def __init__(self, *args, **kwargs): Line 151: unittest.TestCase.__init__(self, *args, **kwargs) Line 152: self.log = logging.getLogger(self.__class__.__name__) Line 153: self.maxDiff = None # disable truncating diff in assert error messages > $ git grep maxDiff done. explanation in commit msg. Line 154: Line 155: def retryAssert(self, *args, **kwargs): Line 156: '''Keep retrying an assertion if AssertionError is raised. Line 157:See function utils.retry for the meaning of the arguments. -- To view, visit https://gerrit.ovirt.org/42902 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I70efdc56e7638ebe363453f1df4d3a520ab44f2e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Mooli Tayer Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
Ala Hino has posted comments on this change. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/42952/1/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 281 Line 282 Line 283 Line 284 Line 285 > Addding self._volinfo makes self._backup_servers_option unneeded. This chan Done -- To view, visit https://gerrit.ovirt.org/42952 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3014723e1e3d1164a986f5e04208e199396b21b0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
automat...@ovirt.org has posted comments on this change. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. Patch Set 2: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42952 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3014723e1e3d1164a986f5e04208e199396b21b0 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Verify volume is Replica 3
automat...@ovirt.org has posted comments on this change. Change subject: gluster: Verify volume is Replica 3 .. Patch Set 40: * Update tracker::#1123052::OK * Check Bug-Url::OK * Check Public Bug::#1123052::OK, public bug * Check Product::#1123052::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/41931 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fee5d023706e2a0613201a9afef7d285b08442f Gerrit-PatchSet: 40 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sahina Bose Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: vm: Continue to sample after errors
Nir Soffer has abandoned this change. Change subject: vm: Continue to sample after errors .. Abandoned Not needed now, this issue is fixed in master. -- To view, visit https://gerrit.ovirt.org/22575 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I8dbe60a4d3b216a5cd998d163407c09b12f2f28c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: vm: Continue to sample after errors
automat...@ovirt.org has posted comments on this change. Change subject: vm: Continue to sample after errors .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found -- To view, visit https://gerrit.ovirt.org/22575 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8dbe60a4d3b216a5cd998d163407c09b12f2f28c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: vm: Avoid log spamming when drive format is undefined
automat...@ovirt.org has posted comments on this change. Change subject: vm: Avoid log spamming when drive format is undefined .. Patch Set 2: * Update tracker::IGNORE, no Bug-Url found -- To view, visit https://gerrit.ovirt.org/22518 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.3]: vm: Avoid log spamming when drive format is undefined
Nir Soffer has abandoned this change. Change subject: vm: Avoid log spamming when drive format is undefined .. Abandoned Not needed now, this issue is fixed in master. -- To view, visit https://gerrit.ovirt.org/22518 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.3 Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sergey Gotliv Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: Yaniv Bronhaim ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
Nir Soffer has posted comments on this change. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/42952/1/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 281 Line 282 Line 283 Line 284 Line 285 Addding self._volinfo makes self._backup_servers_option unneeded. This change is not related to adding validation in the next patch -- To view, visit https://gerrit.ovirt.org/42952 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3014723e1e3d1164a986f5e04208e199396b21b0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Safe device type check for console device
Martin Polednik has posted comments on this change. Change subject: virt: Safe device type check for console device .. Patch Set 1: Code-Review-1 (1 comment) https://gerrit.ovirt.org/#/c/42881/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3907: if not hasattr(dev, 'alias'): Line 3908: dev.alias = alias Line 3909: Line 3910: for dev in self.conf['devices']: Line 3911: if dev.get('device') == hwclass.CONSOLE and \ > From my point of view - yes - the Engine should send both 'device' and 'typ Actually it has to be present (it is stated in the API) and I'm not sure if it's not actually enforced via minimum params validation. I believe that this fix is not perfectly correct, because it is pretty much swallowing of an exception that may point out to an error. Line 3912: not dev.get('alias'): Line 3913: dev['alias'] = alias Line 3914: Line 3915: def _getUnderlyingSmartcardDeviceInfo(self): -- To view, visit https://gerrit.ovirt.org/42881 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a0c1adc6c3b2ab8d50b6e3ac712ce2dcf04cc6c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Shmuel Leib Melamud Gerrit-Reviewer: Shmuel Melamud Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Verify volume is Replica 3
Ala Hino has posted comments on this change. Change subject: gluster: Verify volume is Replica 3 .. Patch Set 38: (1 comment) https://gerrit.ovirt.org/#/c/41931/38/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 314: self.log.warn("Using user specified backup-volfile-servers option") Line 315: return "" Line 316: Line 317: volinfo = supervdsm.getProxy().glusterVolumeInfo(self._volname, Line 318: self._volfileServer) > This patch does not make sense - why are you duplicating the call to gluste I agree that the duplicate calls are bad. I was thinking that introducing volinfo property without "real" reason would be questioned. Anyway, this is done. Line 319: servers = [brick.split(":")[0] Line 320:for brick in volinfo[self._volname]['bricks']] Line 321: servers.remove(self._volfileServer) Line 322: if not servers: -- To view, visit https://gerrit.ovirt.org/41931 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fee5d023706e2a0613201a9afef7d285b08442f Gerrit-PatchSet: 38 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sahina Bose Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
automat...@ovirt.org has posted comments on this change. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found -- To view, visit https://gerrit.ovirt.org/42940 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieb2e9d224fb1ad9acff6e9a77157851460d29a89 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
Ala Hino has abandoned this change. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. Abandoned Not needed -- To view, visit https://gerrit.ovirt.org/42940 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ieb2e9d224fb1ad9acff6e9a77157851460d29a89 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Verify volume is Replica 3
automat...@ovirt.org has posted comments on this change. Change subject: gluster: Verify volume is Replica 3 .. Patch Set 39: * Update tracker::#1123052::OK * Check Bug-Url::OK * Check Public Bug::#1123052::OK, public bug * Check Product::#1123052::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/41931 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fee5d023706e2a0613201a9afef7d285b08442f Gerrit-PatchSet: 39 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sahina Bose Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
automat...@ovirt.org has posted comments on this change. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42952 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3014723e1e3d1164a986f5e04208e199396b21b0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
Ala Hino has uploaded a new change for review. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. gluster:refactor: Refactor GlusterFSConnection class Gluster volume info is required when validating that gluster volume is replica 3 and when calculating gluster backup servers option. volinfo property introduced to save multiple calls to get gluster volume info. Change-Id: I3014723e1e3d1164a986f5e04208e199396b21b0 Signed-off-by: Ala Hino --- M vdsm/storage/storageServer.py 1 file changed, 15 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/42952/1 diff --git a/vdsm/storage/storageServer.py b/vdsm/storage/storageServer.py index 55f9b84..c34d216 100644 --- a/vdsm/storage/storageServer.py +++ b/vdsm/storage/storageServer.py @@ -283,6 +283,9 @@ options=options, mountClass=mountClass) self._backup_servers_option = None +self._volinfo = None +self._volfileServer, volname = self._remotePath.split(":", 1) +self._volname = volname.strip('/') @property def options(self): @@ -291,23 +294,29 @@ return ",".join( p for p in (self._options, self._backup_servers_option) if p) +@property +def volinfo(self): +if self._volinfo is None: +self._volinfo = self._get_gluster_volinfo() +return self._volinfo + def _get_backup_servers_option(self): if "backup-volfile-servers" in self._options: self.log.warn("Using user specified backup-volfile-servers option") return "" -volfileServer, volname = self._remotePath.split(":", 1) -volname = volname.strip('/') -volInfo = supervdsm.getProxy().glusterVolumeInfo(volname, - volfileServer) servers = [brick.split(":")[0] - for brick in volInfo[volname]['bricks']] -servers.remove(volfileServer) + for brick in self.volinfo[self._volname]['bricks']] +servers.remove(self._volfileServer) if not servers: return "" return "backup-volfile-servers=" + ":".join(servers) +def _get_gluster_volinfo(self): +return supervdsm.getProxy().glusterVolumeInfo(self._volname, + self._volfileServer) + class NFSConnection(object): DEFAULT_OPTIONS = ["soft", "nosharecache"] -- To view, visit https://gerrit.ovirt.org/42952 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3014723e1e3d1164a986f5e04208e199396b21b0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hook: diskunmap: To include UNMAP support for disk and lun d...
Amador Pahim has posted comments on this change. Change subject: hook: diskunmap: To include UNMAP support for disk and lun devices .. Patch Set 7: (1 comment) https://gerrit.ovirt.org/#/c/29770/7/vdsm_hooks/diskunmap/before_vm_start.py File vdsm_hooks/diskunmap/before_vm_start.py: Line 43: device = disk.getAttribute('device') Line 44: target = disk.getElementsByTagName('target')[0] Line 45: bus = target.getAttribute('bus') Line 46: if ((device == 'disk' or device == 'lun') Line 47:and (bus == 'scsi' or bus == 'ide')): > Why not virtio? virtio does not support discard. Only IDE and virtio-scsi do. Line 48: driver = disk.getElementsByTagName('driver')[0] Line 49: driver.setAttribute('discard', 'unmap') Line 50: Line 51: -- To view, visit https://gerrit.ovirt.org/29770 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36385f1af24043755b3d4b6594bbe598b0d9518d Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim Gerrit-Reviewer: Amador Pahim Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Fabian Deutsch Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Yoav Kleinberger Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding the StorageDomain.upgradeVersion verb
automat...@ovirt.org has posted comments on this change. Change subject: adding the StorageDomain.upgradeVersion verb .. Patch Set 5: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42478 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id8fb87c6e04e48072ec90d07a0f4c7a61411a808 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.updateVmData()
automat...@ovirt.org has posted comments on this change. Change subject: adding StorageDomain.updateVmData() .. Patch Set 2: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42930 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id79e7d6375e60216d1eb9a58e5a5c087db98625f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.getBackedUpVmsInfo
automat...@ovirt.org has posted comments on this change. Change subject: adding StorageDomain.getBackedUpVmsInfo .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42129 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie2896897bccc493457695181436a55f688bb596f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hook: diskunmap: To include UNMAP support for disk and lun d...
Dan Kenigsberg has posted comments on this change. Change subject: hook: diskunmap: To include UNMAP support for disk and lun devices .. Patch Set 7: (1 comment) https://gerrit.ovirt.org/#/c/29770/7/vdsm_hooks/diskunmap/before_vm_start.py File vdsm_hooks/diskunmap/before_vm_start.py: Line 43: device = disk.getAttribute('device') Line 44: target = disk.getElementsByTagName('target')[0] Line 45: bus = target.getAttribute('bus') Line 46: if ((device == 'disk' or device == 'lun') Line 47:and (bus == 'scsi' or bus == 'ide')): Why not virtio? Line 48: driver = disk.getElementsByTagName('driver')[0] Line 49: driver.setAttribute('discard', 'unmap') Line 50: Line 51: -- To view, visit https://gerrit.ovirt.org/29770 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36385f1af24043755b3d4b6594bbe598b0d9518d Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim Gerrit-Reviewer: Amador Pahim Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Fabian Deutsch Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Yoav Kleinberger Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Verify volume is Replica 3
Nir Soffer has posted comments on this change. Change subject: gluster: Verify volume is Replica 3 .. Patch Set 38: (1 comment) https://gerrit.ovirt.org/#/c/41931/38/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 314: self.log.warn("Using user specified backup-volfile-servers option") Line 315: return "" Line 316: Line 317: volinfo = supervdsm.getProxy().glusterVolumeInfo(self._volname, Line 318: self._volfileServer) This patch does not make sense - why are you duplicating the call to glusterVolumeInfo? The correct way to break this into smaller patches is: - Extract volinto property and use it in _get_backup_server_option - Add validate interface and implement it using new volinfo property Line 319: servers = [brick.split(":")[0] Line 320:for brick in volinfo[self._volname]['bricks']] Line 321: servers.remove(self._volfileServer) Line 322: if not servers: -- To view, visit https://gerrit.ovirt.org/41931 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fee5d023706e2a0613201a9afef7d285b08442f Gerrit-PatchSet: 38 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sahina Bose Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: rwlock: Support timed acquire
Nir Soffer has posted comments on this change. Change subject: rwlock: Support timed acquire .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/42909/6/lib/vdsm/rwlock.py File lib/vdsm/rwlock.py: Line 89: def __init__(self, wants_write): Line 90: self.wants_write = wants_write Line 91: self._event = threading.Event() Line 92: Line 93: def wait(self, timeout): > I think timeout=None is a good idea if Waiter is intended for external cons This is intended for internal use only, but using timeout=None make it more clear, so I'll add it. Line 94: if not self._event.wait(timeout): Line 95: raise Timeout("Timeout acquiring lock for %s" % Line 96: "writing" if self.wants_write else "reading") Line 97: -- To view, visit https://gerrit.ovirt.org/42909 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd9b11452d74b566b8989f41244f2d0534327214 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: rwlock: Add simpler RWLock
Nir Soffer has posted comments on this change. Change subject: rwlock: Add simpler RWLock .. Patch Set 4: Code-Review-1 - Fix reader wakeup - Make this compatible with older misc.RWLock -- To view, visit https://gerrit.ovirt.org/42908 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2466c137c89598772fb46347eb02195916883cac Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Add RWLock tests
Nir Soffer has posted comments on this change. Change subject: tests: Add RWLock tests .. Patch Set 4: Code-Review-1 More missing tests: - When waking up blocked readers, all readers up to the first writer should wake up. Current code acquire a read lock and wake up the next reader only when releasing a read lock. -- To view, visit https://gerrit.ovirt.org/42907 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If404f0e8c68fcdb2f7643bdd6d5c1f97f230a227 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: rwlock: Add simpler RWLock
Nir Soffer has posted comments on this change. Change subject: rwlock: Add simpler RWLock .. Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/42908/4/lib/vdsm/rwlock.py File lib/vdsm/rwlock.py: Line 31: self._waiters = [] Line 32: self._readers = set() Line 33: self._writer = None Line 34: Line 35: def acquireWrite(self): > Can we have pep8 method names? I'm tried to keep compatibility with misc.RWLock, so this can version can replace the old one. But it seems that nobody is calling the old code acquireRead and acquireWrite - only shared and exclusive context managers are used in the current code. So I will change the api to pep8 style. Line 36: with self._lock: Line 37: if self._writer or self._readers or self._waiters: Line 38: self._wait(True) Line 39: self._writer = threading.current_thread() Line 73: self._lock.acquire() Line 74: finally: Line 75: self._waiters.remove(waiter) Line 76: Line 77: def _wakeup_waiter(self): > Does this wake up all queued readers after write unlock? This wakes up exactly one waiter, the one that got first in the queue. If there are multiple readers, each will wake up the next. But it looks like that a reader will wake up the next reader when releasing the read lock, instead of when taking the read lock. I will add another test for this. Line 78: if self._readers and self._waiters[0].wants_write: Line 79: return Line 80: self._waiters[0].wakeup() Line 81: -- To view, visit https://gerrit.ovirt.org/42908 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2466c137c89598772fb46347eb02195916883cac Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Verify volume is Replica 3
Ala Hino has posted comments on this change. Change subject: gluster: Verify volume is Replica 3 .. Patch Set 37: (1 comment) https://gerrit.ovirt.org/#/c/41931/37/vdsm/storage/storageServer.py File vdsm/storage/storageServer.py: Line 298: @property Line 299: def options(self): Line 300: backup_servers_option = self._get_backup_servers_option() Line 301: return ",".join(p for p in (self._options, backup_servers_option) if p) Line 302: > This change require access to volume info, which was accessed before by _ge Done. Returned original code plus validate logic and added new patch (https://gerrit.ovirt.org/42940/) to do the refactor. Line 303: @property Line 304: def volinfo(self): Line 305: if self._volinfo is None: Line 306: self._volinfo = self._get_gluster_volinfo() -- To view, visit https://gerrit.ovirt.org/41931 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fee5d023706e2a0613201a9afef7d285b08442f Gerrit-PatchSet: 37 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sahina Bose Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
automat...@ovirt.org has posted comments on this change. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42940 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieb2e9d224fb1ad9acff6e9a77157851460d29a89 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster:refactor: Refactor GlusterFSConnection class
Ala Hino has uploaded a new change for review. Change subject: gluster:refactor: Refactor GlusterFSConnection class .. gluster:refactor: Refactor GlusterFSConnection class Gluster volume info is required when validating that gluster volume is replica 3 and when calculating gluster backup servers option. volinfo property introduced to save multiple calls to get gluster volume info. Change-Id: Ieb2e9d224fb1ad9acff6e9a77157851460d29a89 Signed-off-by: Ala Hino --- M vdsm/storage/storageServer.py 1 file changed, 15 insertions(+), 11 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/40/42940/1 diff --git a/vdsm/storage/storageServer.py b/vdsm/storage/storageServer.py index 5ced157..43b2737 100644 --- a/vdsm/storage/storageServer.py +++ b/vdsm/storage/storageServer.py @@ -291,21 +291,23 @@ vfsType=vfsType, options=options, mountClass=mountClass) -self._backup_servers_option = None self._volfileServer, volname = self._remotePath.split(":", 1) self._volname = volname.strip('/') +self._volinfo = None @property def options(self): -if self._backup_servers_option is None: -self._backup_servers_option = self._get_backup_servers_option() -return ",".join( -p for p in (self._options, self._backup_servers_option) if p) +backup_servers_option = self._get_backup_servers_option() +return ",".join(p for p in (self._options, backup_servers_option) if p) + +@property +def volinfo(self): +if self._volinfo is None: +self._volinfo = self._get_gluster_volinfo() +return self._volinfo def validate(self): -volinfo = supervdsm.getProxy().glusterVolumeInfo(self._volname, - self._volfileServer) -replicaCount = volinfo[self._volname]['replicaCount'] +replicaCount = self.volinfo[self._volname]['replicaCount'] if replicaCount not in self.ALLOWED_REPLICA_COUNTS: raise se.UnsupportedGlusterVolumeReplicaCountError(replicaCount) @@ -314,16 +316,18 @@ self.log.warn("Using user specified backup-volfile-servers option") return "" -volinfo = supervdsm.getProxy().glusterVolumeInfo(self._volname, - self._volfileServer) servers = [brick.split(":")[0] - for brick in volinfo[self._volname]['bricks']] + for brick in self.volinfo[self._volname]['bricks']] servers.remove(self._volfileServer) if not servers: return "" return "backup-volfile-servers=" + ":".join(servers) +def _get_gluster_volinfo(self): +return supervdsm.getProxy().glusterVolumeInfo(self._volname, + self._volfileServer) + class NFSConnection(object): DEFAULT_OPTIONS = ["soft", "nosharecache"] -- To view, visit https://gerrit.ovirt.org/42940 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ieb2e9d224fb1ad9acff6e9a77157851460d29a89 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: gluster: Verify volume is Replica 3
automat...@ovirt.org has posted comments on this change. Change subject: gluster: Verify volume is Replica 3 .. Patch Set 38: * Update tracker::#1123052::OK * Check Bug-Url::OK * Check Public Bug::#1123052::OK, public bug * Check Product::#1123052::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/41931 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fee5d023706e2a0613201a9afef7d285b08442f Gerrit-PatchSet: 38 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Darshan N Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Sahina Bose Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: rwlock: Support timed acquire
Dima Kuznetsov has posted comments on this change. Change subject: rwlock: Support timed acquire .. Patch Set 6: Code-Review+1 (1 comment) https://gerrit.ovirt.org/#/c/42909/6/lib/vdsm/rwlock.py File lib/vdsm/rwlock.py: Line 89: def __init__(self, wants_write): Line 90: self.wants_write = wants_write Line 91: self._event = threading.Event() Line 92: Line 93: def wait(self, timeout): I think timeout=None is a good idea if Waiter is intended for external consumption Line 94: if not self._event.wait(timeout): Line 95: raise Timeout("Timeout acquiring lock for %s" % Line 96: "writing" if self.wants_write else "reading") Line 97: -- To view, visit https://gerrit.ovirt.org/42909 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd9b11452d74b566b8989f41244f2d0534327214 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: rwlock: Add simpler RWLock
Dima Kuznetsov has posted comments on this change. Change subject: rwlock: Add simpler RWLock .. Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/42908/4/lib/vdsm/rwlock.py File lib/vdsm/rwlock.py: Line 31: self._waiters = [] Line 32: self._readers = set() Line 33: self._writer = None Line 34: Line 35: def acquireWrite(self): Can we have pep8 method names? Line 36: with self._lock: Line 37: if self._writer or self._readers or self._waiters: Line 38: self._wait(True) Line 39: self._writer = threading.current_thread() Line 73: self._lock.acquire() Line 74: finally: Line 75: self._waiters.remove(waiter) Line 76: Line 77: def _wakeup_waiter(self): Does this wake up all queued readers after write unlock? Line 78: if self._readers and self._waiters[0].wants_write: Line 79: return Line 80: self._waiters[0].wakeup() Line 81: -- To view, visit https://gerrit.ovirt.org/42908 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2466c137c89598772fb46347eb02195916883cac Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Freddy Rolland Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Add RWLock tests
Dima Kuznetsov has posted comments on this change. Change subject: tests: Add RWLock tests .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/42907 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If404f0e8c68fcdb2f7643bdd6d5c1f97f230a227 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Safe device type check for console device
Shmuel Leib Melamud has posted comments on this change. Change subject: virt: Safe device type check for console device .. Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/42881/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3907: if not hasattr(dev, 'alias'): Line 3908: dev.alias = alias Line 3909: Line 3910: for dev in self.conf['devices']: Line 3911: if dev.get('device') == hwclass.CONSOLE and \ > we hit this one time for rng devices, but now I'm starting to see one patte From my point of view - yes - the Engine should send both 'device' and 'type' fields for every device. Line 3912: not dev.get('alias'): Line 3913: dev['alias'] = alias Line 3914: Line 3915: def _getUnderlyingSmartcardDeviceInfo(self): -- To view, visit https://gerrit.ovirt.org/42881 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a0c1adc6c3b2ab8d50b6e3ac712ce2dcf04cc6c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Shmuel Leib Melamud Gerrit-Reviewer: Shmuel Melamud Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding the StorageDomain.upgradeVersion verb
automat...@ovirt.org has posted comments on this change. Change subject: adding the StorageDomain.upgradeVersion verb .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42478 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id8fb87c6e04e48072ec90d07a0f4c7a61411a808 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.updateVmData()
automat...@ovirt.org has posted comments on this change. Change subject: adding StorageDomain.updateVmData() .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42930 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id79e7d6375e60216d1eb9a58e5a5c087db98625f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Add RWLock tests
Nir Soffer has posted comments on this change. Change subject: tests: Add RWLock tests .. Patch Set 4: Missing tests - forbid promotion - take read lock when holding write lock (not sure what is the intention in current code) - reader wait if a writer is waiting, preventing writers starvation - recursive write lock - recursive read lock -- To view, visit https://gerrit.ovirt.org/42907 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If404f0e8c68fcdb2f7643bdd6d5c1f97f230a227 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches