Dan Kenigsberg has posted comments on this change.
Change subject: [WIP] BZ#855924 fix VM config dictionary
......................................................................
Patch Set 6: I would prefer that you didn't submit this
(1 inline comment)
I think that a much cleaner approach would be to maintain backward
compatibility in the migrationCreate API itself.
Have you considered carrying "cdrom" and "floppy" everywhere? rhev-3.1 could
know how to handle a case where a cdrom appears both as "cdrom" and as
devices[cdrom].
....................................................
File vdsm/vm.py
Line 127: self.log.debug('Destination server is: ' + serverAddress)
Line 128: try:
Line 129: self.log.debug('Initiating connection with destination')
Line 130: self._dst_version = str(max(
Line 131:
self.destServer.getVersionInfo()['info']['supportedRHEVMs']))
has this ever been tested?
What would happen if getVersionInfo() is missing from the destination?
you assume that string comparison would always work for versions. I do not
think this is safe.
I would like to evade this, but maybe it is time to add capability flags to
getCapabilities in order to check if "devices" is supported.
Line 132: status = self.destServer.getVmStats(self._vm.id)
Line 133: if not status['status']['code']:
Line 134: self.log.error("Machine already exists on the
destination")
Line 135: self.status = errCode['exist']
--
To view, visit http://gerrit.ovirt.org/8378
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I33e957f8daac4266d9e19cc008cf830bd3959521
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Peter V. Saveliev <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Peter V. Saveliev <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
