Alon Bar-Lev has posted comments on this change. Change subject: fklsnr: Introduce standalone fence_kdump listener ......................................................................
Patch Set 5: (7 comments) http://gerrit.ovirt.org/#/c/27201/5/ovirt-engine.spec.in File ovirt-engine.spec.in: Line 295: Line 296: %description backend Line 297: The backend engine of %{ovirt_product_name_short} Line 298: Line 299: %package fence-kdump-listener > In 3.5 fence_kdump listener will be executed on the same host as engine. Bu yes, I understand. the question is if there is actual need. the websocket-proxy is exposed from users (public network), it also creates a lot of load (graphic session per user), so in its case it makes sense to split. I do not see any reason to split this service, as it does not create a load and it is being accessed from internal network. Line 300: Summary: %{ovirt_product_name_short} fence_kdump listener Line 301: Group: %{ovirt_product_group} Line 302: Requires: %{name}-lib >= %{version}-%{release} Line 303: Requires: python-dateutil Line 936: %ghost %config(noreplace) %{engine_pki}/serial.txt Line 937: Line 938: %files fence-kdump-listener Line 939: Line 940: %{engine_data}/services/ovirt-fence-kdump-listener > OK. yes, I will fix it. thanks! Line 941: %{engine_etc}/ovirt-fence-kdump-listener.conf.d/ Line 942: Line 943: %if %{ovirt_install_systemd} Line 944: %{_unitdir}/ovirt-fence-kdump-listener.service http://gerrit.ovirt.org/#/c/27201/5/packaging/services/ovirt-fence-kdump-listener/db.py File packaging/services/ovirt-fence-kdump-listener/db.py: Line 149: config_values, Line 150: ) Line 151: return config_values Line 152: Line 153: def create_fence_kdump_message(self, connection, host, received, status): > OK, with above, just one question: oh... you want to survive db restarts... so what I suggest is to always execute: select 1 if success execute real statement. if fails, then reopen connection. all within the execute. Line 154: self.execute( Line 155: connection=connection, Line 156: statement=""" Line 157: INSERT INTO fence_kdump_messages (host, received, status) http://gerrit.ovirt.org/#/c/27201/5/packaging/services/ovirt-fence-kdump-listener/listener.py File packaging/services/ovirt-fence-kdump-listener/listener.py: Line 174: ) Line 175: ) Line 176: ) Line 177: entry['status'] = self.SESSION_STATE_DUMPING Line 178: self._sessions[entry['host']] = entry > If record is not successfully store to db, I don't want to update in memory so remove it if you detect failure. the session tracking mechanism should be abstracted (reusable). Line 179: Line 180: elif entry['status'] == self.SESSION_STATE_DUMPING: Line 181: self.logger.debug( Line 182: 'dumping %s', Line 271: Line 272: for record in self._db.get_unfinished_flows(): Line 273: # if record is saved to db, in memory cache status Line 274: # is always 'dumping' Line 275: record['status'] = self.SESSION_STATE_DUMPING > 1) I identify incoming messages only by IP address, I don't care about sour if someone adds nat we may get same ip but different port per server. but then you won't be able to detect what vdsm it is... hmmm... bad protocol. can you store the host:port within the unfinished flows in database? Line 276: sessions[record['host']] = record Line 277: Line 278: return sessions Line 279: http://gerrit.ovirt.org/#/c/27201/5/packaging/services/ovirt-fence-kdump-listener/ovirt-fence-kdump-listener.conf.in File packaging/services/ovirt-fence-kdump-listener/ovirt-fence-kdump-listener.conf.in: Line 2: # This is a default configuration file for oVirt/RHEV-M fence_kdump listener Line 3: # Line 4: TRACE_ENABLE=False Line 5: TRACE_FILE= Line 6: ENGINE_USR="@ENGINE_USR@" > 1) Listen address and port can be changed only in engine-setup 1) why? 2) oh! that's an answer. 3) you mean engine-config, right? ok. /me/ as a sysadmin... would have expected it here... so can you please add a comment that the at least the listen address and listen port are available at engine-config? http://gerrit.ovirt.org/#/c/27201/5/packaging/services/ovirt-fence-kdump-listener/ovirt-fence-kdump-listener.py File packaging/services/ovirt-fence-kdump-listener/ovirt-fence-kdump-listener.py: Line 93: port=self._config.get("ENGINE_DB_PORT"), Line 94: database=self._config.get("ENGINE_DB_DATABASE"), Line 95: username=self._config.get("ENGINE_DB_USER"), Line 96: password=self._config.get("ENGINE_DB_PASSWORD"), Line 97: secured=self._config.get("ENGINE_DB_SECURED") == 'True', > I don't understand your comment. From engine service configuration I just n right! so you need two config objects. one is your own and one is of engine. no? Line 98: secure_validation=( Line 99: self._config.get("ENGINE_DB_SECURED_VALIDATION") == 'True' Line 100: ), Line 101: ) -- To view, visit http://gerrit.ovirt.org/27201 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieec3bad47bbba860a52a9ff4e2eb7f61277f4e36 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
