Change in vdsm[master]: External hypervisor VMs integration

nsoffer at redhat.com nsoffer at redhat.com
Wed Nov 26 10:39:31 UTC 2014


Nir Soffer has posted comments on this change.

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


Patch Set 28: Code-Review-1

(1 comment)

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

Line 64:         return size * 1024
Line 65:     elif lunit in ('tib', 't'):
Line 66:         return size * 1024 * 1024
Line 67:     else:
Line 68:         raise InvalidVMConfiguration("Invalid memory unit: %r", unit)
This will create this message that does no make any sense:

    Invalid Invalid memory unit: %r value: 'unit value'

Seems that the InvalidConfiguration class you created does not fit all the uses you need. You want to raise it either with 2 values (for field_name, bad_value), or for general errors like invalid memory unit.

So the simple solution is to remove the implementation of InvalidConfigration, and just call it with the message you need based on context.

If you think that having a special class for 2 errors is worth the effort, create a subclass (e.g. InvalidFieldValue) that log field_name, bad_value pair.
Line 69: 
Line 70: 
Line 71: def _addGeneralInfo(root, params):
Line 72:     e = root.find('./uuid')


-- 
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: 28
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