Shu Ming has posted comments on this change.

Change subject: forceVMstart: Initial commit
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(5 inline comments)

....................................................
File contrib/forceVMstart.py
Line 54:     sys.exit(1)
Line 55: 
Line 56: # General Macros
Line 57: VERSION = "1.0.0"
Line 58: VDSM_PORT = "54321"
It is better to let the user to give this port in the command option list. 
Another way is to get the port from local vdsm.conf file.  Only If the port not 
given in the command option list,  a default number "54321" should be used.
Line 59: 
Line 60: #DEBUG MODE
Line 61: DEBUG = "False" # True or False
Line 62: 


Line 82:             print "Unable to connect %s" % server
Line 83:             sk.close()
Line 84:             return -1
Line 85: 
Line 86:         self.s = vdscli.connect(server + ':' + port, self.useSSL, 
self.truststore)
Can we have a more meaningful name for 's', like "conn_info"?
Line 87: 
Line 88:         print "OK, Connected to vdsmd!"
Line 89:         return 0
Line 90: 


Line 97:     def getIpManagementIP(self):
Line 98:         """get the IP from management interface"""
Line 99: 
Line 100:         # TODO: avoid this kind of hack, find a better approach 
(vdsClient provide the IP of ovirtmgmt/rhevm interface?)
Line 101:         # strCmd = "ifconfig  ovirtmgmt | grep \"inet addr\" | cut -d 
\':\' -f 2 | cut -d \' \' -f 1"
It seems that this file is not PEP8 clean.  Please fix it.
Line 102: 
Line 103:         # Code to make it work for the rhevm or the ovirtmgmt 
interface
Line 104: 
Line 105:         strCmd = "ifconfig ovirtmgmt 2>/dev/null|grep inet|grep -v 
inet6|awk '{print $2}'|cut -d ':' -f2"


Line 192:             print "Cannot execute getConnectedStoragePoolsList()"
Line 193:             sys.exit(1)
Line 194: 
Line 195:         for entry in list['poollist']:
Line 196:             self.spUUID = entry
I am not sure why we need a "for" loop here.  It seems self.spUUID always equal 
to the last entry in list['poollist'].
Line 197: 
Line 198:         if not self.spUUID:
Line 199:             print "Cannot locate Storage Pools List.. aborting!"
Line 200:             sys.exit(1)


Line 405:                         while (i <> len(checkvms)):
Line 406:                             if self.vmName == checkvms[i]:
Line 407:                                 nrmVms = nrmVms + 1
Line 408:                                 self.startVM(cmd, destHostStart)
Line 409:                             i += 1
I really don't like this big loop from line 283.
Line 410: 
Line 411:         print "Total VMs found: %s" % nrmVms
Line 412: 
Line 413:     def startVM(self, cmd, destHostStart):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a70b31ce0730194880406701316f219c9f92ceb
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pablo Iranzo Gómez <[email protected]>
Gerrit-Reviewer: Amador Pahim <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Lee Yarwood <[email protected]>
Gerrit-Reviewer: Pablo Iranzo Gómez <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to