Change in vdsm[master]: py3: make toolBondingTests pass

[Dan Kenigsberg]

Dan Kenigsberg has uploaded a new change for review.

Change subject: py3: make toolBondingTests pass
......................................................................

py3: make toolBondingTests pass

I don't really know why, but under Python 3, file.close() seems to issue
another attempt to flush "invalid" data. The added code explicitly
ignores the ensuing exception.

Change-Id: I4487b787c3a14ca9508847572622f431f1188e7f
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M lib/vdsm/network/netinfo/bonding.py
M tests/Makefile.am
2 files changed, 9 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/65666/1

diff --git a/lib/vdsm/network/netinfo/bonding.py b/lib/vdsm/network/netinfo/bonding.py
index 0ca2259..2f40b88 100644
--- a/lib/vdsm/network/netinfo/bonding.py
+++ b/lib/vdsm/network/netinfo/bonding.py
@@ -218,7 +218,7 @@
     e.g. 'ad_num_ports' or 'slaves'.
     """
     return dict(((opt, val) for (opt, val)
-                 in _bond_opts_name2numeric(bond).iteritems()
+                 in six.iteritems(_bond_opts_name2numeric(bond))
                  if opt not in EXCLUDED_BONDING_ENTRIES))
 
 
@@ -268,6 +268,14 @@
 
             vals[name] = numeric
 
+        # On Python 3, the final close seems to issue another flush attempt,
+        # which results in a repeated exception. Ignore it.
+        try:
+            opt_file.close()
+        except EnvironmentError as e:
+            if e.errno not in (errno.EINVAL, errno.EPERM, errno.EACCES):
+                raise
+
     return vals
 
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index e96f226..6eb815b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -227,7 +227,6 @@
 	storage_workarounds_test.py \
 	storagefakelibTests.py \
 	storagetestlib_test.py \
-	toolBondingTests.py \
 	unicode_test.py \
 	utilsTests.py \
 	v2vTests.py \


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4487b787c3a14ca9508847572622f431f1188e7f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;

gerrit-hooks has posted comments on this change.

Change subject: py3: make toolBondingTests pass
......................................................................


Patch Set 1:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4487b787c3a14ca9508847572622f431f1188e7f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Petr Horáček has posted comments on this change.

Change subject: py3: make toolBondingTests pass
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/65666/1/lib/vdsm/network/netinfo/bonding.py
File lib/vdsm/network/netinfo/bonding.py:

Line 258: 
Line 259: 
Line 260: def _bond_opts_name2numeric_scan(opt_path):
Line 261:     vals = {}
Line 262:     with open(opt_path, 'w') as opt_file:
why do we use open as a context manager if we close it ourselves anyways? won't be
better to create open_ignoring_flush_error that will do open and ignoring close?
Line 263:         for numeric_val in range(32):
Line 264:             name, numeric = _bond_opts_name2numeric_getval(opt_path, opt_file,
Line 265:                                                            numeric_val)
Line 266:             if name is None:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4487b787c3a14ca9508847572622f431f1188e7f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <leon.otium(a)gmail.com&gt;
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: py3: make toolBondingTests pass
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/65666/1/lib/vdsm/network/netinfo/bonding.py
File lib/vdsm/network/netinfo/bonding.py:

Line 258: 
Line 259: 
Line 260: def _bond_opts_name2numeric_scan(opt_path):
Line 261:     vals = {}
Line 262:     with open(opt_path, 'w') as opt_file:
>  Also this open the file in text more instead of binary in python 3.
> Better  
right! thanks!
Line 263:         for numeric_val in range(32):
Line 264:             name, numeric = _bond_opts_name2numeric_getval(opt_path, opt_file,
Line 265:                                                            numeric_val)
Line 266:             if name is None:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4487b787c3a14ca9508847572622f431f1188e7f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <leon.otium(a)gmail.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: py3: make toolbonding_tests pass
......................................................................


Patch Set 3: Code-Review+2 Verified+1

passes new functional tests

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4487b787c3a14ca9508847572622f431f1188e7f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <leon.otium(a)gmail.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has submitted this change and it was merged.

Change subject: py3: make toolbonding_tests pass
......................................................................


py3: make toolbonding_tests pass

I don't really know why, but under Python 3, file.close() seems to issue
another attempt to flush "invalid" data. This patch opens
/sys/class/net/bond0/bonding/option in a binary non-buffered mode, in
order to avoid this hassle.

Change-Id: I4487b787c3a14ca9508847572622f431f1188e7f
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M lib/vdsm/network/netinfo/bonding.py
M tests/Makefile.am
2 files changed, 7 insertions(+), 6 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Verified; Looks good to me, approved
  Edward Haas: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I4487b787c3a14ca9508847572622f431f1188e7f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <leon.otium(a)gmail.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Ondřej Svoboda <osvoboda(a)redhat.com&gt;
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;