Change in vdsm[master]: Added _execGlusterXml()

danken at redhat.com danken at redhat.com
Thu Aug 30 22:21:19 UTC 2012


Dan Kenigsberg has posted comments on this change.

Change subject: Added _execGlusterXml()
......................................................................


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

(3 inline comments)

....................................................
File vdsm/gluster/cli.py
Line 72:     rc, out, err = utils.execCmd(cmd)
Line 73:     if rc != 0:
Line 74:         raise ge.GlusterCmdExecFailedException(rc, out, err)
Line 75:     try:
Line 76:         tree = etree.fromstring('\n'.join(out))
you'd save some time if you call execCmd(raw=True)
Line 77:         rv = tree.find('opRet').text
Line 78:         msg = tree.find('opErrstr').text
Line 79:     except (etree.ParseError, AttributeError):
Line 80:         raise ge.GlusterXmlErrorException(err=out)


Line 78:         msg = tree.find('opErrstr').text
Line 79:     except (etree.ParseError, AttributeError):
Line 80:         raise ge.GlusterXmlErrorException(err=out)
Line 81:     if rv == '0':
Line 82:         return (tree, out)
'tree' is 'out' parsed. so why do you need to return both?
Line 83:     else:
Line 84:         raise ge.GlusterCmdFailedException(rc=rv, err=[msg])
Line 85: 
Line 86: 


Line 80:         raise ge.GlusterXmlErrorException(err=out)
Line 81:     if rv == '0':
Line 82:         return (tree, out)
Line 83:     else:
Line 84:         raise ge.GlusterCmdFailedException(rc=rv, err=[msg])
here rc is set to a string; else where it is an int. that's legal but confusing.
Line 85: 
Line 86: 
Line 87: def _parseVolumeInfo(out):
Line 88:     if not out[0].strip():


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385f992005d446b417be84eb7ff484e4edf6e5b6
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list