Maor Lipchuk has posted comments on this change.

Change subject: core: unregisterLibvirtSecrets on remove
......................................................................


Patch Set 9: Code-Review+2

(1 comment)

https://gerrit.ovirt.org/#/c/41562/9/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/LibvirtSecretCommandBase.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/LibvirtSecretCommandBase.java:

Line 77:                     getParameters().getLibvirtSecret().getId(), 
getStorageDomain().getName());
Line 78:         }
Line 79:     }
Line 80: 
Line 81:     protected void unregisterLibvirtSecret() {
> You already implemented the same method at LibvirtSecretCommandBase, no?
rephrasing my comment, this is very similar to registerLibvirtSecret...
Suggestion: You can create one method that does all this logic and gets also an 
indication if you want to regsiter/unregister the storage. Not sure if that 
will be more better then this but I guess this is an implementation issue
Line 82:         if (getStorageDomain().getStatus() == 
StorageDomainStatus.Active) {
Line 83:             List<VDS> hostsInStatusUp = getAllRunningVdssInPool();
Line 84:             for (VDS vds : hostsInStatusUp) {
Line 85:                 CINDERStorageHelper.unregisterLibvirtSecrets(


-- 
To view, visit https://gerrit.ovirt.org/41562
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I53b2982a1b40f9305028fa38eaba08e718fa6e79
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to