Daniel Erez has posted comments on this change. Change subject: core: registerLibvirtSecrets on add and update secret ......................................................................
Patch Set 9: (4 comments) https://gerrit.ovirt.org/#/c/41561/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 54: @Override Line 55: public StorageDomain getStorageDomain() { Line 56: if (storageDomain == null) { Line 57: Guid providerId = getParameters().getLibvirtSecret().getProviderId(); Line 58: List<StorageDomain> storageDomains = getStorageDomainDAO().getAllByConnectionId(providerId); > Are you sure you will always get one Storage Domain here? or is there any r yes, since the action is performed only on a single provider (i.e. single storage domain). It's also being validated in the canDo. Line 59: storageDomain = storageDomains.get(0); Line 60: } Line 61: return storageDomain; Line 62: } https://gerrit.ovirt.org/#/c/41561/9/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CINDERStorageHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CINDERStorageHelper.java: Line 43: import org.slf4j.LoggerFactory; Line 44: Line 45: public class CINDERStorageHelper extends StorageHelperBase { Line 46: Line 47: private static Logger log = LoggerFactory.getLogger(CINDERStorageHelper.class); > Not related need since 'registerLibvirtSecrets' is static now.. Line 48: Line 49: private boolean runInNewTransaction = true; Line 50: Line 51: public boolean isRunInNewTransaction() { Line 68: public boolean disconnectStorageFromDomainByVdsId(StorageDomain storageDomain, Guid vdsId) { Line 69: return unregisterLibvirtSecrets(storageDomain, vdsId); Line 70: } Line 71: Line 72: public static Pair<Boolean, VdcFault> registerLibvirtSecrets(StorageDomain storageDomain, VDS vds, > How static is related to the patch? needed since I use it in LibvirtSecretCommandBase Line 73: List<LibvirtSecret> libvirtSecrets) { Line 74: VDSReturnValue returnValue; Line 75: if (!libvirtSecrets.isEmpty()) { Line 76: try { Line 123: } Line 124: return true; Line 125: } Line 126: Line 127: private static void addMessageToAuditLog(AuditLogType auditLogType, String storageDomainName, String vdsName){ > How this is related to the patch? since 'registerLibvirtSecrets' is static now.. Line 128: AuditLogableBase logable = new AuditLogableBase(); Line 129: logable.addCustomValue("StorageDomainName", storageDomainName); Line 130: logable.addCustomValue("VdsName", vdsName); Line 131: new AuditLogDirector().log(logable, auditLogType); -- To view, visit https://gerrit.ovirt.org/41561 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9d000e7d4cfcc946b7d055c9ed205aa54e9e2c7b Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
