From: Hyman Huang(黄勇) <huangy81@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(黄勇) <huangy81@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'/>
</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)) ||
+ 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;
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7a185061d8..18a72a79a8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6860,6 +6860,9 @@ qemuBuildCpuCommandLine(virCommand *cmd,
virBufferAddLit(&buf, ",kvm-pv-ipi=off");
break;
+ case VIR_DOMAIN_KVM_DIRTY_RING:
+ break;
+
case VIR_DOMAIN_KVM_LAST:
break;
}
@@ -7280,6 +7283,15 @@ qemuBuildAccelCommandLine(virCommand *cmd,
case VIR_DOMAIN_VIRT_KVM:
virBufferAddLit(&buf, "kvm");
+ /*
+ * only handle the kvm case, tcg case use the legacy style
+ * not that either kvm or tcg can be specified by libvirt
+ * so do not worry about the conflict of specifying both
+ * */
+ if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON &&
+ def->kvm_features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON) {
+ virBufferAsprintf(&buf, ",dirty-ring-size=%d", def->dirty_ring_size);
+ }
break;
case VIR_DOMAIN_VIRT_KQEMU:
--
2.27.0
On 11/23/21 15:36, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@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(黄勇) <huangy81@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
On 12/14/21 17:22, Michal Prívozník wrote:
> On 11/23/21 15:36, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@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(黄勇) <huangy81@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
>
Ok, i'll rebase the master once the changes get merged and test if the
dirty ring still works.
--
Best Regards
Hyman Huang(黄勇)
On 12/14/21 12:20, Hyman Huang wrote: > > Ok, i'll rebase the master once the changes get merged and test if the > dirty ring still works. > You can find all the patches applied in my branch: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/review/ Michal
On 12/14/21 10:22, Michal Prívozník wrote: > On 11/23/21 15:36, huangy81@chinatelecom.cn wrote: >> From: Hyman Huang(黄勇) <huangy81@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(黄勇) <huangy81@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(-) >> > 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). Merged now. Congratulations on your first libvirt contribution! Michal
On 12/14/21 20:21, Michal Prívozník wrote: > On 12/14/21 10:22, Michal Prívozník wrote: >> On 11/23/21 15:36, huangy81@chinatelecom.cn wrote: >>> From: Hyman Huang(黄勇) <huangy81@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(黄勇) <huangy81@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(-) >>> > > >> 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). > > Merged now. Congratulations on your first libvirt contribution! > > Michal > Thanks :) -- Best Regards Hyman Huang(黄勇)
© 2016 - 2026 Red Hat, Inc.