Skip to content
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.

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

grub2-bhyve: Add -c, --cons-dev option to choose terminal #2

Merged
merged 1 commit into from
Oct 28, 2014

Conversation

cemeyer
Copy link
Contributor

@cemeyer cemeyer commented Oct 26, 2014

This enables interactive GRUB menus, etc from users such as libvirt.

This enables interactive GRUB menus, etc from users such as libvirt.
@@ -272,6 +326,13 @@ grub_ncurses_init (struct grub_term_output *term __attribute__ ((unused)))
static grub_err_t
grub_ncurses_fini (struct grub_term_output *term __attribute__ ((unused)))
{

#ifdef BHYVE
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this block go after the endwin() call ? The curs_initscr(3X) man page states that delscreen should be called after endwin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The man page says this, but it also says delscreen releases memory while endwin resets the terminal. I have no idea if the order is important. I didn't see crashes or anything with the code as-is. I can dive into the curses implementation further if you want.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order may not matter, but having endwin() first matches the text of the man page's suggested tear-down sequence, and won't require any comments to explain to folk such as myself who look at the man page and then wonder why it's not in the same order :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, feel free to reorder them :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just briefly looking at the ncurses code, it appears endwin() does do something with CURRENT_SCREEN; however, the delscreen() code does something that looks like resetting the current screen if the deleted screen is the current screen.

grehan-freebsd added a commit that referenced this pull request Oct 28, 2014
grub2-bhyve: Add -c, --cons-dev option to choose terminal
@grehan-freebsd grehan-freebsd merged commit 808fa4f into grehan-freebsd:master Oct 28, 2014
@grehan-freebsd
Copy link
Owner

Thanks for the patch: this is extremely useful.

I'll change the curses ordering in a follow-up commit.

@cemeyer
Copy link
Contributor Author

cemeyer commented Oct 28, 2014

Thanks! libvirt support for this is in the works :-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants