[libvirt PATCH 00/16] Refactor virDomainFeaturesDefParse
Some refactoring in preparation for adding support for qemu's "hv-passthrough" and the yet-to-be-merged "hv-defaults". Tim Wiederhake (16): virDomainFeaturesDefParse: Factor out HyperV parsing into separate function virDomainFeaturesHyperVDefParse: Inline hyperv/stimer parsing virDomainFeaturesHyperVDefParse: Remove ctxt virDomainFeaturesHyperVDefParse: Remove tautological "if" virDomainFeaturesDefParse: Factor out KVM parsing into separate function virDomainFeaturesKVMDefParse: Remove ctxt virDomainFeaturesKVMDefParse: Remove tautological "switch" virDomainFeaturesKVMDefParse: Remove tautological "if" virDomainFeaturesDefParse: Factor out XEN parsing into separate function virDomainFeaturesXENDefParse: Remove ctxt virDomainFeaturesXENDefParse: Remove tautological "if" virDomainFeaturesDefParse: Inline SMM parsing virDomainFeaturesDefParse: Inline MSRS parsing virDomainFeaturesDefParse: Factor out capabilities parsing into separate function virDomainFeaturesCapabilitiesDefParse: Remove ctxt virDomainFeaturesDefParse: Simplify APIC parsing src/conf/domain_conf.c | 557 ++++++++++++++++++++++------------------- 1 file changed, 296 insertions(+), 261 deletions(-) -- 2.31.1
Only moving code, cleanup to follow. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 236 ++++++++++++++++++++++------------------- 1 file changed, 127 insertions(+), 109 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f65509d8ec..8cf57db7ba 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17286,6 +17286,128 @@ virDomainResourceDefParse(xmlNodePtr node, } +static int +virDomainFeaturesHyperVDefParse(virDomainDef *def, + xmlXPathContext *ctxt) +{ + g_autofree xmlNodePtr *nodes = NULL; + size_t i; + int n; + + def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON; + + if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { + int feature; + virTristateSwitch value; + if ((n = virXPathNodeSet("./features/hyperv/*", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + feature = virDomainHypervTypeFromString((const char *)nodes[i]->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported HyperV Enlightenment feature: %s"), + nodes[i]->name); + return -1; + } + + if (virXMLPropTristateSwitch(nodes[i], "state", + VIR_XML_PROP_REQUIRED, &value) < 0) + return -1; + + def->hyperv_features[feature] = value; + + switch ((virDomainHyperv) feature) { + case VIR_DOMAIN_HYPERV_RELAXED: + case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: + case VIR_DOMAIN_HYPERV_RESET: + case VIR_DOMAIN_HYPERV_FREQUENCIES: + case VIR_DOMAIN_HYPERV_REENLIGHTENMENT: + case VIR_DOMAIN_HYPERV_TLBFLUSH: + case VIR_DOMAIN_HYPERV_IPI: + case VIR_DOMAIN_HYPERV_EVMCS: + break; + + case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (value != VIR_TRISTATE_SWITCH_ON) + break; + + if (virXMLPropUInt(nodes[i], "retries", 0, + VIR_XML_PROP_REQUIRED, + &def->hyperv_spinlocks) < 0) + return -1; + + if (def->hyperv_spinlocks < 0xFFF) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("HyperV spinlock retry count must be " + "at least 4095")); + return -1; + } + break; + + case VIR_DOMAIN_HYPERV_VENDOR_ID: + if (value != VIR_TRISTATE_SWITCH_ON) + break; + + if (!(def->hyperv_vendor_id = virXMLPropString(nodes[i], + "value"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'value' attribute for " + "HyperV feature 'vendor_id'")); + return -1; + } + + if (strlen(def->hyperv_vendor_id) > VIR_DOMAIN_HYPERV_VENDOR_ID_MAX) { + virReportError(VIR_ERR_XML_ERROR, + _("HyperV vendor_id value must not be more " + "than %d characters."), + VIR_DOMAIN_HYPERV_VENDOR_ID_MAX); + return -1; + } + + /* ensure that the string can be passed to qemu */ + if (strchr(def->hyperv_vendor_id, ',')) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("HyperV vendor_id value is invalid")); + return -1; + } + break; + + case VIR_DOMAIN_HYPERV_LAST: + break; + } + } + VIR_FREE(nodes); + } + + if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { + if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + if (STRNEQ((const char *)nodes[i]->name, "direct")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Hyper-V stimer feature: %s"), + nodes[i]->name); + return -1; + } + + if (virXMLPropTristateSwitch(nodes[i], "state", + VIR_XML_PROP_REQUIRED, + &def->hyperv_stimer_direct) < 0) + return -1; + } + VIR_FREE(nodes); + } + + return 0; +} + + static int virDomainFeaturesDefParse(virDomainDef *def, xmlXPathContextPtr ctxt) @@ -17323,13 +17445,17 @@ virDomainFeaturesDefParse(virDomainDef *def, case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: - case VIR_DOMAIN_FEATURE_HYPERV: case VIR_DOMAIN_FEATURE_KVM: case VIR_DOMAIN_FEATURE_MSRS: case VIR_DOMAIN_FEATURE_XEN: def->features[val] = VIR_TRISTATE_SWITCH_ON; break; + case VIR_DOMAIN_FEATURE_HYPERV: + if (virDomainFeaturesHyperVDefParse(def, ctxt) < 0) + return -1; + break; + case VIR_DOMAIN_FEATURE_CAPABILITIES: { virDomainCapabilitiesPolicy policy; @@ -17463,114 +17589,6 @@ virDomainFeaturesDefParse(virDomainDef *def, } VIR_FREE(nodes); - if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { - int feature; - virTristateSwitch value; - if ((n = virXPathNodeSet("./features/hyperv/*", ctxt, &nodes)) < 0) - return -1; - - for (i = 0; i < n; i++) { - feature = virDomainHypervTypeFromString((const char *)nodes[i]->name); - if (feature < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported HyperV Enlightenment feature: %s"), - nodes[i]->name); - return -1; - } - - if (virXMLPropTristateSwitch(nodes[i], "state", - VIR_XML_PROP_REQUIRED, &value) < 0) - return -1; - - def->hyperv_features[feature] = value; - - switch ((virDomainHyperv) feature) { - case VIR_DOMAIN_HYPERV_RELAXED: - case VIR_DOMAIN_HYPERV_VAPIC: - case VIR_DOMAIN_HYPERV_VPINDEX: - case VIR_DOMAIN_HYPERV_RUNTIME: - case VIR_DOMAIN_HYPERV_SYNIC: - case VIR_DOMAIN_HYPERV_STIMER: - case VIR_DOMAIN_HYPERV_RESET: - case VIR_DOMAIN_HYPERV_FREQUENCIES: - case VIR_DOMAIN_HYPERV_REENLIGHTENMENT: - case VIR_DOMAIN_HYPERV_TLBFLUSH: - case VIR_DOMAIN_HYPERV_IPI: - case VIR_DOMAIN_HYPERV_EVMCS: - break; - - case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (value != VIR_TRISTATE_SWITCH_ON) - break; - - if (virXMLPropUInt(nodes[i], "retries", 0, - VIR_XML_PROP_REQUIRED, - &def->hyperv_spinlocks) < 0) - return -1; - - if (def->hyperv_spinlocks < 0xFFF) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("HyperV spinlock retry count must be " - "at least 4095")); - return -1; - } - break; - - case VIR_DOMAIN_HYPERV_VENDOR_ID: - if (value != VIR_TRISTATE_SWITCH_ON) - break; - - if (!(def->hyperv_vendor_id = virXMLPropString(nodes[i], - "value"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing 'value' attribute for " - "HyperV feature 'vendor_id'")); - return -1; - } - - if (strlen(def->hyperv_vendor_id) > VIR_DOMAIN_HYPERV_VENDOR_ID_MAX) { - virReportError(VIR_ERR_XML_ERROR, - _("HyperV vendor_id value must not be more " - "than %d characters."), - VIR_DOMAIN_HYPERV_VENDOR_ID_MAX); - return -1; - } - - /* ensure that the string can be passed to qemu */ - if (strchr(def->hyperv_vendor_id, ',')) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("HyperV vendor_id value is invalid")); - return -1; - } - break; - - case VIR_DOMAIN_HYPERV_LAST: - break; - } - } - VIR_FREE(nodes); - } - - if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { - if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes)) < 0) - return -1; - - for (i = 0; i < n; i++) { - if (STRNEQ((const char *)nodes[i]->name, "direct")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported Hyper-V stimer feature: %s"), - nodes[i]->name); - return -1; - } - - if (virXMLPropTristateSwitch(nodes[i], "state", - VIR_XML_PROP_REQUIRED, - &def->hyperv_stimer_direct) < 0) - return -1; - } - VIR_FREE(nodes); - } - if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { int feature; virTristateSwitch value; -- 2.31.1
Iterating over all child elements of a node does not require xpath. By doing away with xpath for this code, the code can be inlined and simplified. This also removes the re-use of `nodes`, elimininating two VIR_FREEs. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 46 ++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8cf57db7ba..4ec5557eba 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17303,6 +17303,8 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, return -1; for (i = 0; i < n; i++) { + xmlNodePtr child; + feature = virDomainHypervTypeFromString((const char *)nodes[i]->name); if (feature < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -17323,7 +17325,6 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, case VIR_DOMAIN_HYPERV_VPINDEX: case VIR_DOMAIN_HYPERV_RUNTIME: case VIR_DOMAIN_HYPERV_SYNIC: - case VIR_DOMAIN_HYPERV_STIMER: case VIR_DOMAIN_HYPERV_RESET: case VIR_DOMAIN_HYPERV_FREQUENCIES: case VIR_DOMAIN_HYPERV_REENLIGHTENMENT: @@ -17332,6 +17333,28 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, case VIR_DOMAIN_HYPERV_EVMCS: break; + case VIR_DOMAIN_HYPERV_STIMER: + if (value != VIR_TRISTATE_SWITCH_ON) + break; + + child = xmlFirstElementChild(nodes[i]); + while (child) { + if (STRNEQ((const char *)child->name, "direct")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Hyper-V stimer feature: %s"), + child->name); + return -1; + } + + if (virXMLPropTristateSwitch(child, "state", + VIR_XML_PROP_REQUIRED, + &def->hyperv_stimer_direct) < 0) + return -1; + + child = xmlNextElementSibling(child); + } + break; + case VIR_DOMAIN_HYPERV_SPINLOCKS: if (value != VIR_TRISTATE_SWITCH_ON) break; @@ -17381,27 +17404,6 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, break; } } - VIR_FREE(nodes); - } - - if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { - if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes)) < 0) - return -1; - - for (i = 0; i < n; i++) { - if (STRNEQ((const char *)nodes[i]->name, "direct")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported Hyper-V stimer feature: %s"), - nodes[i]->name); - return -1; - } - - if (virXMLPropTristateSwitch(nodes[i], "state", - VIR_XML_PROP_REQUIRED, - &def->hyperv_stimer_direct) < 0) - return -1; - } - VIR_FREE(nodes); } return 0; -- 2.31.1
Iterating over all child elements of a node does not require xpath. By doing away with xpath for this code, the code can be simplified. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4ec5557eba..b778dfe463 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17288,32 +17288,27 @@ virDomainResourceDefParse(xmlNodePtr node, static int virDomainFeaturesHyperVDefParse(virDomainDef *def, - xmlXPathContext *ctxt) + xmlNodePtr node) { - g_autofree xmlNodePtr *nodes = NULL; - size_t i; - int n; - def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON; if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { int feature; virTristateSwitch value; - if ((n = virXPathNodeSet("./features/hyperv/*", ctxt, &nodes)) < 0) - return -1; - for (i = 0; i < n; i++) { + node = xmlFirstElementChild(node); + while (node != NULL) { xmlNodePtr child; - feature = virDomainHypervTypeFromString((const char *)nodes[i]->name); + feature = virDomainHypervTypeFromString((const char *)node->name); if (feature < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported HyperV Enlightenment feature: %s"), - nodes[i]->name); + node->name); return -1; } - if (virXMLPropTristateSwitch(nodes[i], "state", + if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED, &value) < 0) return -1; @@ -17337,7 +17332,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, if (value != VIR_TRISTATE_SWITCH_ON) break; - child = xmlFirstElementChild(nodes[i]); + child = xmlFirstElementChild(node); while (child) { if (STRNEQ((const char *)child->name, "direct")) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -17359,7 +17354,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, if (value != VIR_TRISTATE_SWITCH_ON) break; - if (virXMLPropUInt(nodes[i], "retries", 0, + if (virXMLPropUInt(node, "retries", 0, VIR_XML_PROP_REQUIRED, &def->hyperv_spinlocks) < 0) return -1; @@ -17376,7 +17371,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, if (value != VIR_TRISTATE_SWITCH_ON) break; - if (!(def->hyperv_vendor_id = virXMLPropString(nodes[i], + if (!(def->hyperv_vendor_id = virXMLPropString(node, "value"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing 'value' attribute for " @@ -17403,6 +17398,8 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, case VIR_DOMAIN_HYPERV_LAST: break; } + + node = xmlNextElementSibling(node); } } @@ -17454,7 +17451,7 @@ virDomainFeaturesDefParse(virDomainDef *def, break; case VIR_DOMAIN_FEATURE_HYPERV: - if (virDomainFeaturesHyperVDefParse(def, ctxt) < 0) + if (virDomainFeaturesHyperVDefParse(def, nodes[i]) < 0) return -1; break; -- 2.31.1
Fix some line wrapping in the process. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 163 +++++++++++++++++++---------------------- 1 file changed, 77 insertions(+), 86 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b778dfe463..3ba41869ec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17292,115 +17292,106 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, { def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON; - if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { + node = xmlFirstElementChild(node); + while (node != NULL) { int feature; virTristateSwitch value; + xmlNodePtr child; - node = xmlFirstElementChild(node); - while (node != NULL) { - xmlNodePtr child; + feature = virDomainHypervTypeFromString((const char *)node->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported HyperV Enlightenment feature: %s"), + node->name); + return -1; + } - feature = virDomainHypervTypeFromString((const char *)node->name); - if (feature < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported HyperV Enlightenment feature: %s"), - node->name); - return -1; - } + if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED, + &value) < 0) + return -1; - if (virXMLPropTristateSwitch(node, "state", - VIR_XML_PROP_REQUIRED, &value) < 0) - return -1; + def->hyperv_features[feature] = value; - def->hyperv_features[feature] = value; + switch ((virDomainHyperv) feature) { + case VIR_DOMAIN_HYPERV_RELAXED: + case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_RESET: + case VIR_DOMAIN_HYPERV_FREQUENCIES: + case VIR_DOMAIN_HYPERV_REENLIGHTENMENT: + case VIR_DOMAIN_HYPERV_TLBFLUSH: + case VIR_DOMAIN_HYPERV_IPI: + case VIR_DOMAIN_HYPERV_EVMCS: + break; - switch ((virDomainHyperv) feature) { - case VIR_DOMAIN_HYPERV_RELAXED: - case VIR_DOMAIN_HYPERV_VAPIC: - case VIR_DOMAIN_HYPERV_VPINDEX: - case VIR_DOMAIN_HYPERV_RUNTIME: - case VIR_DOMAIN_HYPERV_SYNIC: - case VIR_DOMAIN_HYPERV_RESET: - case VIR_DOMAIN_HYPERV_FREQUENCIES: - case VIR_DOMAIN_HYPERV_REENLIGHTENMENT: - case VIR_DOMAIN_HYPERV_TLBFLUSH: - case VIR_DOMAIN_HYPERV_IPI: - case VIR_DOMAIN_HYPERV_EVMCS: + case VIR_DOMAIN_HYPERV_STIMER: + if (value != VIR_TRISTATE_SWITCH_ON) break; - case VIR_DOMAIN_HYPERV_STIMER: - if (value != VIR_TRISTATE_SWITCH_ON) - break; + child = xmlFirstElementChild(node); + while (child) { + if (STRNEQ((const char *)child->name, "direct")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Hyper-V stimer feature: %s"), + child->name); + return -1; + } - child = xmlFirstElementChild(node); - while (child) { - if (STRNEQ((const char *)child->name, "direct")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported Hyper-V stimer feature: %s"), - child->name); - return -1; - } + if (virXMLPropTristateSwitch(child, "state", VIR_XML_PROP_REQUIRED, + &def->hyperv_stimer_direct) < 0) + return -1; - if (virXMLPropTristateSwitch(child, "state", - VIR_XML_PROP_REQUIRED, - &def->hyperv_stimer_direct) < 0) - return -1; + child = xmlNextElementSibling(child); + } + break; - child = xmlNextElementSibling(child); - } + case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (value != VIR_TRISTATE_SWITCH_ON) break; - case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (value != VIR_TRISTATE_SWITCH_ON) - break; + if (virXMLPropUInt(node, "retries", 0, VIR_XML_PROP_REQUIRED, + &def->hyperv_spinlocks) < 0) + return -1; - if (virXMLPropUInt(node, "retries", 0, - VIR_XML_PROP_REQUIRED, - &def->hyperv_spinlocks) < 0) - return -1; + if (def->hyperv_spinlocks < 0xFFF) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("HyperV spinlock retry count must be at least 4095")); + return -1; + } + break; - if (def->hyperv_spinlocks < 0xFFF) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("HyperV spinlock retry count must be " - "at least 4095")); - return -1; - } + case VIR_DOMAIN_HYPERV_VENDOR_ID: + if (value != VIR_TRISTATE_SWITCH_ON) break; - case VIR_DOMAIN_HYPERV_VENDOR_ID: - if (value != VIR_TRISTATE_SWITCH_ON) - break; - - if (!(def->hyperv_vendor_id = virXMLPropString(node, - "value"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing 'value' attribute for " - "HyperV feature 'vendor_id'")); - return -1; - } - - if (strlen(def->hyperv_vendor_id) > VIR_DOMAIN_HYPERV_VENDOR_ID_MAX) { - virReportError(VIR_ERR_XML_ERROR, - _("HyperV vendor_id value must not be more " - "than %d characters."), - VIR_DOMAIN_HYPERV_VENDOR_ID_MAX); - return -1; - } + if (!(def->hyperv_vendor_id = virXMLPropString(node, "value"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'value' attribute for HyperV feature 'vendor_id'")); + return -1; + } - /* ensure that the string can be passed to qemu */ - if (strchr(def->hyperv_vendor_id, ',')) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("HyperV vendor_id value is invalid")); - return -1; - } - break; + if (strlen(def->hyperv_vendor_id) > VIR_DOMAIN_HYPERV_VENDOR_ID_MAX) { + virReportError(VIR_ERR_XML_ERROR, + _("HyperV vendor_id value must not be more than %d characters."), + VIR_DOMAIN_HYPERV_VENDOR_ID_MAX); + return -1; + } - case VIR_DOMAIN_HYPERV_LAST: - break; + /* ensure that the string can be passed to qemu */ + if (strchr(def->hyperv_vendor_id, ',')) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("HyperV vendor_id value is invalid")); + return -1; } + break; - node = xmlNextElementSibling(node); + case VIR_DOMAIN_HYPERV_LAST: + break; } + + node = xmlNextElementSibling(node); } return 0; -- 2.31.1
Only moving code, cleanup to follow. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 88 +++++++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 35 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3ba41869ec..384740c364 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17398,6 +17398,54 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, } +static int +virDomainFeaturesKVMDefParse(virDomainDef *def, + xmlXPathContext *ctxt) +{ + g_autofree xmlNodePtr *nodes = NULL; + size_t i; + int n; + + def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; + + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { + int feature; + virTristateSwitch value; + if ((n = virXPathNodeSet("./features/kvm/*", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + feature = virDomainKVMTypeFromString((const char *)nodes[i]->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported KVM feature: %s"), + nodes[i]->name); + return -1; + } + + switch ((virDomainKVM) feature) { + case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: + if (virXMLPropTristateSwitch(nodes[i], "state", + VIR_XML_PROP_REQUIRED, + &value) < 0) + return -1; + + def->kvm_features[feature] = value; + break; + + case VIR_DOMAIN_KVM_LAST: + break; + } + } + VIR_FREE(nodes); + } + + return 0; +} + + static int virDomainFeaturesDefParse(virDomainDef *def, xmlXPathContextPtr ctxt) @@ -17435,7 +17483,6 @@ virDomainFeaturesDefParse(virDomainDef *def, case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: - case VIR_DOMAIN_FEATURE_KVM: case VIR_DOMAIN_FEATURE_MSRS: case VIR_DOMAIN_FEATURE_XEN: def->features[val] = VIR_TRISTATE_SWITCH_ON; @@ -17446,6 +17493,11 @@ virDomainFeaturesDefParse(virDomainDef *def, return -1; break; + case VIR_DOMAIN_FEATURE_KVM: + if (virDomainFeaturesKVMDefParse(def, ctxt) < 0) + return -1; + break; + case VIR_DOMAIN_FEATURE_CAPABILITIES: { virDomainCapabilitiesPolicy policy; @@ -17579,40 +17631,6 @@ virDomainFeaturesDefParse(virDomainDef *def, } VIR_FREE(nodes); - if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { - int feature; - virTristateSwitch value; - if ((n = virXPathNodeSet("./features/kvm/*", ctxt, &nodes)) < 0) - return -1; - - for (i = 0; i < n; i++) { - feature = virDomainKVMTypeFromString((const char *)nodes[i]->name); - if (feature < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported KVM feature: %s"), - nodes[i]->name); - return -1; - } - - switch ((virDomainKVM) feature) { - case VIR_DOMAIN_KVM_HIDDEN: - case VIR_DOMAIN_KVM_DEDICATED: - case VIR_DOMAIN_KVM_POLLCONTROL: - if (virXMLPropTristateSwitch(nodes[i], "state", - VIR_XML_PROP_REQUIRED, - &value) < 0) - return -1; - - def->kvm_features[feature] = value; - break; - - case VIR_DOMAIN_KVM_LAST: - break; - } - } - VIR_FREE(nodes); - } - if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { int feature; virTristateSwitch value; -- 2.31.1
Iterating over all child elements of a node does not require xpath. By doing away with xpath for this code, the code can be simplified. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 384740c364..2ad4cbc5a3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17400,26 +17400,21 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def, static int virDomainFeaturesKVMDefParse(virDomainDef *def, - xmlXPathContext *ctxt) + xmlNodePtr node) { - g_autofree xmlNodePtr *nodes = NULL; - size_t i; - int n; - def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { int feature; virTristateSwitch value; - if ((n = virXPathNodeSet("./features/kvm/*", ctxt, &nodes)) < 0) - return -1; - for (i = 0; i < n; i++) { - feature = virDomainKVMTypeFromString((const char *)nodes[i]->name); + node = xmlFirstElementChild(node); + while (node) { + feature = virDomainKVMTypeFromString((const char *)node->name); if (feature < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported KVM feature: %s"), - nodes[i]->name); + node->name); return -1; } @@ -17427,7 +17422,7 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: case VIR_DOMAIN_KVM_POLLCONTROL: - if (virXMLPropTristateSwitch(nodes[i], "state", + if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED, &value) < 0) return -1; @@ -17438,8 +17433,9 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, case VIR_DOMAIN_KVM_LAST: break; } + + node = xmlNextElementSibling(node); } - VIR_FREE(nodes); } return 0; @@ -17494,7 +17490,7 @@ virDomainFeaturesDefParse(virDomainDef *def, break; case VIR_DOMAIN_FEATURE_KVM: - if (virDomainFeaturesKVMDefParse(def, ctxt) < 0) + if (virDomainFeaturesKVMDefParse(def, nodes[i]) < 0) return -1; break; -- 2.31.1
`feature` is always one of the values listed in the switch, ensured by `virDomainKVMTypeFromString` above. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2ad4cbc5a3..62565601ab 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17418,21 +17418,11 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, return -1; } - switch ((virDomainKVM) feature) { - case VIR_DOMAIN_KVM_HIDDEN: - case VIR_DOMAIN_KVM_DEDICATED: - case VIR_DOMAIN_KVM_POLLCONTROL: - if (virXMLPropTristateSwitch(node, "state", - VIR_XML_PROP_REQUIRED, - &value) < 0) - return -1; - - def->kvm_features[feature] = value; - break; + if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED, + &value) < 0) + return -1; - case VIR_DOMAIN_KVM_LAST: - break; - } + def->kvm_features[feature] = value; node = xmlNextElementSibling(node); } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 62565601ab..24529f3093 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17404,28 +17404,26 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, { def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; - if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { + node = xmlFirstElementChild(node); + while (node) { int feature; virTristateSwitch value; - node = xmlFirstElementChild(node); - while (node) { - feature = virDomainKVMTypeFromString((const char *)node->name); - if (feature < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported KVM feature: %s"), - node->name); - return -1; - } + feature = virDomainKVMTypeFromString((const char *)node->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported KVM feature: %s"), + node->name); + return -1; + } - if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED, - &value) < 0) - return -1; + if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED, + &value) < 0) + return -1; - def->kvm_features[feature] = value; + def->kvm_features[feature] = value; - node = xmlNextElementSibling(node); - } + node = xmlNextElementSibling(node); } return 0; -- 2.31.1
Only moving code, cleanup to follow. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 108 ++++++++++++++++++++++++----------------- 1 file changed, 63 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 24529f3093..e687f18afe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17430,6 +17430,64 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, } +static int +virDomainFeaturesXENDefParse(virDomainDef *def, + xmlXPathContext *ctxt) +{ + g_autofree xmlNodePtr *nodes = NULL; + size_t i; + int n; + + def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON; + + if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { + int feature; + virTristateSwitch value; + + if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + feature = virDomainXenTypeFromString((const char *)nodes[i]->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Xen feature: %s"), + nodes[i]->name); + return -1; + } + + if (virXMLPropTristateSwitch(nodes[i], "state", + VIR_XML_PROP_REQUIRED, &value) < 0) + return -1; + + def->xen_features[feature] = value; + + switch ((virDomainXen) feature) { + case VIR_DOMAIN_XEN_E820_HOST: + break; + + case VIR_DOMAIN_XEN_PASSTHROUGH: + if (value != VIR_TRISTATE_SWITCH_ON) + break; + + if (virXMLPropEnum(nodes[i], "mode", + virDomainXenPassthroughModeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->xen_passthrough_mode) < 0) + return -1; + break; + + case VIR_DOMAIN_XEN_LAST: + break; + } + } + VIR_FREE(nodes); + } + + return 0; +} + + static int virDomainFeaturesDefParse(virDomainDef *def, xmlXPathContextPtr ctxt) @@ -17468,7 +17526,6 @@ virDomainFeaturesDefParse(virDomainDef *def, case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: case VIR_DOMAIN_FEATURE_MSRS: - case VIR_DOMAIN_FEATURE_XEN: def->features[val] = VIR_TRISTATE_SWITCH_ON; break; @@ -17482,6 +17539,11 @@ virDomainFeaturesDefParse(virDomainDef *def, return -1; break; + case VIR_DOMAIN_FEATURE_XEN: + if (virDomainFeaturesXENDefParse(def, ctxt) < 0) + return -1; + break; + case VIR_DOMAIN_FEATURE_CAPABILITIES: { virDomainCapabilitiesPolicy policy; @@ -17615,50 +17677,6 @@ virDomainFeaturesDefParse(virDomainDef *def, } VIR_FREE(nodes); - if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { - int feature; - virTristateSwitch value; - - if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) < 0) - return -1; - - for (i = 0; i < n; i++) { - feature = virDomainXenTypeFromString((const char *)nodes[i]->name); - if (feature < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported Xen feature: %s"), - nodes[i]->name); - return -1; - } - - if (virXMLPropTristateSwitch(nodes[i], "state", - VIR_XML_PROP_REQUIRED, &value) < 0) - return -1; - - def->xen_features[feature] = value; - - switch ((virDomainXen) feature) { - case VIR_DOMAIN_XEN_E820_HOST: - break; - - case VIR_DOMAIN_XEN_PASSTHROUGH: - if (value != VIR_TRISTATE_SWITCH_ON) - break; - - if (virXMLPropEnum(nodes[i], "mode", - virDomainXenPassthroughModeTypeFromString, - VIR_XML_PROP_NONZERO, - &def->xen_passthrough_mode) < 0) - return -1; - break; - - case VIR_DOMAIN_XEN_LAST: - break; - } - } - VIR_FREE(nodes); - } - if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) { int rv = virParseScaledValue("string(./features/smm/tseg)", "string(./features/smm/tseg/@unit)", -- 2.31.1
Iterating over all child elements of a node does not require xpath. By doing away with xpath for this code, the code can be simplified. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e687f18afe..45c4b9cedf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17432,31 +17432,25 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, static int virDomainFeaturesXENDefParse(virDomainDef *def, - xmlXPathContext *ctxt) + xmlNodePtr node) { - g_autofree xmlNodePtr *nodes = NULL; - size_t i; - int n; - def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON; if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { int feature; virTristateSwitch value; - if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) < 0) - return -1; - - for (i = 0; i < n; i++) { - feature = virDomainXenTypeFromString((const char *)nodes[i]->name); + node = xmlFirstElementChild(node); + while (node) { + feature = virDomainXenTypeFromString((const char *)node->name); if (feature < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported Xen feature: %s"), - nodes[i]->name); + node->name); return -1; } - if (virXMLPropTristateSwitch(nodes[i], "state", + if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED, &value) < 0) return -1; @@ -17470,7 +17464,7 @@ virDomainFeaturesXENDefParse(virDomainDef *def, if (value != VIR_TRISTATE_SWITCH_ON) break; - if (virXMLPropEnum(nodes[i], "mode", + if (virXMLPropEnum(node, "mode", virDomainXenPassthroughModeTypeFromString, VIR_XML_PROP_NONZERO, &def->xen_passthrough_mode) < 0) @@ -17480,8 +17474,9 @@ virDomainFeaturesXENDefParse(virDomainDef *def, case VIR_DOMAIN_XEN_LAST: break; } + + node = xmlNextElementSibling(node); } - VIR_FREE(nodes); } return 0; @@ -17540,7 +17535,7 @@ virDomainFeaturesDefParse(virDomainDef *def, break; case VIR_DOMAIN_FEATURE_XEN: - if (virDomainFeaturesXENDefParse(def, ctxt) < 0) + if (virDomainFeaturesXENDefParse(def, nodes[i]) < 0) return -1; break; -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 58 ++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 45c4b9cedf..02c06d5ab9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17436,47 +17436,45 @@ virDomainFeaturesXENDefParse(virDomainDef *def, { def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON; - if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { + node = xmlFirstElementChild(node); + while (node) { int feature; virTristateSwitch value; - node = xmlFirstElementChild(node); - while (node) { - feature = virDomainXenTypeFromString((const char *)node->name); - if (feature < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported Xen feature: %s"), - node->name); - return -1; - } - - if (virXMLPropTristateSwitch(node, "state", - VIR_XML_PROP_REQUIRED, &value) < 0) - return -1; + feature = virDomainXenTypeFromString((const char *)node->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Xen feature: %s"), + node->name); + return -1; + } - def->xen_features[feature] = value; + if (virXMLPropTristateSwitch(node, "state", + VIR_XML_PROP_REQUIRED, &value) < 0) + return -1; - switch ((virDomainXen) feature) { - case VIR_DOMAIN_XEN_E820_HOST: - break; + def->xen_features[feature] = value; - case VIR_DOMAIN_XEN_PASSTHROUGH: - if (value != VIR_TRISTATE_SWITCH_ON) - break; + switch ((virDomainXen) feature) { + case VIR_DOMAIN_XEN_E820_HOST: + break; - if (virXMLPropEnum(node, "mode", - virDomainXenPassthroughModeTypeFromString, - VIR_XML_PROP_NONZERO, - &def->xen_passthrough_mode) < 0) - return -1; + case VIR_DOMAIN_XEN_PASSTHROUGH: + if (value != VIR_TRISTATE_SWITCH_ON) break; - case VIR_DOMAIN_XEN_LAST: - break; - } + if (virXMLPropEnum(node, "mode", + virDomainXenPassthroughModeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->xen_passthrough_mode) < 0) + return -1; + break; - node = xmlNextElementSibling(node); + case VIR_DOMAIN_XEN_LAST: + break; } + + node = xmlNextElementSibling(node); } return 0; -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 02c06d5ab9..b411c1fb8c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17554,8 +17554,7 @@ virDomainFeaturesDefParse(virDomainDef *def, case VIR_DOMAIN_FEATURE_HAP: case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: - case VIR_DOMAIN_FEATURE_VMPORT: - case VIR_DOMAIN_FEATURE_SMM: { + case VIR_DOMAIN_FEATURE_VMPORT: { virTristateSwitch state; if (virXMLPropTristateSwitch(nodes[i], "state", @@ -17569,6 +17568,31 @@ virDomainFeaturesDefParse(virDomainDef *def, break; } + case VIR_DOMAIN_FEATURE_SMM: { + virTristateSwitch state; + + if (virXMLPropTristateSwitch(nodes[i], "state", + VIR_XML_PROP_NONE, &state) < 0) + return -1; + + if ((state == VIR_TRISTATE_SWITCH_ABSENT) || + (state == VIR_TRISTATE_SWITCH_ON)) { + int rv = virParseScaledValue("string(./features/smm/tseg)", + "string(./features/smm/tseg/@unit)", + ctxt, + &def->tseg_size, + 1024 * 1024, /* Defaults to mebibytes */ + ULLONG_MAX, + false); + if (rv < 0) + return -1; + + def->features[val] = VIR_TRISTATE_SWITCH_ON; + def->tseg_specified = rv != 0; + } + break; + } + case VIR_DOMAIN_FEATURE_GIC: if (virXMLPropEnum(nodes[i], "version", virGICVersionTypeFromString, VIR_XML_PROP_NONZERO, &def->gic_version) < 0) @@ -17670,19 +17694,6 @@ virDomainFeaturesDefParse(virDomainDef *def, } VIR_FREE(nodes); - if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) { - int rv = virParseScaledValue("string(./features/smm/tseg)", - "string(./features/smm/tseg/@unit)", - ctxt, - &def->tseg_size, - 1024 * 1024, /* Defaults to mebibytes */ - ULLONG_MAX, - false); - if (rv < 0) - return -1; - def->tseg_specified = rv; - } - if (def->features[VIR_DOMAIN_FEATURE_MSRS] == VIR_TRISTATE_SWITCH_ON) { virDomainMsrsUnknown unknown; xmlNodePtr node = NULL; -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b411c1fb8c..915303adcd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17518,10 +17518,21 @@ virDomainFeaturesDefParse(virDomainDef *def, case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: - case VIR_DOMAIN_FEATURE_MSRS: def->features[val] = VIR_TRISTATE_SWITCH_ON; break; + case VIR_DOMAIN_FEATURE_MSRS: { + virDomainMsrsUnknown unknown; + if (virXMLPropEnum(nodes[i], "unknown", + virDomainMsrsUnknownTypeFromString, + VIR_XML_PROP_REQUIRED, &unknown) < 0) + return -1; + + def->features[val] = VIR_TRISTATE_SWITCH_ON; + def->msrs_features[VIR_DOMAIN_MSRS_UNKNOWN] = unknown; + break; + } + case VIR_DOMAIN_FEATURE_HYPERV: if (virDomainFeaturesHyperVDefParse(def, nodes[i]) < 0) return -1; @@ -17694,19 +17705,6 @@ virDomainFeaturesDefParse(virDomainDef *def, } VIR_FREE(nodes); - if (def->features[VIR_DOMAIN_FEATURE_MSRS] == VIR_TRISTATE_SWITCH_ON) { - virDomainMsrsUnknown unknown; - xmlNodePtr node = NULL; - if ((node = virXPathNode("./features/msrs", ctxt)) == NULL) - return -1; - - if (virXMLPropEnum(node, "unknown", virDomainMsrsUnknownTypeFromString, - VIR_XML_PROP_REQUIRED, &unknown) < 0) - return -1; - - def->msrs_features[VIR_DOMAIN_MSRS_UNKNOWN] = unknown; - } - if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0) return -1; -- 2.31.1
Cleanup to follow. This removes the last re-use of `nodes` in this function, eliminating two VIR_FREEs. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 78 +++++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 915303adcd..84c684b004 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17481,6 +17481,51 @@ virDomainFeaturesXENDefParse(virDomainDef *def, } +static int +virDomainFeaturesCapabilitiesDefParse(virDomainDef *def, + xmlNodePtr node, + xmlXPathContext *ctxt) +{ + g_autofree xmlNodePtr *nodes = NULL; + size_t i; + int n; + virDomainCapabilitiesPolicy policy; + + if (virXMLPropEnumDefault(node, "policy", + virDomainCapabilitiesPolicyTypeFromString, + VIR_XML_PROP_NONE, &policy, + VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT) < 0) + return -1; + + def->features[VIR_DOMAIN_FEATURE_CAPABILITIES] = policy; + + if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + virTristateSwitch state; + int val = virDomainProcessCapsFeatureTypeFromString((const char *)nodes[i]->name); + if (val < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unexpected capability feature '%s'"), nodes[i]->name); + return -1; + } + + + if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE, + &state) < 0) + return -1; + + if (state == VIR_TRISTATE_SWITCH_ABSENT) + state = VIR_TRISTATE_SWITCH_ON; + + def->caps_features[val] = state; + } + + return 0; +} + + static int virDomainFeaturesDefParse(virDomainDef *def, xmlXPathContextPtr ctxt) @@ -17549,15 +17594,8 @@ virDomainFeaturesDefParse(virDomainDef *def, break; case VIR_DOMAIN_FEATURE_CAPABILITIES: { - virDomainCapabilitiesPolicy policy; - - if (virXMLPropEnumDefault(nodes[i], "policy", - virDomainCapabilitiesPolicyTypeFromString, - VIR_XML_PROP_NONE, &policy, - VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT) < 0) + if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i], ctxt) < 0) return -1; - - def->features[val] = policy; break; } @@ -17703,31 +17741,7 @@ virDomainFeaturesDefParse(virDomainDef *def, break; } } - VIR_FREE(nodes); - if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0) - return -1; - - for (i = 0; i < n; i++) { - virTristateSwitch state; - int val = virDomainProcessCapsFeatureTypeFromString((const char *)nodes[i]->name); - if (val < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unexpected capability feature '%s'"), nodes[i]->name); - return -1; - } - - - if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE, - &state) < 0) - return -1; - - if (state == VIR_TRISTATE_SWITCH_ABSENT) - state = VIR_TRISTATE_SWITCH_ON; - - def->caps_features[val] = state; - } - VIR_FREE(nodes); return 0; } -- 2.31.1
Iterating over all child elements of a node does not require xpath. By doing away with xpath for this code, the code can be simplified. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 84c684b004..50717c4f44 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17483,12 +17483,8 @@ virDomainFeaturesXENDefParse(virDomainDef *def, static int virDomainFeaturesCapabilitiesDefParse(virDomainDef *def, - xmlNodePtr node, - xmlXPathContext *ctxt) + xmlNodePtr node) { - g_autofree xmlNodePtr *nodes = NULL; - size_t i; - int n; virDomainCapabilitiesPolicy policy; if (virXMLPropEnumDefault(node, "policy", @@ -17499,27 +17495,25 @@ virDomainFeaturesCapabilitiesDefParse(virDomainDef *def, def->features[VIR_DOMAIN_FEATURE_CAPABILITIES] = policy; - if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0) - return -1; - - for (i = 0; i < n; i++) { + node = xmlFirstElementChild(node); + while (node) { virTristateSwitch state; - int val = virDomainProcessCapsFeatureTypeFromString((const char *)nodes[i]->name); + int val = virDomainProcessCapsFeatureTypeFromString((const char *)node->name); if (val < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unexpected capability feature '%s'"), nodes[i]->name); + _("unexpected capability feature '%s'"), node->name); return -1; } - if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE, - &state) < 0) + if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_NONE, &state) < 0) return -1; if (state == VIR_TRISTATE_SWITCH_ABSENT) state = VIR_TRISTATE_SWITCH_ON; def->caps_features[val] = state; + node = xmlNextElementSibling(node); } return 0; @@ -17594,7 +17588,7 @@ virDomainFeaturesDefParse(virDomainDef *def, break; case VIR_DOMAIN_FEATURE_CAPABILITIES: { - if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i], ctxt) < 0) + if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i]) < 0) return -1; break; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 50717c4f44..d78f846a52 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17532,7 +17532,6 @@ virDomainFeaturesDefParse(virDomainDef *def, return -1; for (i = 0; i < n; i++) { - g_autofree char *tmp = NULL; int val = virDomainFeatureTypeFromString((const char *)nodes[i]->name); if (val < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -17541,18 +17540,6 @@ virDomainFeaturesDefParse(virDomainDef *def, } switch ((virDomainFeature) val) { - case VIR_DOMAIN_FEATURE_APIC: - if ((tmp = virXPathString("string(./features/apic/@eoi)", ctxt))) { - int eoi; - if ((eoi = virTristateSwitchTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown value for attribute eoi: '%s'"), - tmp); - return -1; - } - def->apic_eoi = eoi; - } - G_GNUC_FALLTHROUGH; case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_VIRIDIAN: @@ -17560,6 +17547,16 @@ virDomainFeaturesDefParse(virDomainDef *def, def->features[val] = VIR_TRISTATE_SWITCH_ON; break; + case VIR_DOMAIN_FEATURE_APIC: { + virTristateSwitch eoi; + if (virXMLPropTristateSwitch(nodes[i], "eoi", VIR_XML_PROP_NONE, &eoi) < 0) + return -1; + + def->features[val] = VIR_TRISTATE_SWITCH_ON; + def->apic_eoi = eoi; + break; + } + case VIR_DOMAIN_FEATURE_MSRS: { virDomainMsrsUnknown unknown; if (virXMLPropEnum(nodes[i], "unknown", -- 2.31.1
On 6/22/21 2:22 PM, Tim Wiederhake wrote:
Some refactoring in preparation for adding support for qemu's "hv-passthrough" and the yet-to-be-merged "hv-defaults".
Tim Wiederhake (16): virDomainFeaturesDefParse: Factor out HyperV parsing into separate function virDomainFeaturesHyperVDefParse: Inline hyperv/stimer parsing virDomainFeaturesHyperVDefParse: Remove ctxt virDomainFeaturesHyperVDefParse: Remove tautological "if" virDomainFeaturesDefParse: Factor out KVM parsing into separate function virDomainFeaturesKVMDefParse: Remove ctxt virDomainFeaturesKVMDefParse: Remove tautological "switch" virDomainFeaturesKVMDefParse: Remove tautological "if" virDomainFeaturesDefParse: Factor out XEN parsing into separate function virDomainFeaturesXENDefParse: Remove ctxt virDomainFeaturesXENDefParse: Remove tautological "if" virDomainFeaturesDefParse: Inline SMM parsing virDomainFeaturesDefParse: Inline MSRS parsing virDomainFeaturesDefParse: Factor out capabilities parsing into separate function virDomainFeaturesCapabilitiesDefParse: Remove ctxt virDomainFeaturesDefParse: Simplify APIC parsing
src/conf/domain_conf.c | 557 ++++++++++++++++++++++------------------- 1 file changed, 296 insertions(+), 261 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (2)
-
Michal Prívozník -
Tim Wiederhake