Change in vdsm[master]: External hypervisor VMs integration

danken at redhat.com danken at redhat.com
Mon Dec 1 12:41:12 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: External hypervisor VMs integration
......................................................................


Patch Set 30: Code-Review-1

(3 comments)

http://gerrit.ovirt.org/#/c/33309/30/vdsm/v2v.py
File vdsm/v2v.py:

Line 27:     ''' Unexpected error while parsing libvirt domain xml '''
Line 28:     pass
Line 29: 
Line 30: 
Line 31: def getExternalVMs(uri, username, password):
new modules should have their api naming conform to PEP8's: get_external_VMs()
Line 32:     conn = libvirtconnection.open_connection(uri=uri,
Line 33:                                              username=username,
Line 34:                                              passwd=password)
Line 35:     with closing(conn):


Line 49:             ret.append(params)
Line 50:         return ret
Line 51: 
Line 52: 
Line 53: def _memToMb(size, unit):
nit: it's MiB, not Mb.
Line 54:     lunit = unit.lower()
Line 55:     if lunit in ('bytes', 'b'):
Line 56:         return size / 1024 / 1024
Line 57:     elif lunit in ('kib', 'k'):


http://gerrit.ovirt.org/#/c/33309/30/vdsm/virt/vmstatus.py
File vdsm/virt/vmstatus.py:

Line 25: DOWN = 'Down'
Line 26: MIGRATION_DESTINATION = 'Migration Destination'
Line 27: MIGRATION_SOURCE = 'Migration Source'
Line 28: PAUSED = 'Paused'
Line 29: POWERING_DOWN = 'Powering down'
this submodule is supposed to define vdsm's API, which is not exactly like libvirt's when it comes to vm states. The added statuses means nothing within Vdms, and would certainly confuse old Engines.

I think that in the context of v2v, it would be safer to report libvirt's numerical vm status without ad-hoc conversions.
Line 30: POWERING_UP = 'Powering up'
Line 31: POWER_MANAGEMENT_SUSPENDED = 'PMSuspended'
Line 32: REBOOT_IN_PROGRESS = 'RebootInProgress'
Line 33: RESTORING_STATE = 'Restoring state'


-- 
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: 30
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi 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