Martin Sivák has posted comments on this change.

Change subject: engine: Add an AuditLog message when VM cannot be prestarted
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/32029/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolMonitor.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolMonitor.java:

Line 137:         boolean prestartingVmSucceeded = 
vdcReturnValue.getSucceeded();
Line 138: 
Line 139:         if (!prestartingVmSucceeded) {
Line 140:             AuditLogableBase log = new AuditLogableBase();
Line 141:             log.setVmId(vmToRunAsStateless.getId());
> i think this would better be a log on the vm pool on not on the specific vm
I agree, but I did not find any setVmPoolId method in the AuditLogableBase..
Line 142:             log.addCustomValue("VmName", 
vmToRunAsStateless.getName());
Line 143:             AuditLogDirector.log(log, 
AuditLogType.VM_CANNOT_BE_PRESTARTED_NOT_ENOUGH_RESOURCES);
Line 144:         }
Line 145: 


Line 139:         if (!prestartingVmSucceeded) {
Line 140:             AuditLogableBase log = new AuditLogableBase();
Line 141:             log.setVmId(vmToRunAsStateless.getId());
Line 142:             log.addCustomValue("VmName", 
vmToRunAsStateless.getName());
Line 143:             AuditLogDirector.log(log, 
AuditLogType.VM_CANNOT_BE_PRESTARTED_NOT_ENOUGH_RESOURCES);
> how do you know its because not enough resources?
I was thinking about this, but we do not return any specific reason anywhere so 
there is nothing to check. canDoAction reports the full detail for each 
considered host, but we can't put that to AuditLog.

Maybe just saying "Vm could not be started, the system will continue trying" 
would be enough to fix the bug.
Line 144:         }
Line 145: 
Line 146:         log.infoFormat("Running Vm {0} as stateless {1}",
Line 147:                 vmToRunAsStateless, prestartingVmSucceeded ? 
"succeeded" : "failed");


http://gerrit.ovirt.org/#/c/32029/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java:

Line 464:     VM_SET_TICKET(164),
Line 465:     VM_SET_TICKET_FAILED(165, AuditLogSeverity.ERROR),
Line 466:     VM_CONSOLE_CONNECTED(167),
Line 467:     VM_CONSOLE_DISCONNECTED(168),
Line 468:     VM_CANNOT_BE_PRESTARTED_NOT_ENOUGH_RESOURCES(169, 
AuditLogSeverity.WARNING),
> i'm afraid this might flood the audit log, in case few vms will fail (lets 
It will indeed, but isn't that what should happen in this case? Or how often is 
ok here?
Line 469: 
Line 470:     VM_MIGRATION_FAILED_DURING_MOVE_TO_MAINTENANCE(140, 
AuditLogSeverity.ERROR),
Line 471:     VM_SET_TO_UNKNOWN_STATUS(142, AuditLogSeverity.WARNING),
Line 472:     VM_WAS_SET_DOWN_DUE_TO_HOST_REBOOT_OR_MANUAL_FENCE(143),


-- 
To view, visit http://gerrit.ovirt.org/32029
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab04acb1746a22627b584d70c806469a818a6c8c
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[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

Reply via email to