Change in vdsm[ovirt-3.2]: Don't fail silently when ifup fails.

asegurap at redhat.com asegurap at redhat.com
Wed Feb 6 17:50:19 UTC 2013


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/11820

to review the following change.

Change subject: Don't fail silently when ifup fails.
......................................................................

Don't fail silently when ifup fails.

Up until now we discarded ifup return codes and ignored the possible
errors present in ifup stdout. Use this information to perform better
error reporting.

The tests change is due to the fact that now that we treat ifup
errors as excepcional occurences, for unit testing the persistent
backups (thing which can be done by a regular user) we do not need
to test ifup and ifdown of the current config as it is not a thing
that the unprivileged user is normally allowed to do.

Change-Id: I1cc9dcc0a6b55d36fc937e1d364bd9c256ecd70a
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
Bug-Url: https://bugzilla.redhat.com/856737
Reviewed-on: http://gerrit.ovirt.org/8415
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Tested-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/configNetworkTests.py
M vdsm/configNetwork.py
M vdsm/neterrors.py
3 files changed, 13 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/20/11820/1

diff --git a/tests/configNetworkTests.py b/tests/configNetworkTests.py
index 5f7cce4..25598b4 100644
--- a/tests/configNetworkTests.py
+++ b/tests/configNetworkTests.py
@@ -362,6 +362,10 @@
             (netinfo, 'NET_CONF_BACK_DIR',
              os.path.join(self._tempdir, 'netback')),
             (netinfo, 'NET_CONF_DIR', self._tempdir),
+            (netinfo, 'NET_CONF_PREF',
+             os.path.join(self._tempdir, 'ifcfg-')),
+            (configNetwork, 'ifdown', lambda x: 0),
+            (configNetwork, 'ifup', lambda *x: 0),
         ]):
             #after vdsm package is installed, the 'vdsm' account will be
             #created if no 'vdsm' account, we should skip this test
diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index 53debfa..02ebea2 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -62,7 +62,14 @@
 
 def ifup(iface, async=False):
     "Bring up an interface"
-    _ifup = lambda netIf: execCmd([constants.EXT_IFUP, netIf], raw=True)
+    def _ifup(netIf):
+        rc, out, err = execCmd([constants.EXT_IFUP, netIf], raw=False)
+
+        if rc != 0:
+            # In /etc/sysconfig/network-scripts/ifup* the last line usually
+            # contains the error reason.
+            raise ConfigNetworkError(ne.ERR_FAILED_IFUP, out[-1])
+        return rc, out, err
 
     if async:
         # wait for dhcp in another thread,
diff --git a/vdsm/neterrors.py b/vdsm/neterrors.py
index 65e2674..5ce38b6 100644
--- a/vdsm/neterrors.py
+++ b/vdsm/neterrors.py
@@ -27,4 +27,5 @@
 ERR_BAD_VLAN = 26
 ERR_BAD_BRIDGE = 27
 ERR_USED_BRIDGE = 28
+ERR_FAILED_IFUP = 29
 ERR_LOST_CONNECTION = 10    # noConPeer


--
To view, visit http://gerrit.ovirt.org/11820
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1cc9dcc0a6b55d36fc937e1d364bd9c256ecd70a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.2
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list