[PATCH 2/3] util/main-loop: Introduce the main loop into QOM

Nicolas Saenz Julienne posted 3 patches 3 years, 11 months ago
Maintainers: Hanna Reitz <hreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Michael Roth <michael.roth@amd.com>, Eric Blake <eblake@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Markus Armbruster <armbru@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Fam Zheng <fam@euphon.net>
There is a newer version of this series
[PATCH 2/3] util/main-loop: Introduce the main loop into QOM
Posted by Nicolas Saenz Julienne 3 years, 11 months ago
'event-loop-backend' provides basic property handling for all
'AioContext' based event loops. So let's define a new 'MainLoopClass'
that inherits from it. This will permit tweaking the main loop's
properties through qapi as well as through the command line using the
'-object' keyword[1]. Only one instance of 'MainLoopClass' might be
created at any time.

'EventLoopBackendClass' learns a new callback, 'can_be_deleted()' so as
to mark 'MainLoop' as non-deletable.

Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>

[1] For example:
      -object main-loop,id=main-loop,poll-max-ns=<value>
---
 include/qemu/main-loop.h | 11 ++++++++++
 qapi/qom.json            | 10 ++++++----
 qga/meson.build          |  2 +-
 tests/unit/meson.build   | 10 +++++-----
 util/event-loop.c        | 13 ++++++++++++
 util/event-loop.h        |  1 +
 util/main-loop.c         | 43 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 80 insertions(+), 10 deletions(-)

diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 8dbc6fcb89..fea5a3e9d4 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -26,9 +26,20 @@
 #define QEMU_MAIN_LOOP_H
 
 #include "block/aio.h"
+#include "qom/object.h"
+#include "util/event-loop.h"
 
 #define SIG_IPI SIGUSR1
 
