[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 0/5] Introduce "-object iothread-group"
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH RFC 0/5] Introduce "-object iothread-group" |
Date: |
Fri, 14 Jul 2017 14:18:31 +0100 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Wed, Jul 12, 2017 at 07:06:33PM +0800, Fam Zheng wrote:
> On Tue, 07/11 15:58, Stefan Hajnoczi wrote:
> > On Mon, Jul 10, 2017 at 03:20:22PM +0800, Fam Zheng wrote:
> > > Last time we've looked at "-object iothread,spawns=N" but it was a bit
> > > abusive.
> > > A dedicated "iothread-group" class is cleaner from the interface point of
> > > view.
> > > This series does that.
> > >
> > > It has the same set of poll parameters as the existing "iothread" object,
> > > plus
> > > a "size" option to specify how many threads to start. Using iothread-group
> > > doesn't require the user to explicitly create the contained IOThreads. The
> > > IOThreads are created by the group object.
> > >
> > > Internally, IOThreads share one AioContext. This is to make it easier to
> > > adapt
> > > this to the current data plane code (see the last patch). But it is an
> > > implementation detail, and will change depending on the block layer
> > > multiqueue
> > > needs.
> > >
> > > TODO:
> > >
> > > - qmp_query_iothread_groups, in addition to proper QOM @child property
> > > from
> > > IOThreadGroup to its IOThread instances.
> > > - Add virtio-scsi.
> > > - Variant of iothread_stop_all().
> > >
> > > Fam Zheng (5):
> > > aio: Wrap poll parameters into AioContextPollParams
> > > iothread: Don't error on windows
> > > iothread: Extract iothread_start
> > > Introduce iothread-group
> > > virtio-blk: Add iothread-group property
> > >
> > > Makefile.objs | 2 +-
> > > hw/block/dataplane/virtio-blk.c | 18 ++--
> > > hw/block/virtio-blk.c | 6 ++
> > > include/block/aio.h | 18 ++--
> > > include/hw/virtio/virtio-blk.h | 2 +
> > > include/sysemu/iothread.h | 35 ++++++-
> > > iothread-group.c | 210
> > > ++++++++++++++++++++++++++++++++++++++++
> > > iothread.c | 97 +++++++++----------
> > > util/aio-posix.c | 10 +-
> > > util/aio-win32.c | 8 +-
> > > 10 files changed, 328 insertions(+), 78 deletions(-)
> > > create mode 100644 iothread-group.c
> >
> > I reviewed QOM "foo[*]" array syntax but it's very limited. Basically
> > all it does it append the new property to foo[0], foo[1], ... (i.e. it
> > allocates an index). AFAICT there is no way to specify an array
> > property and really arrays are just individual properties.
> >
> > Daniel Berrange has a patch for non-scalar properties:
> > "[PATCH v14 15/21] qom: support non-scalar properties with -object"
> > https://patchwork.kernel.org/patch/9358503/
> >
> > I think the challenge with existing "[*]" or pre-allocated "foo[0]",
> > "foo[1]", ... is that they don't support variable-sized arrays via QAPI.
>
> Plus I'm not a fan of such a "spelling out indexes explicitly" syntax. It
> makes
> me feel like a slave of a poorly coded programme. With it we'll avoid open
> coding an iothread group class in QEMU, but will ask the user to "open type"
> group configurations.
Indexes are necessary if the user wants to remove, swap, etc elements
later, so they do serve a purpose. Sometimes they might not be
necessary.
> > When the virtio-blk-pci object is realized it can loop over iothread[*]
> > properties to set them up (if necessary).
> >
> > Your current RFC series uses a single AioContext in IOThreadGroup. I
> > think something similar can be achieved with an iothread property:
> >
> > -object iothread,id=iothread1,share-event-loop-with=iothread0
> >
> > The reason I am suggesting this approach is to keep IOThread as the
> > first-class object. Existing QMP APIs continue to apply to all objects.
> > We avoid introducing an ad-hoc group object just for IOThread.
>
> Another possibility is to introduce 1) a "TYPE_GROUPABLE" InterfaceInfo that
> will add a ".group" str property to IOThread, and build a generic group object
> out of the objects sharing the same .group name; 2) a qdev prop
> DEFINE_PROP_LINK_TO_GROUP() that can be used to reference a QOM object group
> from virtio-blk/scsi. That way it is not specific to IOThread. (Since I am not
> QOM expert, this is basically brainstorming.)
What you describe sounds like qom/container.c except QOM links are used
instead of a QOM child relationship.
No new QOM interface is necessary in order to do this - the objects in a
group don't have to be aware that they are being grouped.
Stefan
signature.asc
Description: PGP signature
- [Qemu-devel] [PATCH RFC 4/5] Introduce iothread-group, (continued)
- [Qemu-devel] [PATCH RFC 4/5] Introduce iothread-group, Fam Zheng, 2017/07/10
- [Qemu-devel] [PATCH RFC 5/5] virtio-blk: Add iothread-group property, Fam Zheng, 2017/07/10
- Re: [Qemu-devel] [PATCH RFC 0/5] Introduce "-object iothread-group", Fam Zheng, 2017/07/10
- Re: [Qemu-devel] [PATCH RFC 0/5] Introduce "-object iothread-group", Stefan Hajnoczi, 2017/07/11
- Re: [Qemu-devel] [PATCH RFC 0/5] Introduce "-object iothread-group", Stefan Hajnoczi, 2017/07/11