Stefan Hajnoczi <stefa...@redhat.com> writes:
> On Mon, Dec 11, 2023 at 04:32:06PM +0100, Markus Armbruster wrote:
>> Kevin Wolf <kw...@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 <stefa...@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.
string_output_visitor_new():
* hmp_info_migrate(): format a list of integers, then print it like
monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
One element per vCPU; can produce a long line.
* netfilter_print_info(): format the property values of a
NetFilterState, then print each like
monitor_printf(mon, ",%s=%s", prop->name, str);
* object_property_print(): format a property value of an object, return
the string.
Function is misnamed. object_property_format() or
object_property_to_string() would be better.
Just one caller: qdev_print_props(), helper for hmp_info_qtree().
Prints the string like
qdev_printf("%s = %s\n", props->name,
*value ? value : "<null>");
where qdev_printf() is a macro wrapping monitor_printf().
This one passes human=true, unlike the others. More on that below.
* hmp_info_memdev(): format a list of integers, then print it like
monitor_printf(mon, " policy: %s\n",
HostMemPolicy_str(m->value->policy));
One element per "host node", whatever that may be; might produce a
long line.
* Tests; not relevant here.
hmp_info_migrate() and hmp_info_memdev() use the visitor as a (somewhat
cumbersome) helper for printing uint32List and uint16List, respectively.
Could do without.
The other two display all properties in HMP. Both kind of assume the
string visitor produces no newlines. I think we could instead use the
QObject output visitor, then format the QObject in human-readable form.
Might be less efficient, because we create a temporary QObject. Perhaps
factor out a single helper first.
string_input_visitor_new(), for good measure:
* hmp_migrate_set_parameter(): parse an uint8_t, uint32, size_t, bool,
str, or QAPI enum from a string.
* object_property_parse(): parse a property value from a string, and
assign it to the property.
Calling this object_property_set_from_string() would be better.
Callers:
- object_apply_global_props(): applying compatibility properties
(defined in C) and defauls set with -global (given by user).
- object_set_propv(): helper for convenience functions to set multiple
properties in C.
- hmp_qom_set(): set the property value when not JSON (-j is off).
- object_parse_property_opt(), for accelerator_set_property(), which
processes the argument of -accel.
- x86_cpu_apply_props() and x86_cpu_apply_version_props(): apply
properties defined in C.
The property settings defined in C could just as well use QObject
input visitor instead. Might be less efficient, because we create a
temporary QObject.
The property settings that come from the user are ABI.
Reminder: supports only scalars and lists of small integers.
Supporting structured values containing strings would involve creating
syntax that's basically reinventing JSON.
I think qom-set demonstrates how to support structured values: just
use JSON.
> 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.
With pretty=false, the JSON formatter produces a string without
newlines. With pretty=true, it adds newlines and indentation.
Instead of scanning the formatted string to change its indentation, we
could perhaps pass indentation instructions to the JSON formatter.
> The goal would be to keep the output unchanged as far as possible and to
> emit JSON for structs and lists.
That's basically JSON, except we print 'str' values without quotes and
escapes (resulting in ambiguity when they contain {[]}, and even more
fun when they contain control characters), plus a few more largely
gratuitous differences.
With human=true (advertized as "a bit easier for humans to read"), we
get strings with quotes (still no escapes), and a tweaked set of
gratuitous differences.
> What do you think about this approach?
Format as JSON and call it a day?
I figure the only thing of value we'd lose is displaying integers both
decimal and hex. In some, but not all places.
