Arik Hadas has posted comments on this change.
Change subject: core: fixed video device setting on import vm/template
......................................................................
Patch Set 3: (2 inline comments)
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java
Line 365: // Note: the fetching of 'default display type' should happen
before reading
Line 366: // the hardware section
Line 367: node =
content.SelectSingleNode(getDefaultDisplayTypeStringRepresentation());
Line 368: if (node != null) {
Line 369: if (!StringHelper.isNullOrEmpty(node.InnerText)) {
I tried to make minimum changes in this patch because it fix a bug that its
target is 3.1.z..
I want to make further refactoring in this class, besides the cleanup patch I
already pushed (see http://gerrit.ovirt.org/#/c/10526/) - eliminate code
duplication of getting the node, check that it's not null, check that its inner
text is not null or empty.. replacing the usage of StringHelper with
StringUtils can be made in that patch
Line 370: defaultDisplayType =
DisplayType.forValue(Integer.parseInt(node.InnerText));
Line 371: vmBase.setdefault_display_type(defaultDisplayType);
Line 372: }
Line 373: }
Line 500: // we need special handling for Monitor to define it as
vnc or spice
Line 501: if (Integer.valueOf(OvfHardware.Monitor) == resourceType)
{
Line 502: // if default display type is defined in the ovf, set
the video device that is suitable for it
Line 503: if (defaultDisplayType != null) {
Line 504:
vmDevice.setDevice(defaultDisplayType.getVmDeviceType().getName());
but then the control flow of the method becomes more complicated.. I think that
we can stick with the 'single exit point' principle here, as the readability is
not too bad (not like in methods which have flags all over just to follow the
single exit point principle)
Line 505: }
Line 506: else {
Line 507: // get number of monitors from VirtualQuantity in
OVF
Line 508: if
(node.SelectSingleNode(OvfProperties.VMD_VIRTUAL_QUANTITY, _xmlNS) != null
--
To view, visit http://gerrit.ovirt.org/10525
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id1bfaa3414507a982b9f22aa21fb3d05fe87e84d
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
