| Message ID | 20170623124700.1389-6-el13635@mail.ntua.gr |
|---|---|
| State | New |
| Headers | show |
On Fri, Jun 23, 2017 at 03:46:57PM +0300, Manos Pitsidianakis wrote: > This is needed to configure throttle filter driver nodes with QAPI. > > Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> > --- > qapi/block-core.json | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 06/23/2017 07:46 AM, Manos Pitsidianakis wrote: > This is needed to configure throttle filter driver nodes with QAPI. > > Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> > --- > qapi/block-core.json | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index f85c2235c7..1d4afafe8c 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -2119,7 +2119,7 @@ > 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', > 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', > 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh', > - 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } > + 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } We already have a '@vxhs: Since 2.10' comment, you should add a similar one for @throttle. > + > +## > +# @BlockdevOptionsThrottle: > +# > +# Driver specific block device options for Throttle > +# > +# @throttling-group: the name of the throttling group to use > +# > +# @options: BlockIOThrottle options > +# Since: 2.9 You missed 2.9; this should be 2.10. > +## > +{ 'struct': 'BlockdevOptionsThrottle', > + 'data': { 'throttling-group': 'str', > + 'file' : 'BlockdevRef', Missing documentation for 'file'. > + '*options' : 'BlockIOThrottle' > + } } >
On Fri 23 Jun 2017 02:46:57 PM CEST, Manos Pitsidianakis wrote: > +# @BlockdevOptionsThrottle: > +# > +# Driver specific block device options for Throttle > +# I would put this earlier in the json file, together with the rest of the BlockdevOptions* structs. > +# @throttling-group: the name of the throttling group to use Why not call it simply "group" ? > +# > +# @options: BlockIOThrottle options > +# Since: 2.9 > +## > +{ 'struct': 'BlockdevOptionsThrottle', > + 'data': { 'throttling-group': 'str', > + 'file' : 'BlockdevRef', > + '*options' : 'BlockIOThrottle' > + } } Not sure if 'file' is the best name for the field ("child"?), but I'm fine with it. Berto
On Wed, Jun 28, 2017 at 03:35:48PM +0200, Alberto Garcia wrote: >On Fri 23 Jun 2017 02:46:57 PM CEST, Manos Pitsidianakis wrote: > >> +# @BlockdevOptionsThrottle: >> +# >> +# Driver specific block device options for Throttle >> +# > >I would put this earlier in the json file, together with the rest of the >BlockdevOptions* structs. > >> +# @throttling-group: the name of the throttling group to use > >Why not call it simply "group" ? Sure! :) > >> +# >> +# @options: BlockIOThrottle options >> +# Since: 2.9 >> +## >> +{ 'struct': 'BlockdevOptionsThrottle', >> + 'data': { 'throttling-group': 'str', >> + 'file' : 'BlockdevRef', >> + '*options' : 'BlockIOThrottle' >> + } } > >Not sure if 'file' is the best name for the field ("child"?), but I'm >fine with it. There doesn't seem to be a consistent naming scheme in block-core.json ("file", "image" are candidates) so I put it file after bs->file.
Am 23.06.2017 um 14:46 hat Manos Pitsidianakis geschrieben: > This is needed to configure throttle filter driver nodes with QAPI. > > Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> > --- > qapi/block-core.json | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index f85c2235c7..1d4afafe8c 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -2119,7 +2119,7 @@ > 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', > 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', > 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh', > - 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } > + 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } > > ## > # @BlockdevOptionsFile: > @@ -2984,6 +2984,7 @@ > 'replication':'BlockdevOptionsReplication', > 'sheepdog': 'BlockdevOptionsSheepdog', > 'ssh': 'BlockdevOptionsSsh', > + 'throttle': 'BlockdevOptionsThrottle', > 'vdi': 'BlockdevOptionsGenericFormat', > 'vhdx': 'BlockdevOptionsGenericFormat', > 'vmdk': 'BlockdevOptionsGenericCOWFormat', > @@ -3723,3 +3724,19 @@ > 'data' : { 'parent': 'str', > '*child': 'str', > '*node': 'str' } } > + > +## > +# @BlockdevOptionsThrottle: > +# > +# Driver specific block device options for Throttle > +# > +# @throttling-group: the name of the throttling group to use > +# > +# @options: BlockIOThrottle options Missing #optional marker. > +# Since: 2.9 > +## > +{ 'struct': 'BlockdevOptionsThrottle', > + 'data': { 'throttling-group': 'str', > + 'file' : 'BlockdevRef', > + '*options' : 'BlockIOThrottle' > + } } Didn't we intend to make 'throttling-group' optional, too? If we don't, then the question of anonymous ThrottleGroup objects is kind of moot (not completely because -drive isn't bound to the schema, but in that case we should just error out there too if it's missing). Kevin
On 06/28/2017 10:50 AM, Kevin Wolf wrote: > Am 23.06.2017 um 14:46 hat Manos Pitsidianakis geschrieben: >> This is needed to configure throttle filter driver nodes with QAPI. >> >> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> >> --- >> qapi/block-core.json | 19 ++++++++++++++++++- >> 1 file changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index f85c2235c7..1d4afafe8c 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> @@ -2119,7 +2119,7 @@ >> 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', >> 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', >> 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh', >> - 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } >> + 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } >> >> ## >> # @BlockdevOptionsFile: >> @@ -2984,6 +2984,7 @@ >> 'replication':'BlockdevOptionsReplication', >> 'sheepdog': 'BlockdevOptionsSheepdog', >> 'ssh': 'BlockdevOptionsSsh', >> + 'throttle': 'BlockdevOptionsThrottle', >> 'vdi': 'BlockdevOptionsGenericFormat', >> 'vhdx': 'BlockdevOptionsGenericFormat', >> 'vmdk': 'BlockdevOptionsGenericCOWFormat', >> @@ -3723,3 +3724,19 @@ >> 'data' : { 'parent': 'str', >> '*child': 'str', >> '*node': 'str' } } >> + >> +## >> +# @BlockdevOptionsThrottle: >> +# >> +# Driver specific block device options for Throttle >> +# >> +# @throttling-group: the name of the throttling group to use >> +# >> +# @options: BlockIOThrottle options > > Missing #optional marker. The marker is now auto-generated based solely on the '*options' below, so we don't need a redundant thing here. > >> +# Since: 2.9 >> +## >> +{ 'struct': 'BlockdevOptionsThrottle', >> + 'data': { 'throttling-group': 'str', >> + 'file' : 'BlockdevRef', >> + '*options' : 'BlockIOThrottle' >> + } } > > Didn't we intend to make 'throttling-group' optional, too? > > If we don't, then the question of anonymous ThrottleGroup objects is > kind of moot (not completely because -drive isn't bound to the schema, > but in that case we should just error out there too if it's missing). > > Kevin > >
Am 28.06.2017 um 18:02 hat Eric Blake geschrieben: > On 06/28/2017 10:50 AM, Kevin Wolf wrote: > > Am 23.06.2017 um 14:46 hat Manos Pitsidianakis geschrieben: > >> This is needed to configure throttle filter driver nodes with QAPI. > >> > >> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> > >> --- > >> qapi/block-core.json | 19 ++++++++++++++++++- > >> 1 file changed, 18 insertions(+), 1 deletion(-) > >> > >> diff --git a/qapi/block-core.json b/qapi/block-core.json > >> index f85c2235c7..1d4afafe8c 100644 > >> --- a/qapi/block-core.json > >> +++ b/qapi/block-core.json > >> @@ -2119,7 +2119,7 @@ > >> 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', > >> 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', > >> 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh', > >> - 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } > >> + 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } > >> > >> ## > >> # @BlockdevOptionsFile: > >> @@ -2984,6 +2984,7 @@ > >> 'replication':'BlockdevOptionsReplication', > >> 'sheepdog': 'BlockdevOptionsSheepdog', > >> 'ssh': 'BlockdevOptionsSsh', > >> + 'throttle': 'BlockdevOptionsThrottle', > >> 'vdi': 'BlockdevOptionsGenericFormat', > >> 'vhdx': 'BlockdevOptionsGenericFormat', > >> 'vmdk': 'BlockdevOptionsGenericCOWFormat', > >> @@ -3723,3 +3724,19 @@ > >> 'data' : { 'parent': 'str', > >> '*child': 'str', > >> '*node': 'str' } } > >> + > >> +## > >> +# @BlockdevOptionsThrottle: > >> +# > >> +# Driver specific block device options for Throttle > >> +# > >> +# @throttling-group: the name of the throttling group to use > >> +# > >> +# @options: BlockIOThrottle options > > > > Missing #optional marker. > > The marker is now auto-generated based solely on the '*options' below, > so we don't need a redundant thing here. Oh nice, progress! Kevin
diff --git a/qapi/block-core.json b/qapi/block-core.json index f85c2235c7..1d4afafe8c 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2119,7 +2119,7 @@ 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh', - 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } + 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } ## # @BlockdevOptionsFile: @@ -2984,6 +2984,7 @@ 'replication':'BlockdevOptionsReplication', 'sheepdog': 'BlockdevOptionsSheepdog', 'ssh': 'BlockdevOptionsSsh', + 'throttle': 'BlockdevOptionsThrottle', 'vdi': 'BlockdevOptionsGenericFormat', 'vhdx': 'BlockdevOptionsGenericFormat', 'vmdk': 'BlockdevOptionsGenericCOWFormat', @@ -3723,3 +3724,19 @@ 'data' : { 'parent': 'str', '*child': 'str', '*node': 'str' } } + +## +# @BlockdevOptionsThrottle: +# +# Driver specific block device options for Throttle +# +# @throttling-group: the name of the throttling group to use +# +# @options: BlockIOThrottle options +# Since: 2.9 +## +{ 'struct': 'BlockdevOptionsThrottle', + 'data': { 'throttling-group': 'str', + 'file' : 'BlockdevRef', + '*options' : 'BlockIOThrottle' + } }
This is needed to configure throttle filter driver nodes with QAPI. Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> --- qapi/block-core.json | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)