Re: [Qemu-devel] [RFC PATCH 0/2] port network layer onto glib

[Anthony Liguori <anthony@codemonkey.ws>]

Paolo Bonzini <pbonzini@redhat.com> writes:

>> 1) It has no facility for timer events
>
> Yup, it's on the todo list.
>
>> 2) It's tied to file descriptors (only a problem for win32)
>
> The other way round: it's not tied to file descriptors for win32,
> which is already a problem for e.g. networked backends.  main-loop.c
> has the code that is needed, but it's low on the todo list.

Okay, I'll change this to:

2) It doesn't have a cross-platform API for file descriptor/handle
events.

If we tried to use AioContext for networking, the result would be that
we'd break win32.  That's the point I was trying to make.

> Note that tap-win32 for example doesn't need this.

Because tap-win32 doesn't share any code with tap :-/  This is
unfortunate really.

>> 3) The fd support is tied to read/write without an extensibility
>> mechanism for other fd events
>
> Yes.  Though internally it uses g_poll, so it's "just" a matter
> of defining the right API.

Ack especially the "just" part :-)

>  
>> 4) It's got no mechanism to interact with signals
>
> signalfd?  (GSource doesn't have any such mechanism directly).

What I was referring to was the GChildWatch support.  I don't think we
make extensive use of SIGCHLD today though so perhaps that's a minor point.

>
>> 5) It's not obvious how we would integrate with polling-based
>> callbacks like we have in the character layer and networking layer.
>
> Isn't io_flush one such mechanism?  Right now it applies to both
> io_read and io_write, but really it is never used for io_write.

Sort of but it's not extensible.  GSource really gets this right from an
abstraction point of view.  It was pretty straight forward to convert
the character layer by just writing a GSource that allowed polling.

> Also, this and (3) might be the same problem.

I agree.  I think it needs a GSource-like abstraction which is why I've
been arguing to just copy glib's main loop routines out-right.  They've
solved this problem already :-)

>> So I agree it's simple but I don't think it can reasonably stay simple.
>> I think if we added all of the above, the best we would expect to end
>> up with is something that looked like glib.
>>
>> As it stands, the lack of (5) would make it extremely difficult to
>> convert the networking layer.
>
> Quite possible, I've never looked very much at the networking layer.

There's another thing that argues in your favor--or at least for !glib
directly.

I think we want to make the main loops first-class QOM objects.  Right
now all we support is 1-thread per dataplane but I'm positive we're
going to want to be able to group devices on threads.

Being able to express AioContexts as properties and even having a
command line interface to create threads woudl be really nice.
Something like:

qemu -object io-thread,id=thread0 \
     -device virtio-blk,queue[0]=thread0 \
     -device virtio-net,rx[0]=thread0,tx=thread0

Likewise, from a libvirt PoV, being able to do:

qom-get /threads/thread0.tid

Is a real nice touch for pinning purposes.

Regards,

Anthony Liguori

>
> Paolo
>
>> Regards,
>> 
>> Anthony Liguori
>> 
>> >
>> >>> and AioContext's code is vastly simpler than GMainLoop's.
>> >> 
>> >> For now.
>> >
>> > Fair enough. :)
>> >
>> >>> AioContext is also documented and unit tested, with tests
>> >>> for both standalone and GSource operation.  Unit tests for
>> >>> AioContext
>> >>> users are trivial to write, we have one in test-thread-pool.
>> >>>
>> >>>> Did you have a specific concern with using glib vs. AioContext?
>> >>>>  Is it
>> >>>> about reusing code in the block layer where AioContext is
>> >>>> required?
>> >>>
>> >>> In the short term yes, code duplication is a concern.  We already
>> >>> have
>> >>> two implementation of virtio.
>> >> 
>> >> I share your concern but in the opposite direction.  We have three
>> >> main
>> >> loops today.
>> >
>> > Yes, and two of them (main-loop.c/qemu-timer.c and async.c) can be
>> > merged.
>> >
>> >>> I would like the dataplane virtio code to
>> >>> grow everything else that needs to be in all dataplane-style
>> >>> devices
>> >>> (for example, things such as setting up the guest<->host
>> >>> notifiers), and
>> >>> the hw/virtio.c API implemented on top of it (or dead
>> >>> altogether).
>> >>> Usage of AioContext is pretty much forced by the block layer.
>> >> 
>> >> I don't think that AioContext is the right answer because it makes
>> >> it
>> >> too easy to shoot yourself in the foot.
>> >
>> > See above, if nesting is the problem it's gone.
>> >
>> > Paolo
>>