Martin Peřina has posted comments on this change. Change subject: fklsnr: Introduce standalone fence_kdump listener ......................................................................
Patch Set 5: (6 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 > any reason we want this to run on different host? if not, maybe better to m In 3.5 fence_kdump listener will be executed on the same host as engine. But some customers may want to execute it on different host, so using separate package, we will be partially prepared to achieve this in future version (similarly to websocket-proxy). 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 > please add / at suffix OK. I copied this from websocket-proxy part, where trailing / is missing also. 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/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 > why do you need to modify _sessions? it is already set for you. If record is not successfully store to db, I don't want to update in memory cache. That's the reason I add new session to _sessions here and not in run() Line 179: Line 180: elif entry['status'] == self.SESSION_STATE_DUMPING: Line 181: self.logger.debug( Line 182: 'dumping %s', Line 180: elif entry['status'] == self.SESSION_STATE_DUMPING: Line 181: self.logger.debug( Line 182: 'dumping %s', Line 183: entry, Line 184: ) > update database? If db is updated successfully (see above), then I log debug message. Line 185: except StandardError as e: Line 186: self.logger.error( Line 187: _( Line 188: "Error updating host {host} status to {status}:" 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 > please use the term address as in my example, per true (ip, port), unless t 1) I identify incoming messages only by IP address, I don't care about source (sender) port. 2) Well, I already use host in db table and engine part, so I wanted to be consistent. But if it's really needed, I can rename it to address ... 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@" > at least two are missing: 1) Listen address and port can be changed only in engine-setup 2) It should be stored in vdc_options, because they will be sent to hosts during hostdeploy (address and port are part of kdump configuration, so kdump initramfs has to be recreated after the change) 3) If user wants to change listener address and port, he can do this using engine-setup and after that all hosts need to be redeployed -- 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
