Tomas Jelinek has posted comments on this change. Change subject: core: adjustments needed to support instance types ......................................................................
Patch Set 24: (9 comments) http://gerrit.ovirt.org/#/c/23828/24/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java: Line 156: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_MIN_MEMORY_CANNOT_EXCEED_MEMORY_SIZE); Line 157: } Line 158: Line 159: return returnValue; Line 160: > redundant line Done Line 161: } Line 162: Line 163: private boolean checkDomain() { Line 164: if (getParameters().getVmTemplateData().getVmInit() != null && Line 264: VmDeviceUtils.updateSmartcardDevice(getVmTemplateId(), getParameters().getVmTemplateData().isSmartcardEnabled()); Line 265: // update audio device Line 266: VmDeviceUtils.updateAudioDevice(mOldTemplate, Line 267: getVmTemplate(), Line 268: getVdsGroup() != null ? getVdsGroup().getcompatibility_version() : null, > maybe instead of null we'll use the DC level? its closer than the default f Done Line 269: getParameters().isSoundDeviceEnabled()); Line 270: Line 271: VmDeviceUtils.updateConsoleDevice(getVmTemplateId(), getParameters().isConsoleEnabled()); Line 272: } http://gerrit.ovirt.org/#/c/23828/24/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateCommand.java: Line 191: jobProperties.put(VdcObjectType.VmTemplate.name().toLowerCase(), getVmTemplateName()); Line 192: } Line 193: return jobProperties; Line 194: } Line 195: > redundent line Done http://gerrit.ovirt.org/#/c/23828/24/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/commands/AffinityGroupCRUDCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/commands/AffinityGroupCRUDCommand.java: Line 40: vmStatic = getVmStaticDAO().get(vmId); Line 41: if (vmStatic == null) { Line 42: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_INVALID_VM_FOR_AFFINITY_GROUP); Line 43: } Line 44: if (!ObjectUtils.equals(vmStatic.getVdsGroupId(), getVdsGroupId())) { > use jdk7 Objects. you some more places you used it too Done Line 45: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_IN_AFFINITY_GROUP_CLUSTER); Line 46: } Line 47: if (vmSet.contains(vmStatic.getId())) { Line 48: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DUPLICTE_VM_IN_AFFINITY_GROUP); http://gerrit.ovirt.org/#/c/23828/24/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmStatic.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmStatic.java: Line 47: Line 48: @EditableField Line 49: private boolean useHostCpuFlags; Line 50: Line 51: @EditableField > on every status? yes, no issues editing it... Line 52: private Guid instanceTypeId; Line 53: private Guid imageTypeId; Line 54: Line 55: @EditableField http://gerrit.ovirt.org/#/c/23828/24/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java: Line 415: mVdsGroupId = getVds().getVdsGroupId(); Line 416: mVdsGroup = getVdsGroupDAO().get(mVdsGroupId); Line 417: } else if (getVm() != null) { Line 418: mVdsGroupId = getVm().getVdsGroupId(); Line 419: if (mVdsGroupId != null) { > the getVdsGroupDAO().get(null) will return null so I'd rather not add here Done Line 420: mVdsGroup = getVdsGroupDAO().get(mVdsGroupId); Line 421: } Line 422: } else if (getVmTemplate() != null) { Line 423: mVdsGroupId = getVmTemplate().getVdsGroupId(); Line 420: mVdsGroup = getVdsGroupDAO().get(mVdsGroupId); Line 421: } Line 422: } else if (getVmTemplate() != null) { Line 423: mVdsGroupId = getVmTemplate().getVdsGroupId(); Line 424: if (mVdsGroupId != null) { > same Done Line 425: mVdsGroup = getVdsGroupDAO().get(mVdsGroupId); Line 426: } Line 427: } Line 428: } http://gerrit.ovirt.org/#/c/23828/24/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/NetworkDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/NetworkDaoDbFacadeImpl.java: Line 91: } Line 92: Line 93: @Override Line 94: public List<Network> getAllForCluster(Guid id) { Line 95: if (id == null) { > @Moti #Network - will Collection.emptyList() makes more sense to you? i.e a I have checked it - there are no mutations of this so the immutable list is indeed better Line 96: return new ArrayList<>(); Line 97: } Line 98: return getAllForCluster(id, null, false); Line 99: } http://gerrit.ovirt.org/#/c/23828/24/packaging/dbscripts/upgrade/03_05_0180_add_boot_time_to_vds_statistics.sql File packaging/dbscripts/upgrade/03_05_0180_add_boot_time_to_vds_statistics.sql: > name of the file ;) uaaaaaaaaa!!! -- To view, visit http://gerrit.ovirt.org/23828 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifaebd849061efa1637f9fa21d8584212ba0e51f2 Gerrit-PatchSet: 24 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
