Change in vdsm[master]: vdsClient: file based mechanism to provide password

ybronhei at redhat.com ybronhei at redhat.com
Tue Feb 25 06:30:00 UTC 2014


Yaniv Bronhaim has posted comments on this change.

Change subject: vdsClient: file based mechanism to provide password
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.ovirt.org/#/c/24733/10/client/vdsClient.py
File client/vdsClient.py:

Line 102:     print
Line 103:     print "Password can be provided as command line argument, path to"
Line 104:     print "file with password or environment variable by providing"
Line 105:     print "auth=file:path or auth=env:name or auth=pass:password"
Line 106: 
currently the vdsClient shows all the commands' info in its usage print, so the above part ^ almost disappear (you may check that). so we can't assume that the user will read this part and understand how to use Password parameter. it should be printed as before in each command that requires password.
Line 107: 
Line 108: def printConf(conf):
Line 109:     try:
Line 110:         print "\n" + conf['vmId']


Line 162:     return ret, length
Line 163: 
Line 164: 
Line 165: def parseArgs(args):
Line 166:     con, length = parseOptions(args[2])
what does 'con' stand for?
Line 167:     if 'auth' in con:
Line 168:         con['password'] = getPassword(con['auth'])
Line 169:     return con
Line 170: 


Line 168:         con['password'] = getPassword(con['auth'])
Line 169:     return con
Line 170: 
Line 171: 
Line 172: def getPassword(string, prefix=''):
the function can get only the value without the prefix, you already splitted it. with that implementation nothing calls it , but it shouldn't contain the prefix that you remove at the beginning
Line 173:     ret = None
Line 174:     (method, value) = string[len(prefix):].split(':', 1)
Line 175:     if method == 'file':
Line 176:         with open(value) as f:


Line 2015:                    'Sends reset signal to the vm'
Line 2016:                    )),
Line 2017:         'setVmTicket': (serv.do_setVmTicket,
Line 2018:                         ('<vmId> <password> <sec> [disconnect|keep|fail], '
Line 2019:                             '[params={}] [args]',
you need to explain args somehow
Line 2020:                          'Set the password to the vm display for the next '
Line 2021:                          '<sec> seconds.',
Line 2022:                          'Optional argument instructs spice regarding '
Line 2023:                          'currently-connected client.',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I363a16e6a7872ca05e19d5f520bdba90fb492374
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yedidyah Bar David <didi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list