Re: [Qemu-devel] [PATCH v2] vl.c: Support multiple CPU ranges on -numa option

[Markus Armbruster]

Anthony Liguori <address@hidden> writes:

> Markus Armbruster <address@hidden> writes:
>
>> Anthony Liguori <address@hidden> writes:
>>
>>> Paolo Bonzini <address@hidden> writes:
>>>
>>>                                                 What about:
>>>
>>> [numa]
>>> node=1
>>> cpus=2
>>> cpus=3
>>>
>>> qemu -readconfig numa.cfg -numa node=1,cpus=1
>>
>> I figure you mean
>>
>>     [numa]
>>     nodeid=1
>>     cpus=2
>>     cpus=3
>>
>>     qemu -readconfig numa.cfg -numa node,nodeid=1,cpus=1
>>
>> The config file configures node 1 to use cpus 2 and 3.
>>
>> The command line configures node 1 to use cpus 1.
>>
>> QemuOpts can optionally be made to merge options with the same ID.  We
>> use that in cases like -machine, where multiple options make no sense,
>> but merging them does.
>
> With QemuOpts, this is treated as two distinct sections with anonymous
> ids so whatever the code is to handle NUMA options would get called
> twice and it's up to that code to determine how it handles the
> conflict.  (This is admittedly irrelevant to the discussion.)

Correct.

>> Merging options doesn't make sense for -numa.  Therefore, configuration
>> file and command line are *not* merged.  In particular, the two cpus
>> lists are not merged.
>>
>> So, what we have is two conflicting bits of configuration for the same
>> thing.  That's not a new problem, we got that everywhere.  Either treat
>> as error, or let the last one win.  Matter of taste.  Currently, we do
>> the latter.
>>
>>> I'm pretty sure you won't be able to say without looking at the source
>>> code.
>>
>> I hereby certify that I did not look at the source code, only at help
>> output.  So there.
>>
>>> Now what about ranges?  I'm pretty sure that what a user really wants to
>>> say is:
>>>
>>> qemu -numa node=1,cpus=0-3:8-11
>>>
>>> This is easy to do with the patch I sent.  We can add range support
>>> integer lists by just adding a check for '-' before doing dispatch.
>>> That's a heck of a lot nicer than:
>>>
>>> qemu -numa
>>> node=1,cpus=0,cpus=1,cpus=2,cpus=3,cpus=8,cpus=9,cpus=10,cpus=11
>>
>> Let me pick up the baby you just threw out with the bathwater for you:
>>
>>     qemu -numa node,nodeid=1,cpus=0-3,cpus=8-11
>
> If you're okay with making '-' be special syntax, why are you not okay
> with ':' being special syntax?

Fair question!

> What I assume your proposing is making cpus be a string list and then
> parsing within the NUMA code.  Why not do it all in QemuOpts core code?

Yes, and another fair question.

I want common QemuOpts syntax used instead of ad hoc syntax whenever
practical, because ad hoc syntax is bound to breed inconsistencies and
special cases.  QemuOpts was created to get us out of that pit; let's
not jump back in without a really good cause.

We already got lists in QemuOpts.

We don't have intervals in QemuOpts.  If we had, I'd certainly argue for
using them here.  If more uses of intervals turn up, I'll argue for
putting intervals in QemuOpts.