[PATCH v6 3/4] conf: introduce dirty_ring_size field

[Hyman]

在 2021/11/22 16:50, Peter Krempa 写道:
> On Sat, Nov 20, 2021 at 03:20:47 -0500, huangy81 at chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81 at chinatelecom.cn>
>>
>> introduce dirty_ring_size in struct "_virDomainDef" to hold
>> the ring size configured by user, and pass dirty_ring_size
>> when building qemu commandline if dirty ring feature enabled.
>>
>> Signed-off-by: Hyman Huang(黄勇) <huangy81 at chinatelecom.cn>
>> ---
>>   src/conf/domain_conf.c  | 76 ++++++++++++++++++++++++++++++++++++++++-
>>   src/conf/domain_conf.h  |  4 +++
>>   src/qemu/qemu_command.c |  3 ++
>>   3 files changed, 82 insertions(+), 1 deletion(-)
> 
> [...]
> 
> 
> 
>> @@ -4826,6 +4827,18 @@ virDomainDefPostParseMemtune(virDomainDef *def)
>>   }
>>   
>>   
>> +static void
>> +virDomainDefPostParseFeatures(virDomainDef *def)
>> +{
>> +    if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON &&
>> +        def->kvm_features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON &&
>> +        def->dirty_ring_size == 0) {
>> +        /* set 4096 as default size if dirty ring size not congfigured */
> 
> Instead of this quite obvious thing I'd prefer a justification of why
> this setting is done in the global post-parse function rather than the
> qemu specific one. If there is no good reason to have it globally,
> please move it to the qemu-specific one.
Ok, i'll move it into virDomainFeaturesKVMDefParse and parse it once for 
all
> 
>> +        def->dirty_ring_size = 4096;
>> +    }
>> +}
> 
> [...]
> 
>> @@ -17566,8 +17581,10 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
>>   
>>   static int
>>   virDomainFeaturesKVMDefParse(virDomainDef *def,
>> +                            xmlXPathContextPtr ctxt,
>>                                xmlNodePtr node)
>>   {
>> +    xmlNodePtr tmp_node = ctxt->node;
> 
> We have an established approach for resetting the XPath context. Please
> use it, as ...[1].
Ok
> 
>>       def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
>>   
>>       node = xmlFirstElementChild(node);
>> @@ -17589,9 +17606,37 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
>>   
>>           def->kvm_features[feature] = value;
>>   
>> +        /* dirty ring feature should parse size property */
>> +        if ((virDomainKVM) feature == VIR_DOMAIN_KVM_DIRTY_RING) {
>> +            if (((virDomainKVM) feature) == VIR_DOMAIN_KVM_DIRTY_RING &&
>> +                  value == VIR_TRISTATE_SWITCH_ON) {
>> +                ctxt->node = node;
>> +
>> +                if (virXMLPropString(node, "size")) {
> 
> virXMLPropString returns an allocated string, so this is leaking memory