Martin Mucha has posted comments on this change.

Change subject: engine: Add query to retrieve host's bonds
......................................................................


Patch Set 19:

(3 comments)

https://gerrit.ovirt.org/#/c/35378/19/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/GetHostBondsByHostIdQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/GetHostBondsByHostIdQuery.java:

Line 9: import org.ovirt.engine.core.common.businessentities.network.Bond;
Line 10: import 
org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface;
Line 11: import org.ovirt.engine.core.common.queries.IdQueryParameters;
Line 12: 
Line 13: public class GetHostBondsByHostIdQuery<P extends IdQueryParameters> 
extends QueriesCommandBase<P> {
> If you follow my suggestion on a previous patch- filling the slaves in the 
If we opt to do it, I'd have to invest time to alter dao layer and then remove 
this as a absolute minimum. We do not have even that time. And I doubt it will 
be that easy.

We agreed, for time-saving reasons, to do it later.
Done.
Line 14:     public GetHostBondsByHostIdQuery(P parameters) {
Line 15:         super(parameters);
Line 16:     }
Line 17: 


Line 23:         List<Bond> bonds = new ArrayList<>();
Line 24:         for (final VdsNetworkInterface nic : nics) {
Line 25:             if (Boolean.TRUE.equals(nic.getBonded())) {
Line 26:                 String bondName = nic.getName();
Line 27:                 Bond bond = new Bond(bondName);
> Why do you create a new bond entity and not adding the slave to the already
Done
Line 28:                 bond.setId(nic.getId());
Line 29:                 bond.setBondOptions(nic.getBondOptions());
Line 30:                 if (bondsToSlaves.containsKey(bondName)) {
Line 31:                     
bond.getSlaves().addAll(bondsToSlaves.get(bondName));


Line 27:                 Bond bond = new Bond(bondName);
Line 28:                 bond.setId(nic.getId());
Line 29:                 bond.setBondOptions(nic.getBondOptions());
Line 30:                 if (bondsToSlaves.containsKey(bondName)) {
Line 31:                     
bond.getSlaves().addAll(bondsToSlaves.get(bondName));
there's also List data type to hold all slaves. I think it should be changed to 
Set, since order is I believe not important (and probably not even enforce by 
db query), while duplicates should not occur.
Line 32:                 }
Line 33: 
Line 34:                 bonds.add(bond);
Line 35:             }


-- 
To view, visit https://gerrit.ovirt.org/35378
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I41a632340312bc484c8e33699353b075aa45b1a4
Gerrit-PatchSet: 19
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to