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 <aha...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches