Shahar Havivi has posted comments on this change. Change subject: PATCH: external VMs integration ......................................................................
Patch Set 4: (10 comments) 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 revie Done Line 12: Change-Id: I7dcfb860626a844d1d08590274b508519a33f4a3 http://gerrit.ovirt.org/#/c/33309/4/client/vdsClient.py File client/vdsClient.py: 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(): Done 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 d Done 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 Done 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 Done 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 do you mean vmStatus variable? 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 :) Done 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 '_' Done 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? I thought about that, its looks like too oriented to qemu and have one connection which it close on exit - and here we want a rapid connection that will close soon... What do you think? 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? I don't think that we can open readonly without authentication 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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
