Change in vdsm[master]: Unified persistence [3/4] - Restore network config

ybronhei at redhat.com ybronhei at redhat.com
Sun Oct 13 15:14:48 UTC 2013


Yaniv Bronhaim has posted comments on this change.

Change subject: Unified persistence [3/4] - Restore network config
......................................................................


Patch Set 30:

(5 comments)

....................................................
File init/vdsmd_init_common.sh.in
Line 191:             return "${rt}"
Line 192:         fi
Line 193:         touch "${restoration_file}"
Line 194:     fi
Line 195: }
if you don't have any additional operations under the condition please make that way:

 if [ ! -f "${restoration_file}" ]; then
     "@PYTHON@" "@VDSMDIR@/vdsm-restore-net-config" || return 1
      touch "${restoration_file}
 fi
 return 0
Line 196: #### pre-start tasks end ####
Line 197: 
Line 198: 
Line 199: #### post-stop tasks ####


....................................................
File lib/vdsm/tool/restore_nets.py
Line 31:     Restores the networks to what was previously persisted via vdsm.
Line 32:     """
Line 33:     rc, out, err = utils.execCmd([os.path.join(
Line 34:         P_VDSM, 'vdsm-restore-net-config')], raw=True,
Line 35:         sudo=kwargs.get('sudo', False))
why do you still have the sudo option? please explain when this operation needs root privilege, and if it does , why can it run without root privileges?
Line 36:     sys.stdout.write(out)
Line 37:     sys.stderr.write(err)


Line 34:         P_VDSM, 'vdsm-restore-net-config')], raw=True,
Line 35:         sudo=kwargs.get('sudo', False))
Line 36:     sys.stdout.write(out)
Line 37:     sys.stderr.write(err)
Line 38:     return rc
if rc!=0 better to raise an Runtime Exception.. your call, depends how you use the verb


....................................................
File tests/functional/networkTests.py
Line 139:                 func(*args, **kwargs)
Line 140:             except Exception:
Line 141:                 # cleanup
Line 142:                 instance = args[0]
Line 143:                 instance.vdsm_net.restoreNetConfig()
might raise an exception too, and you'll see only that instead of the original exception
Line 144:                 raise
Line 145:         return wrapper
Line 146: 
Line 147:     @cleanupNet


....................................................
File tests/functional/utils.py
Line 118:         self.vdscli.setSafeNetworkConfig()
Line 119: 
Line 120:     @netinfo_altering
Line 121:     def restoreNetConfig(self):
Line 122:         restore(sudo=True)
let supervdsm call that verb if it needs root privilege, we're trying to avoid such calls
Line 123: 
Line 124:     @netinfo_altering
Line 125:     def addNetwork(self, bridge, vlan=None, bond=None, nics=None, opts=None):
Line 126:         result = self.vdscli.addNetwork(bridge,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I73462b160ecfbaa7efe71eed905a3bbd69ee6c23
Gerrit-PatchSet: 30
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Petr Ĺ ebek <psebek at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list