On Thu, Oct 17, 2024 at 08:14:12AM -0700, Steve Sistare wrote:
> Complete monitor connections as early as possible, prior to
> qemu_create_early_backends, so the user can issue commands during the
> precreate phase.
>
> Make a list of the chardev's referenced by all monitors. Create the
> chardevs, then create the monitors. Exclude monitor chardevs and
> monitors from the later creation phases.
>
> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
> ---
> system/vl.c | 81
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 77 insertions(+), 4 deletions(-)
>
> diff --git a/system/vl.c b/system/vl.c
> index 3c592b9..a985ab8 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1939,6 +1939,11 @@ static bool object_create_early(const ObjectOption
> *opt)
> return false;
> }
>
> + /* Reason: already created. */
> + if (g_str_equal(type, "mon")) {
> + return false;
> + }
Why monitor are part of "object"s? I thought it was only registered on
qemu_find_opts("mon").
Same question to object_create_late() below.
> +
> return true;
> }
>
> @@ -1956,6 +1961,68 @@ static void qemu_apply_machine_options(QDict *qdict)
> }
> }
>
> +typedef struct NamedElement {
> + char *name;
> + QTAILQ_ENTRY(NamedElement) next;
> +} NamedElement;
> +
> +static QTAILQ_HEAD(, NamedElement) monitor_chardevs =
> + QTAILQ_HEAD_INITIALIZER(monitor_chardevs);
> +
> +static void chardev_add(const char *name)
> +{
> + NamedElement *elem = g_new0(NamedElement, 1);
> +
> + elem->name = g_strdup(name);
> + QTAILQ_INSERT_TAIL(&monitor_chardevs, elem, next);
> +}
> +
> +static bool chardev_find(const char *name)
> +{
> + NamedElement *elem;
> +
> + QTAILQ_FOREACH(elem, &monitor_chardevs, next) {
> + if (g_str_equal(elem->name, name)) {
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static int monitor_add_chardev(void *opaque, QemuOpts *opts, Error **errp)
> +{
> + g_autofree char *chardev = NULL;
> + int ret = monitor_chardev_name(opts, &chardev, errp);
> +
> + if (!ret && chardev) {
> + chardev_add(chardev);
> + }
> + return ret;
> +}
> +
> +static bool option_is_monitor_chardev(void *opaque, QemuOpts *opts)
> +{
> + return chardev_find(qemu_opts_id(opts));
> +}
> +
> +static bool option_is_not_monitor_chardev(void *opaque, QemuOpts *opts)
> +{
> + return !chardev_find(qemu_opts_id(opts));
> +}
> +
> +static void qemu_create_monitors(void)
Would be good to add some generic comment on why monitors' chardev can be
created earlier before the rest.
PS: I'm not yet sure this is required for the initial support for
cpr-transfer, as there's no chardev fds involved yet? IOW, I am curious
what happens if this code init all chardevs instead of monitor-only.
> +{
> + qemu_opts_foreach(qemu_find_opts("mon"),
> + monitor_add_chardev, NULL, &error_fatal);
> +
> + qemu_opts_filter_foreach(qemu_find_opts("chardev"),
> + option_is_monitor_chardev,
> + chardev_init_func, NULL, &error_fatal);
> +
> + qemu_opts_foreach(qemu_find_opts("mon"),
> + mon_init_func, NULL, &error_fatal);
> +}
> +
> static void qemu_create_early_backends(void)
> {
> MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);
> @@ -1994,7 +2061,8 @@ static void qemu_create_early_backends(void)
> /* spice must initialize before chardevs (for spicevmc and spiceport) */
> qemu_spice.init();
>
> - qemu_opts_foreach(qemu_find_opts("chardev"),
> + qemu_opts_filter_foreach(qemu_find_opts("chardev"),
> + option_is_not_monitor_chardev,
> chardev_init_func, NULL, &error_fatal);
>
> #ifdef CONFIG_VIRTFS
> @@ -2020,6 +2088,11 @@ static void qemu_create_early_backends(void)
> */
> static bool object_create_late(const ObjectOption *opt)
> {
> + /* Reason: already created. */
> + if (g_str_equal(ObjectType_str(opt->opts->qom_type), "mon")) {
> + return false;
> + }
> +
> return !object_create_early(opt) && !object_create_pre_sandbox(opt);
> }
>
> @@ -2045,9 +2118,6 @@ static void qemu_create_late_backends(void)
> exit(1);
> }
>
> - qemu_opts_foreach(qemu_find_opts("mon"),
> - mon_init_func, NULL, &error_fatal);
> -
> if (foreach_device_config(DEV_SERIAL, serial_parse) < 0)
> exit(1);
> if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
> @@ -3730,6 +3800,9 @@ void qemu_init(int argc, char **argv)
>
> accel = configure_accelerators(argv[0]);
>
> + os_setup_signal_handling();
Didn't immediately see why this line. Some explanations / comments could
be helpful..
> + qemu_create_monitors();
> +
> /*
> * QOM objects created after this point see all global and
> * compat properties.
> --
> 1.8.3.1
>
--
Peter Xu
