Moti Asayag has posted comments on this change.
Change subject: core: Allow Non-Required Networks Not On Host (Do not submit)
......................................................................
Patch Set 2: (5 inline comments)
What should be the case of hotplug-nic ? doesn't same mitigation for network
requirement should be applied there as well ?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsSelector.java
Line 336:
Line 337: /**
Line 338: * Determine whether all required Networks are attached to the
Host's Nics. A required Network is defined as:
Line 339: * Depending on
ConfigValue.OnlyRequiredNetworksMandatoryForVdsSelection: 1. false: any network
that is defined on
Line 340: * an Active vNic of the VM. 2. true: a Cluster-Required Network
that is defined on an Active vNic of the VM.
please format the java doc
Line 341: * @param vdsId
Line 342: * The Host id.
Line 343: * @return <code>true</code> if all required Networks are
attached to a Host Nic, otherwise, <code>false</code>.
Line 344: */
Line 341: * @param vdsId
Line 342: * The Host id.
Line 343: * @return <code>true</code> if all required Networks are
attached to a Host Nic, otherwise, <code>false</code>.
Line 344: */
Line 345: boolean areRequiredNetworksAvailable(VDS vds) {
although not part of the patch - please make this method private, as not being
called from elsewhere.
Line 346: final List<VdsNetworkInterface> allInterfacesForVds =
getInterfaceDAO().getAllInterfacesForVds(vds.getId());
Line 347: final List<Network> clusterNetworks =
getNetworkDAO().getAllForCluster(vds.getvds_group_id());
Line 348: final Map<String, Network> networksByName =
NetworkUtils.networksByName(clusterNetworks);
Line 349:
Line 344: */
Line 345: boolean areRequiredNetworksAvailable(VDS vds) {
Line 346: final List<VdsNetworkInterface> allInterfacesForVds =
getInterfaceDAO().getAllInterfacesForVds(vds.getId());
Line 347: final List<Network> clusterNetworks =
getNetworkDAO().getAllForCluster(vds.getvds_group_id());
Line 348: final Map<String, Network> networksByName =
NetworkUtils.networksByName(clusterNetworks);
please replace this with Entities.entitiesByName(clusterNetworks)
I think that NetworkUtils.networksByName should be eliminated and be replaced
with NetworkUtils.networksByName() everywhere.
Line 349:
Line 350: boolean onlyRequiredNetworks =
Line 351: Config.<Boolean>
GetValue(ConfigValues.OnlyRequiredNetworksMandatoryForVdsSelection);
Line 352: for (final VmNetworkInterface vmIf : getVmNICs()) {
Line 368: private NetworkDAO getNetworkDAO() {
Line 369: return DbFacade.getInstance().getNetworkDAO();
Line 370: }
Line 371:
Line 372: private boolean networkRequiredOnVds(VmNetworkInterface vmIf,
s/vmIf/vmIface
Line 373: Map<String, Network> networksByName,
Line 374: boolean onlyRequiredNetworks) {
Line 375: boolean networkRequiredOnVds = true;
Line 376: if (!vmIf.isActive()) {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 1508: @TypeConverterAttribute(String.class)
Line 1509: @DefaultValueAttribute("")
Line 1510: IPTablesConfigForGluster(388),
Line 1511:
Line 1512: @Reloadable
I think that config value annotated as Reloadable should appear on the
engine-config.properties with a proper description since it implies the value
could be modified by the user. So either remove the Reloadable or add an entry
to engine-config.properties
Line 1513: @TypeConverterAttribute(Boolean.class)
Line 1514: @DefaultValueAttribute("false")
Line 1515: OnlyRequiredNetworksMandatoryForVdsSelection(384),
Line 1516:
--
To view, visit http://gerrit.ovirt.org/7992
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4909a658bf729d839c9f01268f1295e43391a2c3
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Muli Salem <[email protected]>
Gerrit-Reviewer: Itzik Brown <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Muli Salem <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
