Martin Peřina has posted comments on this change. Change subject: restapi: Host Power-Management Refactor (#977674) - WIP ......................................................................
Patch Set 12: (5 comments) I just went over just a few files, because it's very hard to review patch that big. Please split this patch into at least 4 subpatches: 1. DB and business entities changes 2. Command changes 3. Frontend changes 4. RESTAPI changes Thanks http://gerrit.ovirt.org/#/c/27578/12/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java: Line 290: getCompensationContext().snapshotNewEntity(vdsStatistics); Line 291: } Line 292: Line 293: private void addFencingAgentsToDb() { Line 294: if (getParameters().getFencingAgents() != null && !getParameters().getFencingAgents().isEmpty()) { There's no need to test isEmpty(), emptiness is handled by for loop Line 295: for (FencingAgent agent : getParameters().getvds().getFencingAgents()) { Line 296: DbFacade.getInstance().getFencingAgentDao().save(agent); Line 297: } Line 298: } http://gerrit.ovirt.org/#/c/27578/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDS.java: Line 169: vds.setHighlyAvailableIsConfigured(getHighlyAvailableIsConfigured()); Line 170: vds.setHighlyAvailableIsActive(getHighlyAvailableIsActive()); Line 171: vds.setHighlyAvailableGlobalMaintenance(getHighlyAvailableGlobalMaintenance()); Line 172: vds.setHighlyAvailableLocalMaintenance(getHighlyAvailableLocalMaintenance()); Line 173: vds.setFencingAgents(getFencingAgents()); Please copy list content into new list instance instead of passing a reference Line 174: return vds; Line 175: } Line 176: Line 177: private Version vdsGroupCompatibilityVersion; http://gerrit.ovirt.org/#/c/27578/12/packaging/dbscripts/create_tables.sql File packaging/dbscripts/create_tables.sql: Not related to this patch Line 1: Line 2: -- Line 3: -- Name: action_version_map; Type: TABLE; Schema: public; Owner: engine; Tablespace: Line 4: -- http://gerrit.ovirt.org/#/c/27578/12/packaging/dbscripts/create_views.sql File packaging/dbscripts/create_views.sql: Line 1768: LEFT Not related to this patch http://gerrit.ovirt.org/#/c/27578/12/packaging/dbscripts/upgrade/post_upgrade/0010_add_object_column_white_list_table.sql File packaging/dbscripts/upgrade/post_upgrade/0010_add_object_column_white_list_table.sql: Line 38: from information_schema.columns Line 39: where table_name = 'vds' and Line 40: column_name in ( Line 41: 'vds_group_id', 'vds_group_name', 'vds_group_description', Line 42: 'vds_id', 'vds_name', 'ip', 'vds_unique_id', 'host_name', 'port', 'vds_strength', Shouldn't column 'ip' be removed also? Line 43: 'server_ssl_enabled', 'vds_type', 'pm_enabled', 'pm_proxy_preferences', 'vds_spm_priority', 'hooks', 'status', 'cpu_cores', Line 44: 'cpu_model', 'cpu_speed_mh', 'if_total_speed', 'kvm_enabled', 'physical_mem_mb', Line 45: 'pending_vcpus_count', 'pending_vmem_size', 'mem_commited', 'vm_active', 'vm_count', Line 46: 'vm_migrating', 'vms_cores_count', 'cpu_over_commit_time_stamp', 'hypervisor_type', -- To view, visit http://gerrit.ovirt.org/27578 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id0b384347a52db8b0aa6ae684fad40a5491dd2f7 Gerrit-PatchSet: 12 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
