Add granular console APIs

[jakecorrenti]

Currently, libkrun forces the user to manually change the source code if they are not building with EFI=1 and want to use the serial console instead of the virtio-console.

Provide an API, krun_set_serial_console, allowing the user to specify whether the serial console should be enabled. The serial console remains the default on EFI=1 use cases.

[tylerfanelli]

LGTM, though I wonder if it makes sense for this to be krun_set_console and the input to be an enum with two supported options:

KRUN_CONSOLE_SERIAL
KRUN_CONSOLE_VIRTIO

Leaves open the possibility of adding other options. Just something to consider.

[slp]

LGTM, though I wonder if it makes sense for this to be krun_set_console and the input to be an enum with two supported options:

KRUN_CONSOLE_SERIAL
KRUN_CONSOLE_VIRTIO

Leaves open the possibility of adding other options. Just something to consider.

It's even more complex. You might want to enable one of them, or both. But, in addition of that, for the non-EFI case and when the kernel command line is not specified with krun_set_kernel, we need to specify which one is going to be put along with console= in the kernel command line.

Perhaps something like...

KRUN_CONSOLE_SERIAL
KRUN_CONSOLE_VIRTIO
KRUN_CONSOLE_BOTH_DEFAULT_SERIAL
KRUN_CONSOLE_BOTH_DEFAULT_VIRTIO

...?

[slp]

Let's rethink about the configuration options.

[mtjhrc]

LGTM, though I wonder if it makes sense for this to be krun_set_console and the input to be an enum with two supported options:

KRUN_CONSOLE_SERIAL
KRUN_CONSOLE_VIRTIO

Leaves open the possibility of adding other options. Just something to consider.

It's even more complex. You might want to enable one of them, or both. But, in addition of that, for the non-EFI case and when the kernel command line is not specified with krun_set_kernel, we need to specify which one is going to be put along with console= in the kernel command line.

Perhaps something like...

KRUN_CONSOLE_SERIAL
KRUN_CONSOLE_VIRTIO
KRUN_CONSOLE_BOTH_DEFAULT_SERIAL
KRUN_CONSOLE_BOTH_DEFAULT_VIRTIO

Yeah I agree we should be more flexible whether you want both, but I'm not really a fan of having an enum like this.

I feel like ideally we should not have a console at all and let the user attach SERIAL/VIRTIO console(s) explicitly.

Perhaps we could retrofit it to the current API... Can we maybe add an krun_no_implicit_consoles(ctx)? And then add functions for explicitly attaching the consoles? In the v2 API the krun_no_implicit_consoles(ctx) could then be the default.
Perhaps something like this:

int32_t ctx = krun_create_ctx();
krun_no_implicit_consoles(ctx);
krun_add_serial_console(/*console id*/ 1, ...);
krun_add_virtio_console(/*console id*/ 2, ...);
krun_set_kernel_console(/*console id*/ 1); // console= in the kernel command line.

This would also make future API for attaching multiple virtio-console(s) nicer.

Though this requires bigger changes in libkrun, I feel like it makes the API much more sane for the users (and extensible for us!).

[slp]

Perhaps we could retrofit it to the current API... Can we maybe add an krun_no_implicit_consoles(ctx)? And then add functions for explicitly attaching the consoles? In the v2 API the krun_no_implicit_consoles(ctx) could then be the default. Perhaps something like this:

int32_t ctx = krun_create_ctx();
krun_no_implicit_consoles(ctx);
krun_add_serial_console(/*console id*/ 1, ...);
krun_add_virtio_console(/*console id*/ 2, ...);
krun_set_kernel_console(/*console id*/ 1); // console= in the kernel command line.

This would also make future API for attaching multiple virtio-console(s) nicer.

Though this requires bigger changes in libkrun, I feel like it makes the API much more sane for the users (and extensible for us!).

Yes, this is probably the way. We know we're going to need a much more flexible console configuration for Android support (Android is a heavy user of virtio-console to communicate the guests with various processes in the host), and this would be a step in the right direction.

That said, perhaps it'd be safer to wait a bit to see exactly what the requirements are on the Android side and then propose an API?

[mtjhrc]

That said, perhaps it'd be safer to wait a bit to see exactly what the requirements are on the Android side and then propose an API?

Yeah maybe, but I don't think it will really be a problem. For now we could just have the same functionality as we already do just make it more explicit. We can also have multiple krun_add_virtio_console_* functions with different arguments. The current console auto-configuration (which automatically configures the stdout/stdin/stderr as separate ports) could still exist but just be e.g. krun_add_virtio_console_default(int console_id).

Details of how to configure the console(s) for Android can still be figured out later. We could add a function like krun_add_virtio_serial(..) (I don't like the misleading QEMU name, but not sure how to call it). If we need to configure a multiport console with specific ports we can just have e.g. krun_add_virtio_console_multiport(int port_id) and configure the ports using multiple calls to krun_add_virtio_console_port(int console_id, const char* name, int port_fd)...

[jakecorrenti]

This should work. I did my best to test it on macOS and x86. I spent more time than I'd like to admit going back and forth on how this should all look, but I tried making a sane starting point that allows for future expansion (for Android, for example). If it doesn't work for someone, or I completely over looked something, let me know :)

[mtjhrc]

LGTM, I added a suggestion to clarify the doc comment a bit more though