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

2022-02-28 Thread Nicolas Saenz Julienne 
On Thu, 2022-02-24 at 10:01 +, 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

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

2022-02-24 Thread Stefan Hajnoczi 
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.


signature.asc
Description: PGP signature

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

2022-02-21 Thread Markus Armbruster 
Nicolas Saenz Julienne  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 
>
> [1] For example:
>   -object main-loop,id=main-loop,poll-max-ns=

QAPI schema
Acked-by: Markus Armbruster

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

2022-02-21 Thread Nicolas Saenz Julienne 
'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 

[1] For example:
  -object main-loop,id=main-loop,poll-max-ns=
---
 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 =