+#define TYPE_MAIN_LOOP "main-loop"
+
+struct MainLoop {
+    EventLoopBackend parent_obj;
+};
+typedef struct MainLoop MainLoop;
+
+DECLARE_INSTANCE_CHECKER(MainLoop, MAIN_LOOP, TYPE_MAIN_LOOP)
+
 /**
  * qemu_init_main_loop: Set up the process so that it can run the main loop.
  *
diff --git a/qapi/qom.json b/qapi/qom.json
index eeb5395ff3..e7730ef62f 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -500,9 +500,9 @@
             '*grab-toggle': 'GrabToggleKeys' } }
 
 ##
-# @IothreadProperties:
+# @EventLoopBackendProperties:
 #
-# Properties for iothread objects.
+# Properties for iothread and main-loop objects.
 #
 # @poll-max-ns: the maximum number of nanoseconds to busy wait for events.
 #               0 means polling is disabled (default: 32768 on POSIX hosts,
@@ -522,7 +522,7 @@
 #
 # Since: 2.0
 ##
-{ 'struct': 'IothreadProperties',
+{ 'struct': 'EventLoopBackendProperties',
   'data': { '*poll-max-ns': 'int',
             '*poll-grow': 'int',
             '*poll-shrink': 'int',
@@ -818,6 +818,7 @@
     { 'name': 'input-linux',
       'if': 'CONFIG_LINUX' },
     'iothread',
+    'main-loop',
     { 'name': 'memory-backend-epc',
       'if': 'CONFIG_LINUX' },
     'memory-backend-file',
@@ -882,7 +883,8 @@
       'input-barrier':              'InputBarrierProperties',
       'input-linux':                { 'type': 'InputLinuxProperties',
                                       'if': 'CONFIG_LINUX' },
-      'iothread':                   'IothreadProperties',
+      'iothread':                   'EventLoopBackendProperties',
+      'main-loop':                  'EventLoopBackendProperties',
       'memory-backend-epc':         { 'type': 'MemoryBackendEpcProperties',
                                       'if': 'CONFIG_LINUX' },
       'memory-backend-file':        'MemoryBackendFileProperties',
diff --git a/qga/meson.build b/qga/meson.build
index 1ee9dca60b..3051473e04 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -52,7 +52,7 @@ qga_ss = qga_ss.apply(config_host, strict: false)
 
 qga = executable('qemu-ga', qga_ss.sources(),
                  link_args: config_host['LIBS_QGA'].split(),
-                 dependencies: [qemuutil, libudev],
+                 dependencies: [qemuutil, libudev, qom],
                  install: true)
 all_qga = [qga]
 
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 64a5e7bfde..7a1af584dd 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -51,7 +51,7 @@ tests = {
 
 if have_system or have_tools
   tests += {
-    'test-qmp-event': [testqapi],
+    'test-qmp-event': [testqapi, qom],
   }
 endif
 
@@ -120,17 +120,17 @@ endif
 if have_system
   tests += {
     'test-iov': [],
-    'test-qmp-cmds': [testqapi],
+    'test-qmp-cmds': [testqapi, qom],
     'test-xbzrle': [migration],
-    'test-timed-average': [],
-    'test-util-sockets': ['socket-helpers.c'],
+    'test-timed-average': [qom],
+    'test-util-sockets': ['socket-helpers.c', qom],
     'test-base64': [],
     'test-bufferiszero': [],
     'test-vmstate': [migration, io],
     'test-yank': ['socket-helpers.c', qom, io, chardev]
   }
   if config_host_data.get('CONFIG_INOTIFY1')
-    tests += {'test-util-filemonitor': []}
+    tests += {'test-util-filemonitor': [qom]}
   endif
 
   # Some tests: test-char, test-qdev-global-props, and test-qga,
diff --git a/util/event-loop.c b/util/event-loop.c
index f3e50909a0..c0ddd61f20 100644
--- a/util/event-loop.c
+++ b/util/event-loop.c
@@ -98,10 +98,23 @@ event_loop_backend_complete(UserCreatable *uc, Error **errp)
     }
 }
 
+static bool event_loop_backend_can_be_deleted(UserCreatable *uc)
+{
+    EventLoopBackendClass *bc = EVENT_LOOP_BACKEND_GET_CLASS(uc);
+    EventLoopBackend *backend = EVENT_LOOP_BACKEND(uc);
+
+    if (bc->can_be_deleted) {
+        return bc->can_be_deleted(backend);
+    }
+
+    return true;
+}
+
 static void event_loop_backend_class_init(ObjectClass *klass, void *class_data)
 {
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
     ucc->complete = event_loop_backend_complete;
+    ucc->can_be_deleted = event_loop_backend_can_be_deleted;
 
     object_class_property_add(klass, "poll-max-ns", "int",
                               event_loop_backend_get_param,
diff --git a/util/event-loop.h b/util/event-loop.h
index 8883a0d086..34cf9309af 100644
--- a/util/event-loop.h
+++ b/util/event-loop.h
@@ -24,6 +24,7 @@ struct EventLoopBackendClass {
     ObjectClass parent_class;
 
     void (*init)(EventLoopBackend *backend, Error **errp);
+    bool (*can_be_deleted)(EventLoopBackend *backend);
 };
 
 struct EventLoopBackend {
diff --git a/util/main-loop.c b/util/main-loop.c
index 4d5a5b9943..395fd9bd3e 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -33,6 +33,7 @@
 #include "qemu/error-report.h"
 #include "qemu/queue.h"
 #include "qemu/compiler.h"
+#include "qom/object.h"
 
 #ifndef _WIN32
 #include <sys/wait.h>
@@ -184,6 +185,48 @@ int qemu_init_main_loop(Error **errp)
     return 0;
 }
 
+MainLoop *mloop;
+
+static void main_loop_init(EventLoopBackend *bc, Error **errp)
+{
+    MainLoop *m = MAIN_LOOP(bc);
+
+    if (mloop) {
+        error_setg(errp, "only one main-loop instance allowed");
+        return;
+    }
+
+    mloop = m;
+    return;
+}
+
+static bool main_loop_can_be_deleted(EventLoopBackend *bc)
+{
+    return false;
+}
+
+static void main_loop_class_init(ObjectClass *oc, void *class_data)
+{
+    EventLoopBackendClass *bc = EVENT_LOOP_BACKEND_CLASS(oc);
+
+    bc->init = main_loop_init;
+    bc->can_be_deleted = main_loop_can_be_deleted;
+}
+
+static const TypeInfo main_loop_info = {
+    .name = TYPE_MAIN_LOOP,
+    .parent = TYPE_EVENT_LOOP_BACKEND,
+    .class_init = main_loop_class_init,
+    .instance_size = sizeof(MainLoop),
+};
+
+static void main_loop_register_types(void)
+{
+    type_register_static(&main_loop_info);
+}
+
+type_init(main_loop_register_types)
+
 static int max_priority;
 
 #ifndef _WIN32
-- 
2.35.1


Re: [PATCH 2/3] util/main-loop: Introduce the main loop into QOM
Posted by Markus Armbruster 3 years, 11 months ago
Nicolas Saenz Julienne <nsaenzju@redhat.com> writes:

> 'event-loop-backend' provides basic property handling for all
> 'AioContext' based event loops. So let's define a new 'MainLoopClass'
> that inherits from it. This will permit tweaking the main loop's
> properties through qapi as well as through the command line using the
> '-object' keyword[1]. Only one instance of 'MainLoopClass' might be
> created at any time.
>
> 'EventLoopBackendClass' learns a new callback, 'can_be_deleted()' so as
> to mark 'MainLoop' as non-deletable.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
>
> [1] For example:
>       -object main-loop,id=main-loop,poll-max-ns=<value>

QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>


Re: [PATCH 2/3] util/main-loop: Introduce the main loop into QOM
Posted by Stefan Hajnoczi 3 years, 11 months ago
On Mon, Feb 21, 2022 at 06:08:44PM +0100, Nicolas Saenz Julienne wrote:
> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
> index 8dbc6fcb89..fea5a3e9d4 100644
> --- a/include/qemu/main-loop.h
> +++ b/include/qemu/main-loop.h
> @@ -26,9 +26,20 @@
>  #define QEMU_MAIN_LOOP_H
>  
>  #include "block/aio.h"
> +#include "qom/object.h"
> +#include "util/event-loop.h"
>  
>  #define SIG_IPI SIGUSR1
>  
> +#define TYPE_MAIN_LOOP "main-loop"
> +
> +struct MainLoop {
> +    EventLoopBackend parent_obj;
> +};
> +typedef struct MainLoop MainLoop;
> +
> +DECLARE_INSTANCE_CHECKER(MainLoop, MAIN_LOOP, TYPE_MAIN_LOOP)

 * Direct usage of this macro should be avoided, and the complete
 * OBJECT_DECLARE_TYPE macro is recommended instead.

Is there a reason for using DECLARE_INSTANCE_CHECKER() instead of
OBJECT_DECLARE_TYPE()?

> @@ -882,7 +883,8 @@
>        'input-barrier':              'InputBarrierProperties',
>        'input-linux':                { 'type': 'InputLinuxProperties',
>                                        'if': 'CONFIG_LINUX' },
> -      'iothread':                   'IothreadProperties',
> +      'iothread':                   'EventLoopBackendProperties',
> +      'main-loop':                  'EventLoopBackendProperties',
>        'memory-backend-epc':         { 'type': 'MemoryBackendEpcProperties',
>                                        'if': 'CONFIG_LINUX' },
>        'memory-backend-file':        'MemoryBackendFileProperties',

Does this commit the QAPI schema to keeping iothread and main-loop
properties identical or can they diverge over time, if necessary?

I think we have the freedom to switch the QAPI schema to different
structs for iothread and main-loop in the future because QMP clients
aren't supposed to rely on the exact type, but I wanted to double-check.

> diff --git a/qga/meson.build b/qga/meson.build
> index 1ee9dca60b..3051473e04 100644
> --- a/qga/meson.build
> +++ b/qga/meson.build
> @@ -52,7 +52,7 @@ qga_ss = qga_ss.apply(config_host, strict: false)
>  
>  qga = executable('qemu-ga', qga_ss.sources(),
>                   link_args: config_host['LIBS_QGA'].split(),
> -                 dependencies: [qemuutil, libudev],
> +                 dependencies: [qemuutil, libudev, qom],

Looks like a change because the first patch added the base class to qom
instead of qemuutil. Maybe this can be undone if the base class is added
to qemuutil instead.
Re: [PATCH 2/3] util/main-loop: Introduce the main loop into QOM
Posted by Nicolas Saenz Julienne 3 years, 11 months ago
On Thu, 2022-02-24 at 10:01 +0000, Stefan Hajnoczi wrote:
> On Mon, Feb 21, 2022 at 06:08:44PM +0100, Nicolas Saenz Julienne wrote:
> > diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
> > index 8dbc6fcb89..fea5a3e9d4 100644
> > --- a/include/qemu/main-loop.h
> > +++ b/include/qemu/main-loop.h
> > @@ -26,9 +26,20 @@
> >  #define QEMU_MAIN_LOOP_H
> >  
> >  #include "block/aio.h"
> > +#include "qom/object.h"
> > +#include "util/event-loop.h"
> >  
> >  #define SIG_IPI SIGUSR1
> >  
> > +#define TYPE_MAIN_LOOP "main-loop"
> > +
> > +struct MainLoop {
> > +    EventLoopBackend parent_obj;
> > +};
> > +typedef struct MainLoop MainLoop;
> > +
> > +DECLARE_INSTANCE_CHECKER(MainLoop, MAIN_LOOP, TYPE_MAIN_LOOP)
> 
>  * Direct usage of this macro should be avoided, and the complete
>  * OBJECT_DECLARE_TYPE macro is recommended instead.
> 
> Is there a reason for using DECLARE_INSTANCE_CHECKER() instead of
> OBJECT_DECLARE_TYPE()?

No, bad copying on my part, I'll change it.

[...]
> > diff --git a/qga/meson.build b/qga/meson.build
> > index 1ee9dca60b..3051473e04 100644
> > --- a/qga/meson.build
> > +++ b/qga/meson.build
> > @@ -52,7 +52,7 @@ qga_ss = qga_ss.apply(config_host, strict: false)
> >  
> >  qga = executable('qemu-ga', qga_ss.sources(),
> >                   link_args: config_host['LIBS_QGA'].split(),
> > -                 dependencies: [qemuutil, libudev],
> > +                 dependencies: [qemuutil, libudev, qom],
> 
> Looks like a change because the first patch added the base class to qom
> instead of qemuutil. Maybe this can be undone if the base class is added
> to qemuutil instead.

I'm looking into it.

-- 
Nicolás Sáenz