Re: [PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
I checked "QEMU_CAPS_OBJECT_JSON" in v3 https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/HIIXCOZG2AVNJJAO5KTPJHXRU4GPO2HI/
Re: [PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
fixed in v3 https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/HIIXCOZG2AVNJJAO5KTPJHXRU4GPO2HI/
Re: [PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
On Thu, Jun 06, 2024 at 08:07:37 -, Chun Feng Wu wrote: > for comment > "Preferrably all these definitions should be shared with as any > change would now need to change two places." Trimming the context and mentioning this random bit makes it rather hard to remember what I've based that comment on ... > > do you mean we use domain xml like the following one(which reuse > definition): ... so I have no idea how to respond to this without going back myself. Plese preferably keep the context you are responding to rather than starting a random new message. Let me do that for you now: > > @@ -6637,6 +6641,164 @@ > > > > > > > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > Preferrably all these definitions should be shared with as any > change would now need to change two places. > > It is okay if the schema is more lax (e.g. allowing 'group_name' to be > optional in the schema, which is then strictly required by the parser), > but it's not okay to have to change two distinct places. > > > + > > + > > + > > + > > + > > + > > do you mean we use domain xml like the following one(which reuse > definition): > > ... > > > limit0 > 1000 > 40 > 10 > > > ... > > > > > > > > No, what I meant is to not reimplement the whole definition on the schema jus tto make 'group_name' required by the schema itself. You can make it required in the parser, and keep it optional in the schema so that the schema can be reused in both places rather than having two distinct copies of it that differ just in the optionality of 'group_name'.
Re: [PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
On Thu, Jun 06, 2024 at 07:58:04 -, Chun Feng Wu wrote:
> Thanks Peter for above comments!
>
> My original design goal is exact the same as what QEMU doc says at
> https://github.com/qemu/qemu/blob/master/docs/throttle.txt:
> "In this example the individual drives have IOPS limits of 2000, 2500
> and 3000 respectively but the total combined I/O can never exceed 4000
> IOPS."
>
> I haven't thought about such case: "somebody would want to apply different
> throttling on a backing image", do we have such case design for other feature?
Currently, we support configuring some specifics such as the qcow2
metadata cache, which can be configured per-image and not just per disk:
https://www.libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms
look for 'metadata_cache'. While the patchet doesn't necessarily need to
implement it for backing images at this point you need to do it so that
future change will be available.
> About configuring old throttling via the 'throttle' blockdev layer, it seems
> possible, however, this new design and implementation has dependency on
> QEMU6, the reason about this is that starting from QEMU6, "-object"(case to
> launching vm along with throttlegroup) supports json format value: e.g.
> -object
> '{"qom-type":"throttle-group","id":"limits0","limits":{"iops-total":200}}'),
> while for qemu4.2, non-stable API works: e.g. -object
> throttle-group,id=limits0,x-iops-total=200, current implementation follows
> json way: it calls "qemuBuildObjectCommandlineFromJSON" to create
> throttle-group object, within "qemuBuildObjectCommandlineFromJSON", I see
> check about QEMU_CAPS_OBJECT_JSON: virQEMUCapsGet(qemuCaps,
> QEMU_CAPS_OBJECT_JSON)
Okay, but I don't see any code in this series which would limit it to
the appropriate qemu versions. In cases when this is not supported by
all qemu releases libvirt supports (currently qemu-4.2 and later) you'll
have to add a capability and interlock the new feature based on it.
>
> in addition, for "object-add"(case to hot attach disk with throttles), it
> seems QMP json format is different between QEMU 6 and QEMU 4.2 ("props"
> required)
>
Okay another case when you must stay compatible and/or reject new
configs with old qemu.
Note that adding support for experimental features (x-NAME) is not
acceptable.
Re: [PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
for comment "Preferrably all these definitions should be shared with as any change would now need to change two places." do you mean we use domain xml like the following one(which reuse definition): ... limit0 1000 40 10 ... ...
Re: [PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
Thanks Peter for above comments!
My original design goal is exact the same as what QEMU doc says at
https://github.com/qemu/qemu/blob/master/docs/throttle.txt:
"In this example the individual drives have IOPS limits of 2000, 2500
and 3000 respectively but the total combined I/O can never exceed 4000
IOPS."
I haven't thought about such case: "somebody would want to apply different
throttling on a backing image", do we have such case design for other feature?
About configuring old throttling via the 'throttle' blockdev layer, it seems
possible, however, this new design and implementation has dependency on QEMU6,
the reason about this is that starting from QEMU6, "-object"(case to launching
vm along with throttlegroup) supports json format value: e.g. -object
'{"qom-type":"throttle-group","id":"limits0","limits":{"iops-total":200}}'),
while for qemu4.2, non-stable API works: e.g. -object
throttle-group,id=limits0,x-iops-total=200, current implementation follows json
way: it calls "qemuBuildObjectCommandlineFromJSON" to create throttle-group
object, within "qemuBuildObjectCommandlineFromJSON", I see check about
QEMU_CAPS_OBJECT_JSON: virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_JSON)
in addition, for "object-add"(case to hot attach disk with throttles), it seems
QMP json format is different between QEMU 6 and QEMU 4.2 ("props" required)
Re: [PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
On Tue, May 14, 2024 at 14:49:58 +0200, Peter Krempa wrote: > On Thu, Apr 11, 2024 at 19:01:55 -0700, w...@linux.ibm.com wrote: [...] > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > > index e2f66b982c..ee9ee8b10c 100644 > > --- a/docs/formatdomain.rst > > +++ b/docs/formatdomain.rst [] > > @@ -3185,6 +3222,17 @@ paravirtualized driver is specified via the ``disk`` > > element. > > :since:`since after 0.4.4`; "sata" attribute value :since:`since 0.9.7`; > > "removable" attribute value :since:`since 1.1.3`; > > "rotation_rate" attribute value :since:`since 7.3.0` > > +``throttlefilters`` > > + The optional ``throttlefilters`` element provides the ability to > > provide additional > > + per-device throttle chain :since:`Since 10.3.0` > > + For example, if we have four different disks and we want to limit I/O > > for each one > > + and we also want to limit combined I/O of all four disks, we can > > leverage > > + ``throttlefilters`` to achieve this goal by setting two > > ``throttlefilter`` for > > + each disk: disk's own filter and combined filter. ``throttlefilters`` > > and ``iotune`` > > + should be used exclusively. > > Please also document how the group chaining is applied in terms of Oops forgot to finish my thought. Please also document how the group chaining is applied in terms of the impact on the actual throttle groups. I'm also wondering if it's okay to apply throttling just at disk level. Libvirt now allows configuring also backing images specifically and the qemu throttling infra can be applied at any point in the backing chain, thus I wonder if it makes sense to do that here. How do you expect this to be used? what are your design goals? I'm trying to prevent situation when the throttling will be deemed insufficient if e.g. somebody would want to apply different throttling on a backing image. I'm also thinking about the integration with the way to configure this. These patches sidestep the issue by disallowing the combination, but I'd really like to also configure the old throttling via the 'throttle' blockdev layer. One additional thing that I didn't yet check: note that libvirt supports also qemu-4.2, thus all of what you've added needs to be either supported by all qemu versions, or we'll need a capability and lock-out any versions which don't support everything you need. I'm specifically asking about this, as I remember that there were some things which didn't work right at the time I was converting libvirt to use '-blockdev' and that's why it's sill using the old command to set throttling.
Re: [PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
On Thu, Apr 11, 2024 at 19:01:55 -0700, w...@linux.ibm.com wrote: > From: Chun Feng Wu > > * Add new elements '' and '' > * contains defintions > * can include multiple throttlegroup references to form > filter chain in qemu > * Chained throttle filters feature in qemu is described at > https://github.com/qemu/qemu/blob/master/docs/throttle.txt > > Signed-off-by: Chun Feng Wu > --- Please separate the changes into a separate patch from throttlefilters. As I've noted previously you'll also need to add test XML files, ideally for qemuxmlconftest, which will apart from testing the commandline output test also the XML schema. > docs/formatdomain.rst | 48 + > src/conf/schemas/domaincommon.rng | 164 +- > 2 files changed, 211 insertions(+), 1 deletion(-) > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index e2f66b982c..ee9ee8b10c 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -1957,6 +1957,34 @@ advertisements to the guest OS. (NB: Only qemu driver > support) > the guest OS itself can choose to circumvent the unavailability of the > sleep > states (e.g. S4 by turning off completely). > > +Throttle Group Management > +- Please make it more obvious that this is for disk throughput shaping ... > + > +:since:`Since 10.3.0` it is possible to create multiple named throttle groups > +and then reference them within ``throttlefilters`` to form filter chain in > QEMU for > +specific disk. The limits(throttlegroups) are shared within domain, hence > the same group ... as the word 'disk' appears only here. > +can be referenced by different filters. > + > +:: > + > + > + ... > + > + > + limit0 > + 1000 > + 40 > + 10 > + > + > + ... > + > + > +All throttlegroups are listed within the ``throttlegroups`` element This feels redundndant. > + > +``throttlegroup`` > + It has the same sub-elements as ``iotune`` (See `Hard drives, floppy > disks, CDROMs`_), > + The difference is that is required. I already forgot how this is formatted, but you'll need to make sure that the group name is formatted first as in the example. > > Hypervisor features > --- > @@ -2704,6 +2732,15 @@ paravirtualized driver is specified via the ``disk`` > element. > > > > + > + > + > + > + > + > + > + > + > > ... > > @@ -3185,6 +3222,17 @@ paravirtualized driver is specified via the ``disk`` > element. > :since:`since after 0.4.4`; "sata" attribute value :since:`since 0.9.7`; > "removable" attribute value :since:`since 1.1.3`; > "rotation_rate" attribute value :since:`since 7.3.0` > +``throttlefilters`` > + The optional ``throttlefilters`` element provides the ability to provide > additional > + per-device throttle chain :since:`Since 10.3.0` > + For example, if we have four different disks and we want to limit I/O for > each one > + and we also want to limit combined I/O of all four disks, we can leverage > + ``throttlefilters`` to achieve this goal by setting two > ``throttlefilter`` for > + each disk: disk's own filter and combined filter. ``throttlefilters`` and > ``iotune`` > + should be used exclusively. Please also document how the group chaining is applied in terms of > + > + ``throttlefilter`` > + The optional ``throttlefilter`` element is to reference defined > throttle group. > ``iotune`` > The optional ``iotune`` element provides the ability to provide additional > per-device I/O tuning, with values that can vary for each device (contrast [...] > @@ -6637,6 +6641,164 @@ > > > > + > + > + > + > + > + > + > + > + > + > + > + Preferrably all these definitions should be shared with as any change would now need to change two places. It is okay if the schema is more lax (e.g. allowing 'group_name' to be optional in the schema, which is then strictly required by the parser), but it's not okay to have to change two distinct places. > + > + > + > + > + > +
[PATCH RFC v2 07/12] schema: Add new domain elements to support multiple throttle filters
From: Chun Feng Wu * Add new elements '' and '' * contains defintions * can include multiple throttlegroup references to form filter chain in qemu * Chained throttle filters feature in qemu is described at https://github.com/qemu/qemu/blob/master/docs/throttle.txt Signed-off-by: Chun Feng Wu --- docs/formatdomain.rst | 48 + src/conf/schemas/domaincommon.rng | 164 +- 2 files changed, 211 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index e2f66b982c..ee9ee8b10c 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1957,6 +1957,34 @@ advertisements to the guest OS. (NB: Only qemu driver support) the guest OS itself can choose to circumvent the unavailability of the sleep states (e.g. S4 by turning off completely). +Throttle Group Management +- + +:since:`Since 10.3.0` it is possible to create multiple named throttle groups +and then reference them within ``throttlefilters`` to form filter chain in QEMU for +specific disk. The limits(throttlegroups) are shared within domain, hence the same group +can be referenced by different filters. + +:: + + + ... + + + limit0 + 1000 + 40 + 10 + + + ... + + +All throttlegroups are listed within the ``throttlegroups`` element + +``throttlegroup`` + It has the same sub-elements as ``iotune`` (See `Hard drives, floppy disks, CDROMs`_), + The difference is that is required. Hypervisor features --- @@ -2704,6 +2732,15 @@ paravirtualized driver is specified via the ``disk`` element. + + + + + + + + + ... @@ -3185,6 +3222,17 @@ paravirtualized driver is specified via the ``disk`` element. :since:`since after 0.4.4`; "sata" attribute value :since:`since 0.9.7`; "removable" attribute value :since:`since 1.1.3`; "rotation_rate" attribute value :since:`since 7.3.0` +``throttlefilters`` + The optional ``throttlefilters`` element provides the ability to provide additional + per-device throttle chain :since:`Since 10.3.0` + For example, if we have four different disks and we want to limit I/O for each one + and we also want to limit combined I/O of all four disks, we can leverage + ``throttlefilters`` to achieve this goal by setting two ``throttlefilter`` for + each disk: disk's own filter and combined filter. ``throttlefilters`` and ``iotune`` + should be used exclusively. + + ``throttlefilter`` + The optional ``throttlefilter`` element is to reference defined throttle group. ``iotune`` The optional ``iotune`` element provides the ability to provide additional per-device I/O tuning, with values that can vary for each device (contrast diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index f386e46fae..e77149c6ca 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -51,6 +51,7 @@ + @@ -1577,7 +1578,10 @@ - + + + + @@ -6637,6 +6641,164 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
