Re: [PATCH v2 1/2] qdev: add IOThreadVirtQueueMappingList property type

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3069 bytes --]

On Mon, Dec 11, 2023 at 04:32:06PM +0100, Markus Armbruster wrote:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 18.09.2023 um 18:16 hat Stefan Hajnoczi geschrieben:
> >> virtio-blk and virtio-scsi devices will need a way to specify the
> >> mapping between IOThreads and virtqueues. At the moment all virtqueues
> >> are assigned to a single IOThread or the main loop. This single thread
> >> can be a CPU bottleneck, so it is necessary to allow finer-grained
> >> assignment to spread the load.
> >> 
> >> Introduce DEFINE_PROP_IOTHREAD_VQ_MAPPING_LIST() so devices can take a
> >> parameter that maps virtqueues to IOThreads. The command-line syntax for
> >> this new property is as follows:
> >> 
> >>   --device '{"driver":"foo","iothread-vq-mapping":[{"iothread":"iothread0","vqs":[0,1,2]},...]}'
> >> 
> >> IOThreads are specified by name and virtqueues are specified by 0-based
> >> index.
> >> 
> >> It will be common to simply assign virtqueues round-robin across a set
> >> of IOThreads. A convenient syntax that does not require specifying
> >> individual virtqueue indices is available:
> >> 
> >>   --device '{"driver":"foo","iothread-vq-mapping":[{"iothread":"iothread0"},{"iothread":"iothread1"},...]}'
> >> 
> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >
> > When testing this, Qing Wang noticed that "info qtree" crashes. This is
> > because the string output visitor doesn't support structs. I suppose
> > IOThreadVirtQueueMapping is the first struct type that is used in a qdev
> > property type.
> >
> > So we'll probably have to add some kind of struct support to the string
> > output visitor before we can apply this. Even if it's as stupid as just
> > printing "<struct IOThreadVirtQueueMapping>" without actually displaying
> > the value.
> 
> The string visitors have been nothing but trouble.
> 
> For input, we can now use keyval_parse() and the QObject input visitor
> instead.  Comes with restrictions, but I'd argue it's a more solid base
> than the string input visitor.
> 
> Perhaps we can do something similar for output: create a suitable
> formatter for use it with the QObject output visitor, replacing the
> string output visitor.

I sent an initial patch that just shows "<omitted>" but would like to
work on a proper solution with your input.

From what I've seen StringOutputVisitor is used in several places in
QEMU. "info qtree" calls it through object_property_print() to print
individual qdev properties. I don't understand the requirements of the
other callers, but object_property_print() wants to return a single
string without newlines.

QObjectOutputVisitor produces a QObject. That could be formatted as
JSON using qobject_to_json_pretty()?

The pretty JSON would contain newlines and existing callers such as
"info qtree" may need to scan the output and insert indentation.

The goal would be to keep the output unchanged as far as possible and to
emit JSON for structs and lists.

What do you think about this approach?

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]