On 11/23/21 15:36, huang...@chinatelecom.cn wrote:
From: Hyman Huang(黄勇) <huang...@chinatelecom.cn>
Dirty ring feature was introduced in qemu-6.1.0, this patch
add the corresponding feature named 'dirty-ring', which enable
dirty ring feature when starting vm.
To implement the dirty-ring feature, dirty_ring_size in struct
"_virDomainDef" is introduced to hold the dirty ring size
configured in xml, and it will be used as dirty-ring-size
property of kvm accelerator when building qemu commandline,
it is something like "-accel dirty-ring-size=xxx".
To enable the feature, the following XML needs to be added to
the guest's domain description:
<features>
<kvm>
<dirty-ring state='on' size='xxx'>
</kvm>
</features>
If property "state=on", property "size" must be specified, which
should be power of 2 and range in [1024, 65526].
Signed-off-by: Hyman Huang(黄勇) <huang...@chinatelecom.cn>
---
docs/formatdomain.rst | 18 ++++++------
docs/schemas/domaincommon.rng | 10 +++++++
src/conf/domain_conf.c | 54 +++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 4 +++
src/qemu/qemu_command.c | 12 ++++++++
5 files changed, 90 insertions(+), 8 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index eb8c973cf1..ea69b61c70 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1843,6 +1843,7 @@ Hypervisors may allow certain CPU / machine features to
be toggled on/off.
<hint-dedicated state='on'/>
<poll-control state='on'/>
<pv-ipi state='off'/>
+ <dirty-ring state='on' size='4096'/>
I was confused at first, what units is @size in but looking into the
qemu docs it's unit-less number:
"[dirty-ring-size] it controls the size of the per-vCPU dirty page
ring buffer (number of entries for each vCPU)."
Therefore I'm okay with having it as a plain attribute. Otherwise for
values with units (traditionally size units like KiB/MiB/...) I would
advise to go with an extra element.
</kvm>
<xen>
<e820_host state='on'/>
@@ -1925,14 +1926,15 @@ are:
``kvm``
Various features to change the behavior of the KVM hypervisor.
- ============== ============================================================================ ======= ============================
- Feature Description
Value Since
- ==============
============================================================================
======= ============================
- hidden Hide the KVM hypervisor from standard MSR based discovery
on, off :since:`1.2.8 (QEMU 2.1.0)`
- hint-dedicated Allows a guest to enable optimizations when running on
dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)`
- poll-control Decrease IO completion latency by introducing a grace period
of busy waiting on, off :since:`6.10.0 (QEMU 4.2)`
- pv-ipi Paravirtualized send IPIs
on, off :since:`7.10.0 (QEMU 3.1)`
- ==============
============================================================================
======= ============================
+ ==============
============================================================================
======================================================
============================
+ Feature Description
Value Since
+ ==============
============================================================================
======================================================
============================
+ hidden Hide the KVM hypervisor from standard MSR based discovery
on, off
:since:`1.2.8 (QEMU 2.1.0)`
+ hint-dedicated Allows a guest to enable optimizations when running on
dedicated vCPUs on, off
:since:`5.7.0 (QEMU 2.12.0)`
+ poll-control Decrease IO completion latency by introducing a grace period
of busy waiting on, off
:since:`6.10.0 (QEMU 4.2)`
+ pv-ipi Paravirtualized send IPIs
on, off
:since:`7.10.0 (QEMU 3.1)`
+ dirty-ring Enable dirty ring feature
on, off; size - must be power of 2, range [1024,65536]
:since:`7.10.0 (QEMU 6.1)`
+ ==============
============================================================================
======================================================
============================
``xen``
Various features to change the behavior of the Xen hypervisor.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f01b7a6470..5f9fe3cc58 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -7212,6 +7212,16 @@
<ref name="featurestate"/>
</element>
</optional>
+ <optional>
+ <element name="dirty-ring">
+ <ref name="featurestate"/>
+ <optional>
+ <attribute name="size">
+ <data type="unsignedInt"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 552d43b845..6f3c925b55 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -205,6 +205,7 @@ VIR_ENUM_IMPL(virDomainKVM,
"hint-dedicated",
"poll-control",
"pv-ipi",
+ "dirty-ring",
);
VIR_ENUM_IMPL(virDomainXen,
@@ -17589,6 +17590,25 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
def->kvm_features[feature] = value;
+ /* dirty ring feature should parse size property */
+ if (((virDomainKVM) feature == VIR_DOMAIN_KVM_DIRTY_RING) &&
+ (value == VIR_TRISTATE_SWITCH_ON)) {
+
+ if (virXMLPropUInt(node, "size", 0, VIR_XML_PROP_REQUIRED,
+ &def->dirty_ring_size) < 0) {
+ return -1;
+ }
+
+ if ((def->dirty_ring_size !=
VIR_ROUND_UP_POWER_OF_TWO(def->dirty_ring_size)) ||
VIR_IS_POW2() which works even for other types than uint.
+ def->dirty_ring_size < 1024 ||
+ def->dirty_ring_size > 65536) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("dirty ring must be power of 2 and ranges [1024,
65536]"));
+
+ return -1;
+ }
+ }
+
node = xmlNextElementSibling(node);
}
@@ -21824,7 +21844,27 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src,
virTristateSwitchTypeToString(dst->kvm_features[i]));
return false;
}
+ break;
+ case VIR_DOMAIN_KVM_DIRTY_RING:
+ if (src->kvm_features[i] != dst->kvm_features[i]) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("State of KVM feature '%s' differs: "
+ "source: '%s', destination: '%s'"),
+ virDomainKVMTypeToString(i),
+
virTristateSwitchTypeToString(src->kvm_features[i]),
+
virTristateSwitchTypeToString(dst->kvm_features[i]));
+ return false;
+ }
+
+ if (src->dirty_ring_size != dst->dirty_ring_size) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("dirty ring size of KVM feature '%s' differs:
"
+ "source: '%d', destination: '%d'"),
+ virDomainKVMTypeToString(i),
+ src->dirty_ring_size, dst->dirty_ring_size);
+ return false;
+ }
break;
case VIR_DOMAIN_KVM_LAST:
@@ -27872,6 +27912,20 @@ virDomainDefFormatFeatures(virBuffer *buf,
def->kvm_features[j]));
break;
+ case VIR_DOMAIN_KVM_DIRTY_RING:
+ if (def->kvm_features[j] != VIR_TRISTATE_SWITCH_ABSENT) {
+ virBufferAsprintf(&childBuf, "<%s state='%s'",
+ virDomainKVMTypeToString(j),
+
virTristateSwitchTypeToString(def->kvm_features[j]));
+ if (def->dirty_ring_size) {
+ virBufferAsprintf(&childBuf, " size='%d'/>\n",
+ def->dirty_ring_size);
+ } else {
+ virBufferAddLit(&childBuf, "/>\n");
+ }
+ }
+ break;
+
case VIR_DOMAIN_KVM_LAST:
break;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8634960313..026edde88f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2084,6 +2084,7 @@ typedef enum {
VIR_DOMAIN_KVM_DEDICATED,
VIR_DOMAIN_KVM_POLLCONTROL,
VIR_DOMAIN_KVM_PVIPI,
+ VIR_DOMAIN_KVM_DIRTY_RING,
VIR_DOMAIN_KVM_LAST
} virDomainKVM;
@@ -2933,6 +2934,9 @@ struct _virDomainDef {
should be re-run before starting */
unsigned int scsiBusMaxUnit;
+
+ /* size of dirty ring for each vcpu */
+ unsigned int dirty_ring_size;
This feels weird. I haven't followed previous versions that closely, but
what I did for TCG features is introducing a struct that lives close to
other features. This could then be something like:
typedef struct _virDomainFeatureKVM virDomainFeatureKVM;
struct _virDomainFeatureKVM {
int features[VIR_DOMAIN_KVM_LAST];
unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no
units */
};
struct _virDomainDef {
...
- int kvm_features[VIR_DOMAIN_KVM_LAST];
+ virDomainFeatureKVM *kvm_features;
...
};
So here's what I suggest doing - let me post a patch that changes 'int
kvm_features' into a separate struct. I would squash it into yours but
it turned out to be quite lengthy change. Then I'll do changes necessary
for your patch (which will be trivial after that).
Michal
