Omer Frenkel has posted comments on this change.
Change subject: engine: watchdog - DB and logic changes
......................................................................
Patch Set 7: (5 inline comments)
....................................................
File backend/manager/dbscripts/create_views.sql
Line 567: WHERE vm_static.entity_type = 'VM';
Line 568:
Line 569:
Line 570:
Line 571: CREATE OR REPLACE VIEW vms_with_tags
shouldnt this view be updated as well? i think its used in search
Line 572: AS
Line 573: SELECT vms.vm_name, vms.vm_mem_size_mb, vms.nice_level,
vms.vmt_guid, vms.vm_os, vms.vm_description,
Line 574: vms.vds_group_id, vms.vm_domain, vms.vm_creation_date,
vms.auto_startup, vms.is_stateless, vms.is_smartcard_enabled,
vms.is_delete_protected,
Line 575: vms.dedicated_vm_for_vds, vms.fail_back,
vms.default_boot_sequence, vms.vm_type,
....................................................
File backend/manager/dbscripts/upgrade/03_03_0110_watchdog.sql
Line 1:
Line 2: select fn_db_add_column('vm_dynamic', 'last_watchdog_event', 'numeric');
please dont use numeric unless its needed
Line 3: select fn_db_add_column('vm_dynamic', 'last_watchdog_action',
'varchar(8)');
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveWatchdogAction.java
Line 5: import org.ovirt.engine.core.common.utils.VmDeviceType;
Line 6: import org.ovirt.engine.core.compat.Guid;
Line 7:
Line 8: @SuppressWarnings("serial")
Line 9: public class RemoveWatchdogAction<T extends VmOperationParameterBase>
extends VmOperationCommandBase<T> {
if this is a command, it should end with "Command"
but i see that device can be removed with update..Command, so maybe this one is
redundant?
Line 10:
Line 11: public RemoveWatchdogAction(Guid commandId) {
Line 12: super(commandId);
Line 13: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateWatchdogCommand.java
Line 51: return specParams;
Line 52: }
Line 53:
Line 54: protected boolean canDoAction() {
Line 55: return true;
return true?
no need to inform the user he sent wrong vm (vm doesnt exist), or other issues?
Line 56: }
Line 57:
Line 52: }
Line 53:
Line 54: protected boolean canDoAction() {
Line 55: return true;
Line 56: }
what about audit log for the command (succeeded? failed?)
Line 57:
--
To view, visit http://gerrit.ovirt.org/13057
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I813a6f97e23008d15446285998a4e9b50b456040
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
