Roy Golan has posted comments on this change. Change subject: core: adjustments needed to support instance types ......................................................................
Patch Set 24: Code-Review+1 (9 comments) relatively minor stuff. if I'll have time I'll send rebase this patch and refactor out all null check of vds_groups_id to BeanValidation 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 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 for the OS 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 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 44: . use jdk7 Objects. you some more places you used it too 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 51: EditableField on every status? 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 more checks. 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 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 are you expecting to mutate the list? 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 ;) -- 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
