On 10/21/2024 3:28 PM, Peter Xu wrote:
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.
Thanks, my mistake, I'll delete it in both places.
+
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.
Sure, for now I will simplify this and init all chardevs early, and add
the filter in the future series when CPR preserves chardevs.
+{
+ 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..
The TERM handler must be installed before the qtest monitor connects, to
catch failure during precreate. Previously it was not installed until
qemu_init_displays -> os_setup_signal_handling. The latter is still called,
because of the comment "must be after terminal init, SDL library changes
signal handlers". It is harmless to call os_setup_signal_handling twice.
- Steve
+ qemu_create_monitors();
+
/*
* QOM objects created after this point see all global and
* compat properties.
--
1.8.3.1
