GNOME Bugzilla – Bug 753471
Update the event list when new events are logged
Last modified: 2018-05-22 13:05:12 UTC
The journal provides sd_journal_get_fd() to give a file descriptor which notifies clients of updates, and this should be used to display new log messages as they are pushed into the journal.
Created attachment 317090 [details] [review] Patch
Review of attachment 317090 [details] [review]: It seems odd to move the code for monitoring the journal from out of gl-journal.c to gl-journal-model.c. The abstraction for pretty much everything journal-related should stay in gl-journal.c, and only the GListModel parts should be in gl-journal-model.c. The better way to handle this is to do the monitoring in GlJournal, and to emit a signal when the journal has changed. You could build up a queue of new log items, which GlJournalModel pushes into the model, adding new API in GlJournal to fetch the new items. That way, the journal state is also kept in GlJournal, and does not have to be driven from the outside. Alternatively, you could set the journal read pointer to the end of the journal (and keep track of the number of new entries), and run the iterations in GlJournalModel. Another possibility is to emit an "entry-added" signal for each entry that is created, which has the GlJournalEntry as part of the signal emission, and to process each one in turn in GlJournalModel. These approaches should hopefully reduce the amount of new GlJournal API that would be needed, and simplify the model code.
Created attachment 318253 [details] [review] patch updated Monitor journal changes in GlJournal, and emit signal "entries_added" to notify GlJournalModel for updating the event list.
Review of attachment 318253 [details] [review]: You shouldn't need to add the _skip() API in GlJournal for this. Simply return the new GlJournalEntry as part of the (entry-added) signal emission, and add it to GlJournalModel as part of handling the signal.
Well, I found that multiple entries could be added at the same time, but only emit the signal once. The reason why I add _skip() API to GlJournal is to separate fetching old entries in batches and fetching new entries. Both need operations on the read pointer of journal.
(In reply to Jonathan Kang from comment #5) > Well, I found that multiple entries could be added at the same time, but > only emit the signal once. Emit the signal once per entry, possibly in an idle handler. > The reason why I add _skip() API to GlJournal is > to separate fetching old entries in batches and fetching new entries. Both > need operations on the read pointer of journal. You do not need access to the read pointer if you return the entry as part of the signal.
Created attachment 318962 [details] [review] patch updated
Review of attachment 318962 [details] [review]: ::: src/gl-journal.c @@ +242,2 @@ gint ret; + GlJournalEntry *entry; You should move this into the more-specific block for the do-while loop. The gint r should probably move to a more-specific block too. @@ +326,3 @@ gobject_class->finalize = gl_journal_finalize; + + entries_signal = g_signal_new ("entries_added", G_OBJECT_CLASS_TYPE (klass), It's better to use the canonical name for the signal here, by replacing the underscore with a dash. Also, as a single GlJournalEntry is associated with the signal emission, the name should be "entry-added". @@ +357,3 @@ } + priv->shown_entries = 0; The private structure is zero-initialized, so there is no need to do so again. @@ +686,3 @@ return NULL; + priv->shown_entries++; The journal should not have any knowledge of the number of entries in the GlJournal Model, so you need to find some other way of keeping track of this.
Created attachment 342120 [details] [review] Add a refresh button Add a refresh button so that users can refresh the log list and view the latest log.
Review of attachment 342120 [details] [review]: There should not be a refresh button - this should be automatic on notification of new events from the journal.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-logs/issues/9.