Shireesh Anjal has posted comments on this change. Change subject: engine: Refresh gluster data periodically ......................................................................
Patch Set 8: (7 inline comments) Responses to most comments in-line. New patch-set to follow with one comment incorporated (removing log from InitBackendServicesOnStartupBean as it is already happening from the GlusterManager#init()) .................................................... File backend/manager/modules/bll/pom.xml Line 53: </dependency> Line 54: Line 55: <dependency> Line 56: <groupId>${engine.groupId}</groupId> Line 57: <artifactId>dal</artifactId> It's a 'test' dependency, as I use data from fixtures.xml for testing the functionality. Let me know if there is a better way. Line 58: <version>${engine.version}</version> Line 59: <type>test-jar</type> Line 60: <scope>test</scope> Line 61: </dependency> .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java Line 199: * @return Line 200: */ Line 201: private boolean isRemovableStatus(VDSStatus status) { Line 202: switch (status) { Line 203: case Up: As mentioned before, this is how engine-code-formatter formats the switch-cases. Line 204: case Down: Line 205: case Maintenance: Line 206: return true; Line 207: default: Line 228: } Line 229: Line 230: private List<String> getVdsIps(VDS vds) { Line 231: List<String> vdsIps = new ArrayList<String>(); Line 232: for (VdsNetworkInterface iface : getInterfaceDao().getAllInterfacesForVds(vds.getId())) { addAll() can be used if vdsIps was List<VdsNetworkInterface>. However it is List<String> and hence I can't use addAll(). Line 233: vdsIps.add(iface.getAddress()); Line 234: } Line 235: return vdsIps; Line 236: } Line 386: for (GlusterVolumeEntity volume : getVolumeDao().getByClusterId(clusterId)) { Line 387: if (!volumesMap.containsKey(volume.getName())) { Line 388: log.debugFormat("Volume {0} has been removed directly using the gluster CLI. Removing it from engine as well.", Line 389: volume.getName()); Line 390: getVolumeDao().remove(volume.getId()); MassOperationsDAO supports only updateAll() operations currently. I could add a removeAll() method, provided you recommend it. Note that if we do this, we will first have to prepare a list of items to be removed using the same for loop, and then invoke the removeAll() operation on the Dao. Please confirm. Line 391: logVolumeMessage(volume, AuditLogType.GLUSTER_VOLUME_DELETED_FROM_CLI); Line 392: } Line 393: } Line 394: } .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java Line 54: }); Line 55: StoragePoolStatusHandler.Init(); Line 56: Line 57: GlusterManager.getInstance().init(); Line 58: log.infoFormat("GlusterManager: {0}", new Date()); Done Line 59: } Line 60: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetNonOperationalVdsCommand.java Line 75: for (Entry<String, String> e : getParameters().getCustomLogValues().entrySet()) { Line 76: AddCustomValue(e.getKey(), e.getValue()); Line 77: } Line 78: switch (getParameters().getNonOperationalReason()) { Line 79: case NETWORK_UNREACHABLE: This is how the engine-code-formatter formats switch-cases (case statements at the same level as switch statement). This is because of following line in the engine-code-format.xml: <setting id="org.eclipse.jdt.core.formatter.indent_switchstatements_compare_to_switch" value="false"/> Changing the value to "true" will indent the case statements inside the switch block. So it looks like it was intentionally kept like this. If you think it was a mistake, I could send a patch to correct this in the formatter. Line 80: return (getSucceeded()) ? AuditLogType.VDS_SET_NONOPERATIONAL_NETWORK Line 81: : AuditLogType.VDS_SET_NONOPERATIONAL_FAILED; Line 82: case STORAGE_DOMAIN_UNREACHABLE: Line 83: return (getSucceeded()) ? AuditLogType.VDS_SET_NONOPERATIONAL_DOMAIN .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java Line 199: GLUSTER_VOLUME_REPLACE_BRICK_START_FAILED(4018), Line 200: GLUSTER_VOLUME_ADD_BRICK(4019), Line 201: GLUSTER_VOLUME_ADD_BRICK_FAILED(4020), Line 202: GLUSTER_HOST_REMOVE_FAILED(4021), Line 203: GLUSTER_VOLUME_CREATED_FROM_CLI(4022), This is the gluster CLI. The Gluster CLI can be used from any of the nodes of the cluster to perform gluster cluster operations. In fact all our gluster related features from the webadmin ultimately end up invoking the same gluster CLI to manage the gluster cluster. However if user performs any operations directly from the CLI (without using webadmin), such changes will be detected by the GlusterManager, synced with the engine DB and audited using these audit log types. Line 204: GLUSTER_VOLUME_DELETED_FROM_CLI(4023), Line 205: GLUSTER_VOLUME_OPTION_SET_FROM_CLI(4024), Line 206: GLUSTER_VOLUME_OPTION_RESET_FROM_CLI(4025), Line 207: GLUSTER_VOLUME_PROPERTIES_CHANGED_FROM_CLI(4026), -- To view, visit http://gerrit.ovirt.org/7288 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b61eb6e93105e46e2706eac1d94bc10717224c2 Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches