Change in vdsm[master]: PATCH: external VMs integration
fromani at redhat.com
fromani at redhat.com
Mon Oct 6 14:01:41 UTC 2014
Francesco Romani has posted comments on this change.
Change subject: PATCH: external VMs integration
......................................................................
Patch Set 4:
(11 comments)
good start. We can improve some things.
One of the biggest problems is the lack of unit tests, please add.
http://gerrit.ovirt.org/#/c/33309/4//COMMIT_MSG
Commit Message:
Line 7: PATCH: external VMs integration
Line 8:
Line 9: Added v2v module - added ability to list VMs from external source
Line 10: (non KVM) via libvirt.
Line 11:
this is a bit terse. OK for a draft, but you'll need to expand it for review, (shortly) documenting the use cases and why this code is needed.
Line 12: Change-Id: I7dcfb860626a844d1d08590274b508519a33f4a3
http://gerrit.ovirt.org/#/c/33309/4/client/vdsClient.py
File client/vdsClient.py:
Line 1890: return status['status']['code'], status['status']['message']
Line 1891:
Line 1892: def getExternalVMList(self, args):
Line 1893: if len(args) != 3:
Line 1894: raise ValueError('Wrong number of arguments')
not so common but won't hurt
Line 1895: uri, username, password = args
Line 1896: status = self.s.getExternalVMList(uri, username, password)
Line 1897: if status['status']['code'] == 0:
Line 1898: vmList = status['vmList']
Line 1896: status = self.s.getExternalVMList(uri, username, password)
Line 1897: if status['status']['code'] == 0:
Line 1898: vmList = status['vmList']
Line 1899: for vm in vmList:
Line 1900: for key in vm.keys():
if vm is a a dict():
for key, val in vm.iteritems(): # or items()
print key + ': ' + str(val)
Line 1901: print key + ': ' + str(vm[key])
Line 1902:
Line 1903: return status['status']['code'], status['status']['message']
Line 1904:
http://gerrit.ovirt.org/#/c/33309/4/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 3588: #
Line 3589: # Since: 4.17.0
Line 3590: ##
Line 3591: {'map': 'VmProperties',
Line 3592: 'key': 'str', 'value': 'str'}
If this is a fixed set, so if we expect a fixed number of keys, better to document that
Line 3593:
Line 3594: ##
Line 3595: # @VMFullInfo:
Line 3596: #
http://gerrit.ovirt.org/#/c/33309/4/vdsm/v2v.py
File vdsm/v2v.py:
Line 1: import libvirt
missing copyright notice
Line 2: import logging
Line 3: import xml.etree.ElementTree as ET
Line 4:
Line 5: from virt import vmstatus
Line 1: import libvirt
Line 2: import logging
Line 3: import xml.etree.ElementTree as ET
Line 4:
Line 5: from virt import vmstatus
please add another space
Line 6:
Line 7: vmStatus = {libvirt.VIR_DOMAIN_NOSTATE: 'Unknown',
Line 8: libvirt.VIR_DOMAIN_RUNNING: vmstatus.UP,
Line 9: libvirt.VIR_DOMAIN_BLOCKED: 'Blocked',
Line 3: import xml.etree.ElementTree as ET
Line 4:
Line 5: from virt import vmstatus
Line 6:
Line 7: vmStatus = {libvirt.VIR_DOMAIN_NOSTATE: 'Unknown',
constants SHOULD_BE_UPPERCASE
if they are private to module, _PREFIXED_WITH_UNDERSCORE
Line 8: libvirt.VIR_DOMAIN_RUNNING: vmstatus.UP,
Line 9: libvirt.VIR_DOMAIN_BLOCKED: 'Blocked',
Line 10: libvirt.VIR_DOMAIN_PAUSED: vmstatus.PAUSED,
Line 11: libvirt.VIR_DOMAIN_SHUTDOWN: vmstatus.POWERING_DOWN,
Line 9: libvirt.VIR_DOMAIN_BLOCKED: 'Blocked',
Line 10: libvirt.VIR_DOMAIN_PAUSED: vmstatus.PAUSED,
Line 11: libvirt.VIR_DOMAIN_SHUTDOWN: vmstatus.POWERING_DOWN,
Line 12: libvirt.VIR_DOMAIN_SHUTOFF: vmstatus.DOWN,
Line 13: libvirt.VIR_DOMAIN_CRASHED: 'Crushed',
typo :)
Line 14: libvirt.VIR_DOMAIN_PMSUSPENDED: 'PMSuspend'}
Line 15:
Line 16:
Line 17: def memToMb(size, unit):
Line 26: elif unit == 'Tib' or unit == 'T':
Line 27: return size * 1024 * 1024
Line 28:
Line 29:
Line 30: def parseVmElements(xml):
same. If private, prefix with '_'
_parseVmElements
Line 31: ret = {}
Line 32: ''' Memory '''
Line 33: root = ET.fromstring(xml)
Line 34: e = root.find('./currentMemory')
Line 79:
Line 80:
Line 81: def getExternalVMList(uri, username, password):
Line 82: """ return external VMs with its state (up/down...) given uri """
Line 83: def req(credentials, user_data):
can we share some code with libvirtconnection?
We can probably reuse some of its facilities, or make them (more) general.
Line 84: for cred in credentials:
Line 85: if cred[0] == libvirt.VIR_CRED_AUTHNAME:
Line 86: cred[4] = username
Line 87: elif cred[0] == libvirt.VIR_CRED_PASSPHRASE:
Line 89: return 0
Line 90:
Line 91: auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_PASSPHRASE],
Line 92: req, None]
Line 93: try:
why not openReadOnly?
Line 94: conn = libvirt.openAuth(uri, auth, 0)
Line 95: except libvirt.libvirtError as e:
Line 96: logging.error(
Line 97: 'v2v.getExternalVMList: error connection to remove server: %s'
--
To view, visit http://gerrit.ovirt.org/33309
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7dcfb860626a844d1d08590274b508519a33f4a3
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi 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