[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Replace GSource with AioContext for chardev
From: |
Daniel P . Berrangé |
Subject: |
Re: Replace GSource with AioContext for chardev |
Date: |
Tue, 14 Apr 2020 11:27:53 +0100 |
User-agent: |
Mutt/1.13.3 (2020-01-12) |
On Tue, Apr 14, 2020 at 09:25:58AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <address@hidden> writes:
>
> > On Thu, Apr 09, 2020 at 03:16:01PM +0200, Marc-André Lureau wrote:
> >> Hi
> >>
> >> On Thu, Apr 9, 2020 at 2:46 PM Coiby Xu <address@hidden> wrote:
> >> >
> >> >
> >> > Hi,
> >> >
> >> > I'm now implementing vhost-user block device backend
> >> > https://patchew.org/QEMU/address@hidden/
> >> > and want to use chardev to help manage vhost-user client connections
> >> > and read socket message. However there are two issues that need to be
> >> > addressed.
> >> >
> >> > Firstly, chardev isn't suitable for the case when exported drive is
> >> > run in an IOThread because for mow chardev use GSource to dispatch
> >> > socket fd events. So I have to specify which IOThread the exported
> >> > drive is using when launching vhost-user block device backend,
> >> > for example, the following syntax will be used,
> >> >
> >> > -drive file=file.img,id=disk -device
> >> > virtio-blk,drive=disk,iothread=iothread0 \
> >> > -object
> >> > vhost-user-blk-server,node-name=disk,chardev=mon1,iothread=iothread0 \
> >> > -object iothread,id=iothread0 \
> >> > -chardev
> >> > socket,id=mon1,path=/tmp/vhost-user-blk_vhost.socket,server,nowait
> >> >
> >> > then iothread_get_g_main_context(IOThread *iothread) has to be called
> >> > to run the gcontext in IOThread. If we use AioContext to dispatch socket
> >> > fd events, we needn't to specify IOThread twice. Besides aio_poll is
> >> > faster
> >> > than g_main_loop_run.
> >> >
> >> > Secondly, socket chardev's async read handler (set through
> >> > qemu_chr_fe_set_handlers) doesn't take the case of socket short read
> >> > into consideration. I plan to add one which will make use
> >> > qio_channel_yield.
> >> >
> >> > According to
> >> > [1] Improving the QEMU Event Loop - Linux Foundation Events
> >> > http://events17.linuxfoundation.org/sites/events/files/slides/Improving%20the%20QEMU%20Event%20Loop%20-%203.pdf
> >> >
> >> > "Convert chardev GSource to aio or an equivalent source" (p.30) should
> >> > have
> >> > been finished. I'm curious why the plan didn't continue. If it's
> >> > desirable,
> >> > I'm going to finish the leftover work to resolve the aforementioned two
> >> > issues.
> >>
> >> Converting all chardevs to Aio might be challenging, and doesn't bring
> >> much benefits imho.
> >>
> >> Perhaps a better approach would be to rely on a new chardev API to
> >> steal the chardev underlying fd or QIO... (mostly keeping -chardev for
> >> CLI/QMP compatibility reason - although breaking some chardev features
> >> that imho aren't compatible with all use cases, like replay, muxing,
> >> swapping etc). The chardev should probably be removed after that...
> >
> > Yeah, I feel like it was a mistake for us to wire up many of our features
> > to chardevs. We mostly did it because -chardev provides a pre-existing
> > syntax for TCP/UNIX sockets and we didn't want to invent new CLI args.
> > IMHO this was a mistake in retrospect.
> >
> > Unfortunately the -chardev API is absolutely terrible for any usage that
> > actually cares about the connection based semantics. Witness the horrible
> > hacks we do for re-connect and re-try when failing to initially connect
> > in vhost-user net code.
> >
> > For features in QEMU where the only desirable chardev backend is one with
> > connection based, socket semantics, I think we would be better off using
> > the QIOChannel APIs directly and completely avoiding the chardev code.
>
> How do we get from here to there?
Ignoring back compat, what would be our ideal CLI syntax ?
Current syntax is
-chardev socket,id=charnet1,path=/tmp/vhost1.sock
-netdev vhost-user,chardev=charnet1,id=hostnet1
Should we have an option that expresses a "SocketAddress" struct on the
CLI ?
-socket type=unix,path=/tmp/vhost1.sock,id=sock0
-netdev vhost-user,socket=sock0,id=hostnet1
IIUC, Marc-André is suggesting that we carry on using -chardev, but
detect when it is a socket chardev, and then ignore chardev APIs and
create a QIOChannel. I can see some appeal in this as it provides a
way to get all existing usage switched over, but I feel uneasy about
sticking with -chardev forever, if we're not actually using a chardev.
We could do the magic -chardev -> -socket conversion though, for a
short period of time to ease the transition.
We would have to
1. Introduce the new -socket and add "socket=$id" to devices that need it
2. Deprecate -chardev with type != socket, with no repacement intended
3. Deprecate -chardev with type == socket, translating to -socket
...wait 2 releases...
4. Delete support for "chardev=$id" from devices with "socket=$id"
The hardest part is probably deciding exactly which set of devices can
be restricted to only sockets, and which must have the full range of
chardev backends available.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|