Yair Zaslavsky has posted comments on this change.

Change subject: core: notify user about broken domain tasks (#753591)
......................................................................


Patch Set 2: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/SetStoragePoolStatusCommand.java
Line 39:                     DbFacade.getInstance()
Line 40:                             .getStoragePoolIsoMapDAO()
Line 41:                             .updateStatus(storageStatusInPool.getId(), 
storageStatusInPool.getstatus());
Line 42:                     //notify async tasks
Line 43:                     final List<Guid> asyncTasks = 
getDbFacade().getAsyncTaskDAO().getAsyncTaskIdsByEntity(storageStatusInPool.getstorage_id());
I agree with Moti here... you can see this pattern (one single event log with a 
list of IDs, instead of X event log objects) in other places in the code.
Line 44:                     for(Guid taskId: asyncTasks) {
Line 45:                         AuditLogableBase auditLogableBase = new 
AuditLogableBase();
Line 46:                         
auditLogableBase.setStorageDomain(getStorageDomain());
Line 47:                         auditLogableBase.setJobId(taskId);


Line 42:                     //notify async tasks
Line 43:                     final List<Guid> asyncTasks = 
getDbFacade().getAsyncTaskDAO().getAsyncTaskIdsByEntity(storageStatusInPool.getstorage_id());
Line 44:                     for(Guid taskId: asyncTasks) {
Line 45:                         AuditLogableBase auditLogableBase = new 
AuditLogableBase();
Line 46:                         
auditLogableBase.setStorageDomain(getStorageDomain());
Following Moti's other comment (in properties file)- please set the storage 
domain name.
Line 47:                         auditLogableBase.setJobId(taskId);
Line 48:                         AuditLogDirector.log(auditLogableBase, 
AuditLogType.STORAGE_DOMAIN_TASKS_ERROR);
Line 49:                     }
Line 50:                 }


Line 43:                     final List<Guid> asyncTasks = 
getDbFacade().getAsyncTaskDAO().getAsyncTaskIdsByEntity(storageStatusInPool.getstorage_id());
Line 44:                     for(Guid taskId: asyncTasks) {
Line 45:                         AuditLogableBase auditLogableBase = new 
AuditLogableBase();
Line 46:                         
auditLogableBase.setStorageDomain(getStorageDomain());
Line 47:                         auditLogableBase.setJobId(taskId);
I would like to add on Moti's comment -
AsyncTaskDao manages VDSM async tasks. the IDs you're getting of the VDSM async 
tasks (the IDs that are recognized by vdsm).
In addition, I looked at the bug, and I wonder - do we really care to set at 
audit log the ID of the VDSM async task?
Line 48:                         AuditLogDirector.log(auditLogableBase, 
AuditLogType.STORAGE_DOMAIN_TASKS_ERROR);
Line 49:                     }
Line 50:                 }
Line 51:             }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b6239668eece2be39b96c9bd360af2bd310a3b0
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to