Skip to content

console: io backup restore #525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
HofiOne wants to merge 9 commits into
base: main
Choose a base branch
from
Open

console: io backup restore #525

HofiOne wants to merge 9 commits into from

Conversation

@HofiOne
Copy link
Contributor

@HofiOne HofiOne commented Feb 21, 2025 •
edited
Loading

console: add support for restoring the original console io handlers after an attached control client has finished its work

Signed-off-by: Hofi hofione@gmail.com

@HofiOne HofiOne changed the title Console io backup restore console: io backup restore Feb 21, 2025
@HofiOne HofiOne marked this pull request as draft February 22, 2025 09:40
@HofiOne HofiOne marked this pull request as ready for review February 22, 2025 11:22
@HofiOne HofiOne force-pushed the console-io-backup-restore branch from f096a01 to f9d4706 Compare February 22, 2025 11:41
bazsi
bazsi reviewed Feb 24, 2025
View reviewed changes
bazsi
bazsi reviewed Feb 24, 2025
View reviewed changes
bazsi
bazsi reviewed Feb 24, 2025
View reviewed changes
@HofiOne HofiOne force-pushed the console-io-backup-restore branch from f9d4706 to ece79b0 Compare February 24, 2025 09:58
@HofiOne HofiOne marked this pull request as draft February 24, 2025 10:49
@HofiOne

This comment was marked as outdated.

Sign in to view
@bazsi
Copy link
Member

bazsi commented Feb 24, 2025

@bazsi the _parse_attach_command_args shows that the intended behavior is to handle only one of the attach target options in a given call, either STDIO, LOGS, or the DEBUGGER. But the logic in the control_connection_attach will always steel multiple variations, e.g. if only LOGS is the target it will steel the STDIO as well automatically currently.

I think we only need to handle one case at a time. And stdio I think we can easily remove. That was the first thing I implemented, when I realized I needed to set the log levels as well. So LOGS and DEBUGGER and both should run only as long as the ctl program is running.

We should restore to the original console (as you implemented) when we finish with any of these commands, so the next one can properly run.

What are the intended, valid combinations that we would like to support here?

My bets are

STDIO option -> only STDIO LOGS option -> only LOGS DEBUGGER option -> only DEBUGGER (or STDIO too to get everything in the caller's console?!?)

Could you please confirm?

I hope I addressed these with the response above. Let me know if it didn't or ping me on discord/reddit to chat.

@HofiOne HofiOne force-pushed the console-io-backup-restore branch 2 times, most recently from 2dc8887 to 2ecac27 Compare February 26, 2025 11:28
@HofiOne HofiOne marked this pull request as ready for review February 26, 2025 11:40
@HofiOne HofiOne requested a review from bazsi February 26, 2025 12:09
@HofiOne HofiOne force-pushed the console-io-backup-restore branch 2 times, most recently from 98f51b9 to 7dba99e Compare February 27, 2025 12:31
@HofiOne HofiOne marked this pull request as draft February 27, 2025 12:52
@HofiOne HofiOne marked this pull request as ready for review February 27, 2025 13:23
@bazsi
Copy link
Member

bazsi commented Mar 18, 2025

just came back from a longish travel, I'll look at this again.

@HofiOne HofiOne force-pushed the console-io-backup-restore branch 2 times, most recently from f5a7b40 to 4e532c3 Compare March 25, 2025 13:23
HofiOne added 8 commits March 28, 2025 12:38
@HofiOne
syslog-ng-ctl: do not override the default help detection of glib hel…
182e5b4 
…p context

This way syslog-ng-ctl -h, syslog-ng-ctl--help and syslog-ng-ctl command -h, syslog-ng-ctl command --help will behave the same way (not like till now)

Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne
syslog-ng-ctl: consolidated and unified the help printing
3271b4b 
Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne
console: add support for restoring the original console io handlers
21b23c0 
Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne
console: remove the now unused console_present, use only using_initia…
a004067 
…l_console instead

Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne
console: added --fds-to-steel option to select the stdio file handl…
8780277 
…ers to redirect on attach

Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne
syslog-ng-ctl-attach: use proper command+sub_command helps to get acc…
bb7da31 
…urate help contexts and help info

Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne
style: removed usage of FALSE == as requested
dba7e9c 
Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne
console: spelling fix
70fc851 
Signed-off-by: Hofi <hofione@gmail.com>

# Conflicts:
#	lib/mainloop-control.c
#	syslog-ng-ctl/commands/attach.c

Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne
syslog-ng-ctl: attach uses the automatic GLib Commandline Option Pars…
6ff5cc5 
…er validation, sub-commands and options do not need to be double validated

Signed-off-by: Hofi <hofione@gmail.com>
@HofiOne HofiOne force-pushed the console-io-backup-restore branch from 4e532c3 to 6ff5cc5 Compare March 28, 2025 11:39
@HofiOne
Copy link
Contributor Author

HofiOne commented Oct 13, 2025 •
edited
Loading

FYI. This has a bug, unfortunately. This will hopefully fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MrAnno MrAnno MrAnno left review comments

@bazsi bazsi Awaiting requested review from bazsi

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@HofiOne @bazsi @MrAnno