Roy Golan has posted comments on this change. Change subject: core: Disable balloon by default - Patch 1 of 2 ......................................................................
Patch Set 8: Code-Review-1 (3 comments) -1 mainly for the osRepository API verb could the topic be prefixed with ppc so it would be easier to track? .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java Line 102: /** Line 103: * @return map (osId -> compatibility version -> Boolean) that indicates balloon disabled for all OSs and Line 104: * compatibility versions Line 105: */ Line 106: public Map<Integer, Map<Version, Boolean>> getBalloonDisabledByDefaultMatrix(); change name too getBalloonSupportMap like nicHotPlugSupport etc Line 107: Line 108: /** Line 109: * Checks if is recommended disable the OS balloon. Line 110: * @param osId Line 110: * @param osId Line 111: * @param version Line 112: * @return an boolean Line 113: */ Line 114: public boolean isBalloonDisabledByDefault(int osId, Version version); again a much better verb is isBallonEnabled . the word "default" here is not adding information I guess, unless I'm missing the point. its also easier to consume the api - (osRepository.isBalloonEnabled(x,y)) { .... } instead of (! osRepository.isBalloonDisabledByDefault(x,y)) { .... } Line 115: Line 116: /** Line 117: * Checks if that OS network devices support hotplug. Line 118: * @param osId .................................................... File packaging/conf/osinfo-defaults.properties Line 55: Line 56: os.other.devices.disk.hotpluggableInterfaces.value = VirtIO_SCSI, VirtIO Line 57: os.other.devices.disk.hotpluggableInterfaces.value.3.0 = Line 58: Line 59: os.other.devices.balloonDisabledByDefault.value = false probably meant enabledByDefault.value = false ? :) or I'm missing something. also I want this to adhere to the general devices.{device}.properties name-sapce os.{id}.devices.balloon.enabled.value = false os.{id}.devices.balloon.otherProps.value = x Line 60: os.other.devices.audio.value = ich6 Line 61: # See VmInterfaceType.java Line 62: os.other.devices.network.value = rtl8139, e1000, pv Line 63: os.other.devices.network.hotplugSupport.value = true -- To view, visit http://gerrit.ovirt.org/22055 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b048b3d54e5e1732f8a13de8c9d4c1df4b6d8e5 Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gustavo Frederico Temple Pedrosa <gustavo.pedr...@eldorado.org.br> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa <gustavo.pedr...@eldorado.org.br> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Leonardo Bianconi <leonardo.bianc...@eldorado.org.br> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Vitor de Lima <vitor.l...@eldorado.org.br> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-Reviewer: ofri masad <oma...@redhat.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches