Moti Asayag has posted comments on this change. Change subject: core: add support for tunnel migration ......................................................................
Patch Set 3: (5 inline comments) .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java Line 151: private boolean allowConsoleReconnect; Line 152: Line 153: /** Line 154: * if this field is null then value should be taken from cluster Line 155: */ hibernate annotations could be spared. Line 156: @Column(name = "tunnel_migration") Line 157: private Boolean tunnelMigration; Line 158: Line 159: /** Line 811: public void setOvfVersion(String ovfVersion) { Line 812: this.ovfVersion = ovfVersion; Line 813: } Line 814: Line 815: public Boolean isTunnelMigration() { since this method returns an object and not a primitive boolean, I'd rather call if getTunnelMigration so if used from if statements the developer will be aware of the possibility the attribute wasn't initialized and consider checking if it value is null. Line 816: return tunnelMigration; Line 817: } Line 818: Line 819: public void setTunnelMigration(Boolean value) { .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java Line 1493: public boolean isNotRunning() { Line 1494: return getStatus().isNotRunning(); Line 1495: } Line 1496: Line 1497: public Boolean isTunnelMigration() { Please change method name to getTunnelMigration(). See blown explanation on VmBase. Line 1498: return vmStatic.isTunnelMigration(); Line 1499: } Line 1500: Line 1501: public void setTunnelMigration(Boolean value) { .................................................... File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/MigrateBrokerVDSCommand.java Line 20: migrationInfo.put(VdsProperties.src, String.format("%1$s", parameters.getSrcHost())); Line 21: migrationInfo.put(VdsProperties.dst, String.format("%1$s", parameters.getDstHost())); Line 22: migrationInfo.put(VdsProperties.method, migMethod); Line 23: if (parameters.getTunnelMigration() != null) { Line 24: migrationInfo.put(VdsProperties.tunneled, String.valueOf(parameters.getTunnelMigration())); since you already verified parameters.getTunnelMigration() is not null you can use parameters.getTunnelMigration().toString() Line 25: } Line 26: } Line 27: Line 28: @Override .................................................... File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java Line 252: public static final String offline = "offline"; Line 253: public static final String online = "online"; Line 254: public static final String domains = "storageDomains"; Line 255: public static final String hooks = "hooks"; Line 256: public static final String tunneled = "tunneled"; Please change constant name to upper-case so it complies with java naming convention. Line 257: Line 258: // storage domains Line 259: public static final String code = "code"; Line 260: public static final String lastCheck = "lastCheck"; -- To view, visit http://gerrit.ovirt.org/11062 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6fa684df7376f8470fe729d590ce0e63d39c4d1e Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches