Re: [PATCH 1/3] util & iothread: Introduce event-loop abstract class

[Paolo Bonzini <pbonzini@redhat.com>]

On 2/24/22 10:48, Stefan Hajnoczi wrote:
> On Mon, Feb 21, 2022 at 06:08:43PM +0100, Nicolas Saenz Julienne wrote:
>> diff --git a/qom/meson.build b/qom/meson.build
>> index 062a3789d8..c20e5dd1cb 100644
>> --- a/qom/meson.build
>> +++ b/qom/meson.build
>> @@ -4,6 +4,7 @@ qom_ss.add(files(
>>     'object.c',
>>     'object_interfaces.c',
>>     'qom-qobject.c',
>> +  '../util/event-loop.c',
> 
> This looks strange. I expected util/event-loop.c to be in
> util/meson.build and added to the util_ss SourceSet instead of qom_ss.

Or alternatively, to be in the root just like iothread.c.

Paolo

> What is the reason for this?
> 
>>   ))
>>   
>>   qmp_ss.add(files('qom-qmp-cmds.c'))
>> diff --git a/util/event-loop.c b/util/event-loop.c
>> new file mode 100644
>> index 0000000000..f3e50909a0
>> --- /dev/null
>> +++ b/util/event-loop.c
> 
> The naming is a little inconsistent. The filename "event-loop.c" does
> match the QOM type or typedef name event-loop-backend/EventLoopBackend.
> 
> I suggest calling the source file event-loop-base.c and the QOM type
> "event-loop-base".
> 
>> @@ -0,0 +1,142 @@
>> +/*
>> + * QEMU event-loop backend
>> + *
>> + * Copyright (C) 2022 Red Hat Inc
>> + *
>> + * Authors:
>> + *  Nicolas Saenz Julienne <nsaenzju@redhat.com>
> 
> Most of the code is cut and pasted. It would be nice to carry over the
> authorship information too.
> 
>> +struct EventLoopBackend {
>> +    Object parent;
>> +
>> +    /* AioContext poll parameters */
>> +    int64_t poll_max_ns;
>> +    int64_t poll_grow;
>> +    int64_t poll_shrink;
> 
> These parameters do not affect the main loop because it cannot poll. If
> you decide to keep them in the base class, please document that they
> have no effect on the main loop so users aren't confused. I would keep
> them unique to IOThread for now.