| Message ID | 1376960839-13033-2-git-send-email-gaowanlong@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 08/19/2013 07:07 PM, Wanlong Gao wrote: > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> > --- > qapi-schema.json | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > +## > +# @NumaNodeOptions > +# > +# Create a guest NUMA node. > +# > +# @nodeid: #optional NUMA node ID > +# > +# @cpus: #optional VCPUs belong to this node > +# > +# @mem: #optional memory size of this node (remain as legacy) What does (remain as legacy) mean, that I shouldn't use this parameter? Is it something where the command line parsing code should be translating the legacy option into the correct usage of the QMP command, so we don't have to expose cruft? > +# > +# Since: 1.7 > +## > +{ 'type': 'NumaNodeOptions', > + 'data': { > + '*nodeid': 'uint16', > + '*cpus': ['uint16'], > + '*mem': 'str' }} Why is size passed as a 'str' instead of an integral type? If anything, at the QMP layer, it should be an integer representing size in bytes (the command line and HMP are already capable of converting shorthand like 1G into proper byte counts for use in QAPI). > + > +## > +# @NumaMemOptions > +# > +# Set memory information of guest NUMA node. > +# > +# @nodeid: #optional NUMA node ID > +# > +# @size: #optional memory size of this node If everything is optional, then what defaults are used if I specify nothing? Should nodeid be mandatory (here, and in NumaNodeOptions)?
On 08/22/2013 04:59 AM, Eric Blake wrote: > On 08/19/2013 07:07 PM, Wanlong Gao wrote: >> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> >> --- >> qapi-schema.json | 47 +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 47 insertions(+) >> > >> +## >> +# @NumaNodeOptions >> +# >> +# Create a guest NUMA node. >> +# >> +# @nodeid: #optional NUMA node ID >> +# >> +# @cpus: #optional VCPUs belong to this node >> +# >> +# @mem: #optional memory size of this node (remain as legacy) > > What does (remain as legacy) mean, that I shouldn't use this parameter? > Is it something where the command line parsing code should be > translating the legacy option into the correct usage of the QMP command, > so we don't have to expose cruft? OK, will remove this. > >> +# >> +# Since: 1.7 >> +## >> +{ 'type': 'NumaNodeOptions', >> + 'data': { >> + '*nodeid': 'uint16', >> + '*cpus': ['uint16'], >> + '*mem': 'str' }} > > Why is size passed as a 'str' instead of an integral type? If anything, > at the QMP layer, it should be an integer representing size in bytes > (the command line and HMP are already capable of converting shorthand > like 1G into proper byte counts for use in QAPI). Since the original "mem" options is MB default, but "size" type is byte default, so we should pass a "str" first to be consistent with original option. > >> + >> +## >> +# @NumaMemOptions >> +# >> +# Set memory information of guest NUMA node. >> +# >> +# @nodeid: #optional NUMA node ID >> +# >> +# @size: #optional memory size of this node > > If everything is optional, then what defaults are used if I specify > nothing? Should nodeid be mandatory (here, and in NumaNodeOptions)? The defaults are all consistent with original behaviour. If nodeid is omitted, the option will be assigned node by node from node0. Thanks, Wanlong Gao >
On 08/21/2013 07:12 PM, Wanlong Gao wrote: >>> + '*mem': 'str' }} >> >> Why is size passed as a 'str' instead of an integral type? If anything, >> at the QMP layer, it should be an integer representing size in bytes >> (the command line and HMP are already capable of converting shorthand >> like 1G into proper byte counts for use in QAPI). > > Since the original "mem" options is MB default, but "size" type is byte default, > so we should pass a "str" first to be consistent with original option. No. HMP is human-friendly - it can default to M. QMP is machine-friendly - it should default to bytes and take an 'int' rather than a 'str'. Part of the glue between HMP and QMP is converting from human-friendly to machine-friendly, so that QMP doesn't have to carry cruft. >>> +# >>> +# @nodeid: #optional NUMA node ID >>> +# >>> +# @size: #optional memory size of this node >> >> If everything is optional, then what defaults are used if I specify >> nothing? Should nodeid be mandatory (here, and in NumaNodeOptions)? > > The defaults are all consistent with original behaviour. If nodeid is > omitted, the option will be assigned node by node from node0. What will be assigned? If I omit both nodeid and size, there's nothing left in the object I'm passing. Just because HMP can do sane defaults doesn't mean that QMP needs to mark all fields as optional.
On 08/22/2013 10:29 AM, Eric Blake wrote: > On 08/21/2013 07:12 PM, Wanlong Gao wrote: > >>>> + '*mem': 'str' }} >>> >>> Why is size passed as a 'str' instead of an integral type? If anything, >>> at the QMP layer, it should be an integer representing size in bytes >>> (the command line and HMP are already capable of converting shorthand >>> like 1G into proper byte counts for use in QAPI). >> >> Since the original "mem" options is MB default, but "size" type is byte default, >> so we should pass a "str" first to be consistent with original option. > > No. HMP is human-friendly - it can default to M. QMP is > machine-friendly - it should default to bytes and take an 'int' rather > than a 'str'. Part of the glue between HMP and QMP is converting from > human-friendly to machine-friendly, so that QMP doesn't have to carry cruft. This "mem" options is only for command line options, I can't understand you are saying QMP command here. Because the original "mem" option treat "1024" as "1024MB", but if I set this to "size" type, this "mem" options will treat "1024" as "124B". So I should pass a str first and make it to "MB" default in the options parse function to be consistent with original one. > > >>>> +# >>>> +# @nodeid: #optional NUMA node ID >>>> +# >>>> +# @size: #optional memory size of this node >>> >>> If everything is optional, then what defaults are used if I specify >>> nothing? Should nodeid be mandatory (here, and in NumaNodeOptions)? >> >> The defaults are all consistent with original behaviour. If nodeid is >> omitted, the option will be assigned node by node from node0. > > What will be assigned? If I omit both nodeid and size, there's nothing If the "-m" option assigned the total memory size is 2G, then if you omit both nodeid and memory size in the options, for example two "-numa mem," options here, it will split total memory across these two node to : node0 1G node1 1G This is the original behaviour and I didn't change any. Thanks, Wanlong Gao > left in the object I'm passing. Just because HMP can do sane defaults > doesn't mean that QMP needs to mark all fields as optional. >
On 08/22/13 05:16, Wanlong Gao wrote: > On 08/22/2013 10:29 AM, Eric Blake wrote: >> On 08/21/2013 07:12 PM, Wanlong Gao wrote: >> >>>>> + '*mem': 'str' }} >>>> >>>> Why is size passed as a 'str' instead of an integral type? If anything, >>>> at the QMP layer, it should be an integer representing size in bytes >>>> (the command line and HMP are already capable of converting shorthand >>>> like 1G into proper byte counts for use in QAPI). >>> >>> Since the original "mem" options is MB default, but "size" type is byte default, >>> so we should pass a "str" first to be consistent with original option. >> >> No. HMP is human-friendly - it can default to M. QMP is >> machine-friendly - it should default to bytes and take an 'int' rather >> than a 'str'. Part of the glue between HMP and QMP is converting from >> human-friendly to machine-friendly, so that QMP doesn't have to carry cruft. > > This "mem" options is only for command line options, I can't understand you > are saying QMP command here. Because the original "mem" option treat "1024" > as "1024MB", but if I set this to "size" type, this "mem" options will > treat "1024" as "124B". So I should pass a str first and make it to "MB" > default in the options parse function to be consistent with original one. Yes. This part of the schema is not for exposure over QMP, it just generates stuff for OptsVisitor, and it must remain compatible with the original, manual parsing of the option. This came up for V6: http://thread.gmane.org/gmane.comp.emulators.qemu/225678/focus=225714 Laszlo
On 08/22/2013 02:46 AM, Laszlo Ersek wrote: > Yes. This part of the schema is not for exposure over QMP, it just > generates stuff for OptsVisitor, and it must remain compatible with the > original, manual parsing of the option. > > This came up for V6: > > http://thread.gmane.org/gmane.comp.emulators.qemu/225678/focus=225714 My fault for coming into the conversation late, but a note to that effect in the commit log, and/or in the description of why this type is listed in the qapi document, would be handy.
On 08/22/13 18:14, Eric Blake wrote: > On 08/22/2013 02:46 AM, Laszlo Ersek wrote: >> Yes. This part of the schema is not for exposure over QMP, it just >> generates stuff for OptsVisitor, and it must remain compatible with the >> original, manual parsing of the option. >> >> This came up for V6: >> >> http://thread.gmane.org/gmane.comp.emulators.qemu/225678/focus=225714 > > My fault for coming into the conversation late, but a note to that > effect in the commit log, and/or in the description of why this type is > listed in the qapi document, would be handy. I agree. We should probably tack a banner to each such structure in the qapi schema json. Thanks Laszlo
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Il 22/08/2013 04:29, Eric Blake ha scritto: >>>>> If everything is optional, then what defaults are used if >>>>> I specify nothing? Should nodeid be mandatory (here, and >>>>> in NumaNodeOptions)? >>> >>> The defaults are all consistent with original behaviour. If >>> nodeid is omitted, the option will be assigned node by node >>> from node0. > What will be assigned? If I omit both nodeid and size, there's > nothing left in the object I'm passing. Just because HMP can do > sane defaults doesn't mean that QMP needs to mark all fields as > optional. Even though this is for the command-line, I agree that both should be mandatory. The default (splitting memory equally across nodes) can be specified by not having "-numa mem" at all, so there is no need to have short versions of the "-numa mem" command line. Paolo -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJSFmTSAAoJEBvWZb6bTYbyzUsP/3BhPGBHfzNsFHdwPL6aiBW4 67TJJ/X9GB4wjav6yjtKtBGuDNOGmEO8BkyJZVNUzw051t74uPpYzDIgd7QvrsU/ nMFRFX6q3vRUxX7k9L5RP3OkJbcAgaq8YQ135XNh59bOEd/VOeTrrEtgqyVPo45h 6tt0JSlGDPF9qc+jGoqgQN8522nvV8mcF/XhJdQ7DqttpAP75ECxViKFKezXwrsh H3dX6v0SbWBMAlihldxEMte8hlKNB/Asb3CDQz5L2ySaPGHcwOI06wSVKGD9slbv 4SSiGg2XMdPx0VKh0ozlKmXE46+hlDJkxi7JLEuscSmZL9gZwFpenz2+29OBk2MM g8fwxKkm36jJE7cnn/z3TPiTVZ7sJm8LvtRW2183CC5I9LpTIUrCZxxP/06ZOREq 2fH41A3PyxDOvhOwp8B3XDAnOkmklSr86hiZR+s3I+59uPa9KzOY2WFeG/6w90uq 6EmZNbmkLrZYfns7wk24YRhH9DAgTFP9GmceaJu0lEbk41INeZggswugwPvrFNI3 bPsXacwK5Y4T4ZkRs4vyaw4Dn2B732D2CMvmPJ+EpKeVIc+8tKMXNtuFKybiraQi zrlHk4zGjeUZwW8+cx8xkHe3qyKUwbYFuRvDqSKXYRrQMzzJNMf/rNGb4JcDHiH6 BmVejHjaYKPJKA+LF3hB =Oqhm -----END PGP SIGNATURE-----
diff --git a/qapi-schema.json b/qapi-schema.json index a51f7d2..b9b18e4 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3773,3 +3773,50 @@ ## { 'command': 'query-rx-filter', 'data': { '*name': 'str' }, 'returns': ['RxFilterInfo'] } + +## +# @NumaOptions +# +# A discriminated record of NUMA options. +# +# Since 1.7 +## +{ 'union': 'NumaOptions', + 'data': { + 'node': 'NumaNodeOptions', + 'mem': 'NumaMemOptions' }} + +## +# @NumaNodeOptions +# +# Create a guest NUMA node. +# +# @nodeid: #optional NUMA node ID +# +# @cpus: #optional VCPUs belong to this node +# +# @mem: #optional memory size of this node (remain as legacy) +# +# Since: 1.7 +## +{ 'type': 'NumaNodeOptions', + 'data': { + '*nodeid': 'uint16', + '*cpus': ['uint16'], + '*mem': 'str' }} + +## +# @NumaMemOptions +# +# Set memory information of guest NUMA node. +# +# @nodeid: #optional NUMA node ID +# +# @size: #optional memory size of this node +# +# Since 1.7 +## +{ 'type': 'NumaMemOptions', + 'data': { + '*nodeid': 'uint16', + '*size': 'size' }}