Separate the code into virDomainNetDefParseXMLDriver. Some local variables were renamed and the scope decreased. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 238 ++++++++++++++++++++++------------------- 1 file changed, 126 insertions(+), 112 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0e25d585c8..621b1df6c8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8827,6 +8827,130 @@ virDomainNetTeamingInfoParseXML(xmlXPathContextPtr ctxt, } +static int +virDomainNetDefParseXMLDriver(virDomainNetDef *def, + xmlXPathContextPtr ctxt) +{ + xmlNodePtr driver_node; + + if ((driver_node = virXPathNode("./driver", ctxt)) && + (virDomainVirtioOptionsParseXML(driver_node, &def->virtio) < 0)) + return -1; + + if (def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && + virDomainNetIsVirtioModel(def)) { + xmlNodePtr hostNode; + xmlNodePtr guestNode; + + if (virXMLPropEnum(driver_node, "name", + virDomainNetBackendTypeFromString, + VIR_XML_PROP_NONZERO, + &def->driver.virtio.name) < 0) + return -1; + + if (virXMLPropEnum(driver_node, "txmode", + virDomainNetVirtioTxModeTypeFromString, + VIR_XML_PROP_NONZERO, + &def->driver.virtio.txmode) < 0) + return -1; + + if (virXMLPropTristateSwitch(driver_node, "ioeventfd", + VIR_XML_PROP_NONE, + &def->driver.virtio.ioeventfd) < 0) + return -1; + + if (virXMLPropTristateSwitch(driver_node, "event_idx", + VIR_XML_PROP_NONE, + &def->driver.virtio.event_idx) < 0) + return -1; + + if (virXMLPropUInt(driver_node, "queues", 10, + VIR_XML_PROP_NONE, + &def->driver.virtio.queues) < 0) + return -1; + + /* There's always at least one TX/RX queue. */ + if (def->driver.virtio.queues == 1) + def->driver.virtio.queues = 0; + + if (virXMLPropUInt(driver_node, "rx_queue_size", 10, + VIR_XML_PROP_NONE, + &def->driver.virtio.rx_queue_size) < 0) + return -1; + + if (virXMLPropUInt(driver_node, "tx_queue_size", 10, + VIR_XML_PROP_NONE, + &def->driver.virtio.tx_queue_size) < 0) + return -1; + + if (virXMLPropTristateSwitch(driver_node, "rss", + VIR_XML_PROP_NONE, + &def->driver.virtio.rss) < 0) + return -1; + + if (virXMLPropTristateSwitch(driver_node, "rss_hash_report", + VIR_XML_PROP_NONE, + &def->driver.virtio.rss_hash_report) < 0) + return -1; + + if ((hostNode = virXPathNode("./driver/host", ctxt))) { + if (virXMLPropTristateSwitch(hostNode, "csum", VIR_XML_PROP_NONE, + &def->driver.virtio.host.csum) < 0) + return -1; + + if (virXMLPropTristateSwitch(hostNode, "gso", VIR_XML_PROP_NONE, + &def->driver.virtio.host.gso) < 0) + return -1; + + if (virXMLPropTristateSwitch(hostNode, "tso4", VIR_XML_PROP_NONE, + &def->driver.virtio.host.tso4) < 0) + return -1; + + if (virXMLPropTristateSwitch(hostNode, "tso6", VIR_XML_PROP_NONE, + &def->driver.virtio.host.tso6) < 0) + return -1; + + if (virXMLPropTristateSwitch(hostNode, "ecn", VIR_XML_PROP_NONE, + &def->driver.virtio.host.ecn) < 0) + return -1; + + if (virXMLPropTristateSwitch(hostNode, "ufo", VIR_XML_PROP_NONE, + &def->driver.virtio.host.ufo) < 0) + return -1; + + if (virXMLPropTristateSwitch(hostNode, "mrg_rxbuf", + VIR_XML_PROP_NONE, + &def->driver.virtio.host.mrg_rxbuf) < 0) + return -1; + } + + if ((guestNode = virXPathNode("./driver/guest", ctxt))) { + if (virXMLPropTristateSwitch(guestNode, "csum", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.csum) < 0) + return -1; + + if (virXMLPropTristateSwitch(guestNode, "tso4", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.tso4) < 0) + return -1; + + if (virXMLPropTristateSwitch(guestNode, "tso6", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.tso6) < 0) + return -1; + + if (virXMLPropTristateSwitch(guestNode, "ecn", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.ecn) < 0) + return -1; + + if (virXMLPropTristateSwitch(guestNode, "ufo", VIR_XML_PROP_NONE, + &def->driver.virtio.guest.ufo) < 0) + return -1; + } + } + + return 0; +} + + static virDomainNetDef * virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr node, @@ -8837,7 +8961,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, virDomainHostdevDef *hostdev; xmlNodePtr source_node = NULL; xmlNodePtr virtualport_node = NULL; - xmlNodePtr driver_node = NULL; xmlNodePtr filterref_node = NULL; xmlNodePtr actual_node = NULL; xmlNodePtr vlan_node = NULL; @@ -9009,10 +9132,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, def->domain_name = virXPathString("string(./backenddomain/@name)", ctxt); model = virXPathString("string(./model/@type)", ctxt); - if ((driver_node = virXPathNode("./driver", ctxt)) && - (virDomainVirtioOptionsParseXML(driver_node, &def->virtio) < 0)) - return NULL; - if ((filterref_node = virXPathNode("./filterref", ctxt))) { filter = virXMLPropString(filterref_node, "filter"); filterparams = virNWFilterParseParamAttributes(filterref_node); @@ -9373,113 +9492,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_clear_pointer(&def->ifname, g_free); } - if (def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && - virDomainNetIsVirtioModel(def)) { - - if (virXMLPropEnum(driver_node, "name", - virDomainNetBackendTypeFromString, - VIR_XML_PROP_NONZERO, - &def->driver.virtio.name) < 0) - return NULL; - - if (virXMLPropEnum(driver_node, "txmode", - virDomainNetVirtioTxModeTypeFromString, - VIR_XML_PROP_NONZERO, - &def->driver.virtio.txmode) < 0) - return NULL; - - if (virXMLPropTristateSwitch(driver_node, "ioeventfd", - VIR_XML_PROP_NONE, - &def->driver.virtio.ioeventfd) < 0) - return NULL; - - if (virXMLPropTristateSwitch(driver_node, "event_idx", - VIR_XML_PROP_NONE, - &def->driver.virtio.event_idx) < 0) - return NULL; - - if (virXMLPropUInt(driver_node, "queues", 10, - VIR_XML_PROP_NONE, - &def->driver.virtio.queues) < 0) - return NULL; - - /* There's always at least one TX/RX queue. */ - if (def->driver.virtio.queues == 1) - def->driver.virtio.queues = 0; - - if (virXMLPropUInt(driver_node, "rx_queue_size", 10, - VIR_XML_PROP_NONE, - &def->driver.virtio.rx_queue_size) < 0) - return NULL; - - if (virXMLPropUInt(driver_node, "tx_queue_size", 10, - VIR_XML_PROP_NONE, - &def->driver.virtio.tx_queue_size) < 0) - return NULL; - - if (virXMLPropTristateSwitch(driver_node, "rss", - VIR_XML_PROP_NONE, - &def->driver.virtio.rss) < 0) - return NULL; - - if (virXMLPropTristateSwitch(driver_node, "rss_hash_report", - VIR_XML_PROP_NONE, - &def->driver.virtio.rss_hash_report) < 0) - return NULL; - - if ((tmpNode = virXPathNode("./driver/host", ctxt))) { - if (virXMLPropTristateSwitch(tmpNode, "csum", VIR_XML_PROP_NONE, - &def->driver.virtio.host.csum) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "gso", VIR_XML_PROP_NONE, - &def->driver.virtio.host.gso) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "tso4", VIR_XML_PROP_NONE, - &def->driver.virtio.host.tso4) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "tso6", VIR_XML_PROP_NONE, - &def->driver.virtio.host.tso6) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "ecn", VIR_XML_PROP_NONE, - &def->driver.virtio.host.ecn) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "ufo", VIR_XML_PROP_NONE, - &def->driver.virtio.host.ufo) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "mrg_rxbuf", - VIR_XML_PROP_NONE, - &def->driver.virtio.host.mrg_rxbuf) < 0) - return NULL; - } - - if ((tmpNode = virXPathNode("./driver/guest", ctxt))) { - if (virXMLPropTristateSwitch(tmpNode, "csum", VIR_XML_PROP_NONE, - &def->driver.virtio.guest.csum) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "tso4", VIR_XML_PROP_NONE, - &def->driver.virtio.guest.tso4) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "tso6", VIR_XML_PROP_NONE, - &def->driver.virtio.guest.tso6) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "ecn", VIR_XML_PROP_NONE, - &def->driver.virtio.guest.ecn) < 0) - return NULL; - - if (virXMLPropTristateSwitch(tmpNode, "ufo", VIR_XML_PROP_NONE, - &def->driver.virtio.guest.ufo) < 0) - return NULL; - } - } + if (virDomainNetDefParseXMLDriver(def, ctxt) < 0) + return NULL; if (def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && virDomainNetIsVirtioModel(def)) { -- 2.37.1

Replace ad-hoc logic that fills the default by use of the proper helper function. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 621b1df6c8..745a7e428c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9002,13 +9002,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, ctxt->node = node; - if ((rv = virXMLPropEnum(node, "type", virDomainNetTypeFromString, - VIR_XML_PROP_NONE, &def->type)) < 0) + if (virXMLPropEnumDefault(node, "type", virDomainNetTypeFromString, + VIR_XML_PROP_NONE, &def->type, VIR_DOMAIN_NET_TYPE_USER) < 0) return NULL; - if (rv == 0) - def->type = VIR_DOMAIN_NET_TYPE_USER; - if (virXMLPropTristateBool(node, "trustGuestRxFilters", VIR_XML_PROP_NONE, &def->trustGuestRxFilters) < 0) return NULL; -- 2.37.1

The variables are only used in code paths which can't fail after they are allocated. Additionally decrease scope of the variables. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 745a7e428c..5c91cbd0db 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6216,17 +6216,17 @@ virDomainNetIPInfoParseXML(const char *source, xmlXPathContextPtr ctxt, virNetDevIPInfo *def) { - g_autoptr(virNetDevIPRoute) route = NULL; int nnodes; int ret = -1; size_t i; g_autofree xmlNodePtr *nodes = NULL; - g_autofree virNetDevIPAddr *ip = NULL; if ((nnodes = virXPathNodeSet("./ip", ctxt, &nodes)) < 0) goto cleanup; for (i = 0; i < nnodes; i++) { + virNetDevIPAddr *ip = NULL; + if (!(ip = virDomainNetIPParseXML(nodes[i]))) goto cleanup; @@ -6238,6 +6238,8 @@ virDomainNetIPInfoParseXML(const char *source, goto cleanup; for (i = 0; i < nnodes; i++) { + virNetDevIPRoute *route = NULL; + if (!(route = virNetDevIPRouteParseXML(source, nodes[i], ctxt))) goto cleanup; -- 2.37.1

Use two separate variables for the nodes and count instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5c91cbd0db..797914b7bc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6216,31 +6216,32 @@ virDomainNetIPInfoParseXML(const char *source, xmlXPathContextPtr ctxt, virNetDevIPInfo *def) { - int nnodes; int ret = -1; size_t i; - g_autofree xmlNodePtr *nodes = NULL; + g_autofree xmlNodePtr *ipNodes = NULL; + int nipNodes; + g_autofree xmlNodePtr *routeNodes = NULL; + int nrouteNodes; - if ((nnodes = virXPathNodeSet("./ip", ctxt, &nodes)) < 0) + if ((nipNodes = virXPathNodeSet("./ip", ctxt, &ipNodes)) < 0) goto cleanup; - for (i = 0; i < nnodes; i++) { + for (i = 0; i < nipNodes; i++) { virNetDevIPAddr *ip = NULL; - if (!(ip = virDomainNetIPParseXML(nodes[i]))) + if (!(ip = virDomainNetIPParseXML(ipNodes[i]))) goto cleanup; VIR_APPEND_ELEMENT(def->ips, def->nips, ip); } - VIR_FREE(nodes); - if ((nnodes = virXPathNodeSet("./route", ctxt, &nodes)) < 0) + if ((nrouteNodes = virXPathNodeSet("./route", ctxt, &routeNodes)) < 0) goto cleanup; - for (i = 0; i < nnodes; i++) { + for (i = 0; i < nrouteNodes; i++) { virNetDevIPRoute *route = NULL; - if (!(route = virNetDevIPRouteParseXML(source, nodes[i], ctxt))) + if (!(route = virNetDevIPRouteParseXML(source, routeNodes[i], ctxt))) goto cleanup; VIR_APPEND_ELEMENT(def->routes, def->nroutes, route); -- 2.37.1

Do the XPath fetches first as they don't require cleanup and rename 'cleanup' to 'error' and take it only on failure. Signed-off-by: Peter Krempa <pkrempa@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 797914b7bc..1b52ea52c4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6216,42 +6216,38 @@ virDomainNetIPInfoParseXML(const char *source, xmlXPathContextPtr ctxt, virNetDevIPInfo *def) { - int ret = -1; size_t i; g_autofree xmlNodePtr *ipNodes = NULL; int nipNodes; g_autofree xmlNodePtr *routeNodes = NULL; int nrouteNodes; - if ((nipNodes = virXPathNodeSet("./ip", ctxt, &ipNodes)) < 0) - goto cleanup; + if ((nipNodes = virXPathNodeSet("./ip", ctxt, &ipNodes)) < 0 || + (nrouteNodes = virXPathNodeSet("./route", ctxt, &routeNodes)) < 0) + return -1; for (i = 0; i < nipNodes; i++) { virNetDevIPAddr *ip = NULL; if (!(ip = virDomainNetIPParseXML(ipNodes[i]))) - goto cleanup; + goto error; VIR_APPEND_ELEMENT(def->ips, def->nips, ip); } - if ((nrouteNodes = virXPathNodeSet("./route", ctxt, &routeNodes)) < 0) - goto cleanup; - for (i = 0; i < nrouteNodes; i++) { virNetDevIPRoute *route = NULL; if (!(route = virNetDevIPRouteParseXML(source, routeNodes[i], ctxt))) - goto cleanup; + goto error; VIR_APPEND_ELEMENT(def->routes, def->nroutes, route); } - ret = 0; - cleanup: - if (ret < 0) - virNetDevIPInfoClear(def); - return ret; + return 0; + error: + virNetDevIPInfoClear(def); + return -1; } -- 2.37.1

In certain cases it's inconvenient to move the XPAth's context current node in the caller. Add a 'node' argument and override it inside the function. VIR_XPATH_NODE_AUTORESTORE handles the cleanup. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1b52ea52c4..8b529acef6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6213,15 +6213,20 @@ virDomainNetIPParseXML(xmlNodePtr node) */ static int virDomainNetIPInfoParseXML(const char *source, + xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevIPInfo *def) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) size_t i; g_autofree xmlNodePtr *ipNodes = NULL; int nipNodes; g_autofree xmlNodePtr *routeNodes = NULL; int nrouteNodes; + if (node) + ctxt->node = node; + if ((nipNodes = virXPathNodeSet("./ip", ctxt, &ipNodes)) < 0 || (nrouteNodes = virXPathNodeSet("./route", ctxt, &routeNodes)) < 0) return -1; @@ -6368,7 +6373,7 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node G_GNUC_UNUSED, _("Missing <interface> element in hostdev net device")); return -1; } - if (virDomainNetIPInfoParseXML(_("Domain hostdev device"), + if (virDomainNetIPInfoParseXML(_("Domain hostdev device"), NULL, ctxt, &def->source.caps.u.net.ip) < 0) return -1; break; @@ -9010,12 +9015,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, return NULL; if ((source_node = virXPathNode("./source", ctxt))) { - xmlNodePtr tmpnode = ctxt->node; - - ctxt->node = source_node; - if (virDomainNetIPInfoParseXML(_("interface host IP"), ctxt, &def->hostIP) < 0) + if (virDomainNetIPInfoParseXML(_("interface host IP"), source_node, ctxt, &def->hostIP) < 0) return NULL; - ctxt->node = tmpnode; if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { network = virXMLPropString(source_node, "network"); @@ -9463,7 +9464,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; } - if (virDomainNetIPInfoParseXML(_("guest interface"), + if (virDomainNetIPInfoParseXML(_("guest interface"), node, ctxt, &def->guestIP) < 0) return NULL; -- 2.37.1

The helper function extracts an UUID with semantics similar to other helpers we have. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 45 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 7 +++++++ 3 files changed, 53 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 25794bc2f4..1e852902ab 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3688,6 +3688,7 @@ virXMLPropTristateBoolAllowDefault; virXMLPropTristateSwitch; virXMLPropUInt; virXMLPropULongLong; +virXMLPropUUID; virXMLSaveFile; virXMLValidateAgainstSchema; virXMLValidatorFree; diff --git a/src/util/virxml.c b/src/util/virxml.c index d6e2e5dd91..04a8b29ba3 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -33,6 +33,7 @@ #include "virfile.h" #include "virstring.h" #include "virutil.h" +#include "viruuid.h" #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_XML @@ -808,6 +809,50 @@ virXMLPropEnumDefault(xmlNodePtr node, } +/** + * virXMLPropUUID: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @flags: Bitwise or of virXMLPropFlags + * @result: array of VIR_UUID_BUFLEN bytes to store the raw UUID + * + * Convenience function to fetch a XML property as UUID. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropUUID(xmlNodePtr node, + const char* name, + virXMLPropFlags flags, + unsigned char *result) +{ + g_autofree char *tmp = NULL; + unsigned char val[VIR_UUID_BUFLEN]; + + if (!(tmp = virXMLPropString(node, name))) { + if (!(flags & VIR_XML_PROP_REQUIRED)) + return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + if (virUUIDParse(tmp, val) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected UUID"), + name, node->name, tmp); + return -1; + } + + memcpy(result, val, VIR_UUID_BUFLEN); + return 1; +} + + /** * virXMLPropEnum: * @node: XML dom node pointer diff --git a/src/util/virxml.h b/src/util/virxml.h index 539228a9ba..dfb58ff276 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -154,6 +154,13 @@ virXMLPropEnum(xmlNodePtr node, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); +int +virXMLPropUUID(xmlNodePtr node, + const char* name, + virXMLPropFlags flags, + unsigned char *result) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4); + int virXMLPropEnumDefault(xmlNodePtr node, const char* name, -- 2.37.1

All callers treat NULL as if the string is not present in the XML. Adjust the description so that it's implied that it's not an error and thus also no error reporting is expected. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 04a8b29ba3..eac2d0a73f 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -443,7 +443,8 @@ virXMLCheckIllegalChars(const char *nodeName, * * Convenience function to return copy of an attribute value of a XML node. * - * Returns the property (attribute) value as string or NULL in case of failure. + * Returns the property (attribute) value as string or NULL if the attribute + * is not present (no error is reported). * The caller is responsible for freeing the returned buffer. */ char * -- 2.37.1

Similarly to virXMLPropString it extracts a string but reports an errori similar to the newer virXMLProp helpers if the attribute is not present. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 27 +++++++++++++++++++++++++++ src/util/virxml.h | 4 ++++ 3 files changed, 32 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e852902ab..01bb349e05 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3683,6 +3683,7 @@ virXMLPropEnum; virXMLPropEnumDefault; virXMLPropInt; virXMLPropString; +virXMLPropStringRequired; virXMLPropTristateBool; virXMLPropTristateBoolAllowDefault; virXMLPropTristateSwitch; diff --git a/src/util/virxml.c b/src/util/virxml.c index eac2d0a73f..b94af9c7a3 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -455,6 +455,33 @@ virXMLPropString(xmlNodePtr node, } +/** + * virXMLPropStringRequired: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * + * Convenience function to return copy of an mandatoryu attribute value of a + * XML node. + * + * Returns the property (attribute) value as string or NULL and if the attribute + * is not present (libvirt error reported). + * The caller is responsible for freeing the returned buffer. + */ +char * +virXMLPropStringRequired(xmlNodePtr node, + const char *name) +{ + char *ret = virXMLPropString(node, name); + + if (!(*ret)) + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + + return ret; +} + + /** * virXMLNodeContentString: * @node: XML dom node pointer diff --git a/src/util/virxml.h b/src/util/virxml.h index dfb58ff276..42d3740c58 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -97,6 +97,10 @@ char * virXMLPropString(xmlNodePtr node, const char *name); char * +virXMLPropStringRequired(xmlNodePtr node, + const char *name); + +char * virXMLNodeContentString(xmlNodePtr node); int -- 2.37.1

Convert the individual 'if' clauses to a swtich statement. By moving the check that 'source_node' is non-null inside of each case rather we will be able to move more type specific code into the swithc statemen when it will be refactored in subsequent patches. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 71 +++++++++++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8b529acef6..d75ce46a7f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9017,32 +9017,47 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, if ((source_node = virXPathNode("./source", ctxt))) { if (virDomainNetIPInfoParseXML(_("interface host IP"), source_node, ctxt, &def->hostIP) < 0) return NULL; + } - if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + switch (def->type) { + case VIR_DOMAIN_NET_TYPE_NETWORK: + if (source_node) { network = virXMLPropString(source_node, "network"); portgroup = virXMLPropString(source_node, "portgroup"); if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) portid = virXMLPropString(source_node, "portid"); } + break; - if (def->type == VIR_DOMAIN_NET_TYPE_VDS) { + case VIR_DOMAIN_NET_TYPE_VDS: + if (source_node) { switchid = virXMLPropString(source_node, "switchid"); portid = virXMLPropString(source_node, "portid"); portgroup = virXMLPropString(source_node, "portgroupid"); connectionid = virXMLPropString(source_node, "connectionid"); } + break; - if (def->type == VIR_DOMAIN_NET_TYPE_INTERNAL) + case VIR_DOMAIN_NET_TYPE_INTERNAL: + if (source_node) { internal = virXMLPropString(source_node, "name"); + } + break; - if (def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) + case VIR_DOMAIN_NET_TYPE_BRIDGE: + if (source_node) { bridge = virXMLPropString(source_node, "bridge"); + } + break; - if (def->type == VIR_DOMAIN_NET_TYPE_DIRECT) { + case VIR_DOMAIN_NET_TYPE_DIRECT: + if (source_node) { dev = virXMLPropString(source_node, "dev"); mode = virXMLPropString(source_node, "mode"); } + break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: /* This clause is only necessary because from 2010 to 2016 it was * possible (but never documented) to configure the name of the * guest-side interface of an openvz domain with <source dev='blah'/>. @@ -9051,33 +9066,40 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, * need to generate an error. If the openvz driver is ever * deprecated, this clause can be removed from here. */ - if (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET && - (dev = virXMLPropString(source_node, "dev"))) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid attempt to set <interface type='ethernet'> " - "device name with <source dev='%s'/>. " - "Use <target dev='%s'/> (for host-side) " - "or <guest dev='%s'/> (for guest-side) instead."), - dev, dev, dev); - return NULL; + if (source_node) { + if ((dev = virXMLPropString(source_node, "dev"))) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid attempt to set <interface type='ethernet'> " + "device name with <source dev='%s'/>. " + "Use <target dev='%s'/> (for host-side) " + "or <guest dev='%s'/> (for guest-side) instead."), + dev, dev, dev); + return NULL; + } } + break; - if (def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if (source_node) { vhostuser_type = virXMLPropString(source_node, "type"); vhostuser_path = virXMLPropString(source_node, "path"); vhostuser_mode = virXMLPropString(source_node, "mode"); if (virDomainChrSourceReconnectDefParseXML(&reconnect, source_node, ctxt) < 0) return NULL; } + break; - if (def->type == VIR_DOMAIN_NET_TYPE_VDPA) + case VIR_DOMAIN_NET_TYPE_VDPA: + if (source_node) { dev = virXMLPropString(source_node, "dev"); + } + break; - if (def->type == VIR_DOMAIN_NET_TYPE_SERVER || - def->type == VIR_DOMAIN_NET_TYPE_CLIENT || - def->type == VIR_DOMAIN_NET_TYPE_MCAST || - def->type == VIR_DOMAIN_NET_TYPE_UDP) { - + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_UDP: + if (source_node) { address = virXMLPropString(source_node, "address"); port = virXMLPropString(source_node, "port"); if (def->type == VIR_DOMAIN_NET_TYPE_UDP) { @@ -9090,6 +9112,13 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, ctxt->node = tmp_node; } } + break; + + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_NULL: + case VIR_DOMAIN_NET_TYPE_LAST: + break; } if ((virtualport_node = virXPathNode("./virtualport", ctxt))) { -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 68 +++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d75ce46a7f..de0a038526 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8955,6 +8955,21 @@ virDomainNetDefParseXMLDriver(virDomainNetDef *def, } +static int +virDomainNetDefParseXMLRequireSource(virDomainNetDef *def, + xmlNodePtr source_node) +{ + if (!source_node) { + virReportError(VIR_ERR_XML_ERROR, + _("interface type='%s' requires a 'source' element"), + virDomainNetTypeToString(def->type)); + return -1; + } + + return 0; +} + + static virDomainNetDef * virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr node, @@ -8966,18 +8981,15 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr source_node = NULL; xmlNodePtr virtualport_node = NULL; xmlNodePtr filterref_node = NULL; - xmlNodePtr actual_node = NULL; xmlNodePtr vlan_node = NULL; xmlNodePtr bandwidth_node = NULL; xmlNodePtr tmpNode; xmlNodePtr mac_node = NULL; g_autoptr(GHashTable) filterparams = NULL; - g_autoptr(virDomainActualNetDef) actual = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) virDomainChrSourceReconnectDef reconnect = {0}; int rv, val; g_autofree char *macaddr = NULL; - g_autofree char *network = NULL; g_autofree char *portgroup = NULL; g_autofree char *portid = NULL; g_autofree char *bridge = NULL; @@ -9021,11 +9033,28 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: - if (source_node) { - network = virXMLPropString(source_node, "network"); - portgroup = virXMLPropString(source_node, "portgroup"); - if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) - portid = virXMLPropString(source_node, "portid"); + if (virDomainNetDefParseXMLRequireSource(def, source_node) < 0) + return NULL; + + if (!(def->data.network.name = virXMLPropStringRequired(source_node, "network"))) + return NULL; + + def->data.network.portgroup = virXMLPropString(source_node, "portgroup"); + + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { + if (virXMLPropUUID(source_node, "portid", VIR_XML_PROP_NONE, + def->data.network.portid) < 0) + return NULL; + } + + if ((flags & VIR_DOMAIN_DEF_PARSE_ACTUAL_NET)) { + xmlNodePtr actual_node = NULL; + + if ((actual_node = virXPathNode("./actual", ctxt)) && + (virDomainActualNetDefParseXML(actual_node, ctxt, def, + &def->data.network.actual, + flags, xmlopt) < 0)) + return NULL; } break; @@ -9163,13 +9192,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, filterparams = virNWFilterParseParamAttributes(filterref_node); } - if ((flags & VIR_DOMAIN_DEF_PARSE_ACTUAL_NET) && - def->type == VIR_DOMAIN_NET_TYPE_NETWORK && - (actual_node = virXPathNode("./actual", ctxt)) && - (virDomainActualNetDefParseXML(actual_node, ctxt, def, - &actual, flags, xmlopt) < 0)) - return NULL; - if ((bandwidth_node = virXPathNode("./bandwidth", ctxt)) && (virNetDevBandwidthParse(&def->bandwidth, NULL, bandwidth_node, def->type == VIR_DOMAIN_NET_TYPE_NETWORK) < 0)) @@ -9225,22 +9247,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: - if (network == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'network' attribute " - "specified with <interface type='network'/>")); - return NULL; - } - if (portid && - virUUIDParse(portid, def->data.network.portid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to parse port id '%s'"), portid); - return NULL; - } - - def->data.network.name = g_steal_pointer(&network); - def->data.network.portgroup = g_steal_pointer(&portgroup); - def->data.network.actual = g_steal_pointer(&actual); break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: -- 2.37.1

Add a helper for parsing long long values from XML properties with semantics like virXMLPropInt. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 64 ++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 9 ++++++ 3 files changed, 74 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 01bb349e05..5be40dbefe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3682,6 +3682,7 @@ virXMLPickShellSafeComment; virXMLPropEnum; virXMLPropEnumDefault; virXMLPropInt; +virXMLPropLongLong; virXMLPropString; virXMLPropStringRequired; virXMLPropTristateBool; diff --git a/src/util/virxml.c b/src/util/virxml.c index b94af9c7a3..0a91fd237e 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -750,6 +750,70 @@ virXMLPropUInt(xmlNodePtr node, } +/** + * virXMLPropLongLong: + * @node: XML dom node pointer + * @name: Name of the property (attribute) to get + * @base: Number base, see strtol + * @flags: Bitwise or of virXMLPropFlags + * @result: The returned value + * @defaultResult: default value of @result in case the property is not found + * + * Convenience function to return value of a long long attribute. + * + * Returns 1 in case of success in which case @result is set, + * or 0 if the attribute is not present, + * or -1 and reports an error on failure. + */ +int +virXMLPropLongLong(xmlNodePtr node, + const char* name, + int base, + virXMLPropFlags flags, + long long *result, + long long defaultResult) +{ + g_autofree char *tmp = NULL; + long long val; + + *result = defaultResult; + + if (!(tmp = virXMLPropString(node, name))) { + if (!(flags & VIR_XML_PROP_REQUIRED)) + return 0; + + virReportError(VIR_ERR_XML_ERROR, + _("Missing required attribute '%s' in element '%s'"), + name, node->name); + return -1; + } + + if (virStrToLong_ll(tmp, NULL, base, &val) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected long long integer value"), + name, node->name, tmp); + return -1; + } + + if ((flags & VIR_XML_PROP_NONNEGATIVE) && (val < 0)) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': '%s'. Expected non-negative value"), + name, node->name, tmp); + return -1; + } + + if ((flags & VIR_XML_PROP_NONZERO) && (val == 0)) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value for attribute '%s' in element '%s': Zero is not permitted"), + name, node->name); + return -1; + } + + *result = val; + return 1; +} + + /** * virXMLPropULongLong: * @node: XML dom node pointer diff --git a/src/util/virxml.h b/src/util/virxml.h index 42d3740c58..3b00d20ea9 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -141,6 +141,15 @@ virXMLPropUInt(xmlNodePtr node, unsigned int *result) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5); +int +virXMLPropLongLong(xmlNodePtr node, + const char* name, + int base, + virXMLPropFlags flags, + long long *result, + long long defaultResult) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5); + int virXMLPropULongLong(xmlNodePtr node, const char* name, -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 78 ++++++++++-------------------------------- 1 file changed, 18 insertions(+), 60 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index de0a038526..cdf94ac75d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8990,8 +8990,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, virDomainChrSourceReconnectDef reconnect = {0}; int rv, val; g_autofree char *macaddr = NULL; - g_autofree char *portgroup = NULL; - g_autofree char *portid = NULL; g_autofree char *bridge = NULL; g_autofree char *dev = NULL; g_autofree char *managed_tap = NULL; @@ -9009,8 +9007,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *vhostuser_path = NULL; g_autofree char *vhostuser_type = NULL; g_autofree char *tap = NULL; - g_autofree char *switchid = NULL; - g_autofree char *connectionid = NULL; const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL; if (!(def = virDomainNetDefNew(xmlopt))) @@ -9059,12 +9055,24 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_VDS: - if (source_node) { - switchid = virXMLPropString(source_node, "switchid"); - portid = virXMLPropString(source_node, "portid"); - portgroup = virXMLPropString(source_node, "portgroupid"); - connectionid = virXMLPropString(source_node, "connectionid"); - } + if (virDomainNetDefParseXMLRequireSource(def, source_node) < 0) + return NULL; + + if (virXMLPropUUID(source_node, "switchid", VIR_XML_PROP_REQUIRED, + def->data.vds.switch_id) < 0) + return NULL; + + if (virXMLPropLongLong(source_node, "portid", 0, VIR_XML_PROP_REQUIRED, + &def->data.vds.port_id, def->data.vds.port_id) < 0) + return NULL; + + if (!(def->data.vds.portgroup_id = virXMLPropStringRequired(source_node, "portgroupid"))) + return NULL; + + if (virXMLPropLongLong(source_node, "connectionid", 0, VIR_XML_PROP_REQUIRED, + &def->data.vds.connection_id, def->data.vds.connection_id) < 0) + return NULL; + break; case VIR_DOMAIN_NET_TYPE_INTERNAL: @@ -9442,56 +9450,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_VDS: - if (!switchid) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing source switchid for interface type '%s'"), - virDomainNetTypeToString(def->type)); - return NULL; - } - - if (!portid) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing source portid for interface type '%s'"), - virDomainNetTypeToString(def->type)); - return NULL; - } - - if (!connectionid) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing source connectionid for interface type '%s'"), - virDomainNetTypeToString(def->type)); - return NULL; - } - - if (!portgroup) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing source portgroup for interface type '%s'"), - virDomainNetTypeToString(def->type)); - return NULL; - } - - if (virUUIDParse(switchid, def->data.vds.switch_id) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to parse switchid '%s'"), switchid); - return NULL; - } - - if (virStrToLong_ll(portid, NULL, 0, &def->data.vds.port_id) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to parse portid '%s'"), portid); - return NULL; - } - - if (virStrToLong_ll(connectionid, NULL, 0, &def->data.vds.connection_id) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to parse connectionid '%s'"), connectionid); - return NULL; - } - - def->data.vds.portgroup_id = g_steal_pointer(&portgroup); - - break; - case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_NULL: -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cdf94ac75d..0c97be1766 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8999,7 +8999,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *localport = NULL; g_autofree char *model = NULL; g_autofree char *filter = NULL; - g_autofree char *internal = NULL; g_autofree char *mode = NULL; g_autofree char *linkstate = NULL; g_autofree char *addrtype = NULL; @@ -9076,9 +9075,11 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_INTERNAL: - if (source_node) { - internal = virXMLPropString(source_node, "name"); - } + if (virDomainNetDefParseXMLRequireSource(def, source_node) < 0) + return NULL; + + if (!(def->data.internal.name = virXMLPropStringRequired(source_node, "name"))) + return NULL; break; case VIR_DOMAIN_NET_TYPE_BRIDGE: @@ -9399,13 +9400,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_INTERNAL: - if (internal == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'name' attribute specified " - "with <interface type='internal'/>")); - return NULL; - } - def->data.internal.name = g_steal_pointer(&internal); break; case VIR_DOMAIN_NET_TYPE_DIRECT: -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0c97be1766..431fd7a255 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8990,7 +8990,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, virDomainChrSourceReconnectDef reconnect = {0}; int rv, val; g_autofree char *macaddr = NULL; - g_autofree char *bridge = NULL; g_autofree char *dev = NULL; g_autofree char *managed_tap = NULL; g_autofree char *address = NULL; @@ -9083,9 +9082,11 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_BRIDGE: - if (source_node) { - bridge = virXMLPropString(source_node, "bridge"); - } + if (virDomainNetDefParseXMLRequireSource(def, source_node) < 0) + return NULL; + + if (!(def->data.bridge.brname = virXMLPropStringRequired(source_node, "bridge"))) + return NULL; break; case VIR_DOMAIN_NET_TYPE_DIRECT: @@ -9334,13 +9335,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_BRIDGE: - if (bridge == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'bridge' attribute " - "specified with <interface type='bridge'/>")); - return NULL; - } - def->data.bridge.brname = g_steal_pointer(&bridge); break; case VIR_DOMAIN_NET_TYPE_CLIENT: -- 2.37.1

Use 'virNetDevMacVLanMode'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3d13c7a3de..8fecf0566c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1130,7 +1130,7 @@ struct _virDomainNetDef { } internal; struct { char *linkdev; - int mode; /* enum virMacvtapMode from util/macvtap.h */ + virNetDevMacVLanMode mode; } direct; struct { virDomainHostdevDef def; -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 431fd7a255..77d3ef564e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8988,7 +8988,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autoptr(GHashTable) filterparams = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) virDomainChrSourceReconnectDef reconnect = {0}; - int rv, val; + int rv; g_autofree char *macaddr = NULL; g_autofree char *dev = NULL; g_autofree char *managed_tap = NULL; @@ -8998,7 +8998,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *localport = NULL; g_autofree char *model = NULL; g_autofree char *filter = NULL; - g_autofree char *mode = NULL; g_autofree char *linkstate = NULL; g_autofree char *addrtype = NULL; g_autofree char *vhostuser_mode = NULL; @@ -9090,10 +9089,18 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_DIRECT: - if (source_node) { - dev = virXMLPropString(source_node, "dev"); - mode = virXMLPropString(source_node, "mode"); - } + if (virDomainNetDefParseXMLRequireSource(def, source_node) < 0) + return NULL; + + if (!(def->data.direct.linkdev = virXMLPropStringRequired(source_node, "dev"))) + return NULL; + + if (virXMLPropEnumDefault(source_node, "mode", + virNetDevMacVLanModeTypeFromString, + VIR_XML_PROP_NONE, + &def->data.direct.mode, + VIR_NETDEV_MACVLAN_MODE_VEPA) < 0) + return NULL; break; case VIR_DOMAIN_NET_TYPE_ETHERNET: @@ -9394,28 +9401,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_INTERNAL: - break; - case VIR_DOMAIN_NET_TYPE_DIRECT: - if (dev == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'dev' attribute specified " - "with <interface type='direct'/>")); - return NULL; - } - - if (mode != NULL) { - if ((val = virNetDevMacVLanModeTypeFromString(mode)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Unknown mode has been specified")); - return NULL; - } - def->data.direct.mode = val; - } else { - def->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_VEPA; - } - - def->data.direct.linkdev = g_steal_pointer(&dev); break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: -- 2.37.1

Move the code fetching the model of the net device before the main code parsing individual device types so that the data is available before the upcoming refactor. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 77d3ef564e..35843f6b72 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9019,6 +9019,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, &def->trustGuestRxFilters) < 0) return NULL; + if ((model = virXPathString("string(./model/@type)", ctxt)) && + virDomainNetSetModelString(def, model) < 0) + return NULL; + if ((source_node = virXPathNode("./source", ctxt))) { if (virDomainNetIPInfoParseXML(_("interface host IP"), source_node, ctxt, &def->hostIP) < 0) return NULL; @@ -9202,7 +9206,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, def->script = virXPathString("string(./script/@path)", ctxt); def->downscript = virXPathString("string(./downscript/@path)", ctxt); def->domain_name = virXPathString("string(./backenddomain/@name)", ctxt); - model = virXPathString("string(./model/@type)", ctxt); if ((filterref_node = virXPathNode("./filterref", ctxt))) { filter = virXMLPropString(filterref_node, "filter"); @@ -9258,10 +9261,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - if (model != NULL && - virDomainNetSetModelString(def, model) < 0) - return NULL; - switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: break; -- 2.37.1

Move the function in place of its forward declaration. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 48 ++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 35843f6b72..dde34404f1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1481,7 +1481,27 @@ virDomainChrSourceDefFormat(virBuffer *buf, static int virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDef *def, xmlNodePtr node, - xmlXPathContextPtr ctxt); + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt) + xmlNodePtr cur; + + ctxt->node = node; + + if ((cur = virXPathNode("./reconnect", ctxt))) { + if (virXMLPropTristateBool(cur, "enabled", VIR_XML_PROP_NONE, + &def->enabled) < 0) + return -1; + + if (def->enabled == VIR_TRISTATE_BOOL_YES) { + if (virXMLPropUInt(cur, "timeout", 10, VIR_XML_PROP_REQUIRED, + &def->timeout) < 0) + return -1; + } + } + + return 0; +} static int virDomainObjOnceInit(void) @@ -8777,32 +8797,6 @@ virDomainNetAppendIPAddress(virDomainNetDef *def, } -static int -virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDef *def, - xmlNodePtr node, - xmlXPathContextPtr ctxt) -{ - VIR_XPATH_NODE_AUTORESTORE(ctxt) - xmlNodePtr cur; - - ctxt->node = node; - - if ((cur = virXPathNode("./reconnect", ctxt))) { - if (virXMLPropTristateBool(cur, "enabled", VIR_XML_PROP_NONE, - &def->enabled) < 0) - return -1; - - if (def->enabled == VIR_TRISTATE_BOOL_YES) { - if (virXMLPropUInt(cur, "timeout", 10, VIR_XML_PROP_REQUIRED, - &def->timeout) < 0) - return -1; - } - } - - return 0; -} - - static int virDomainNetTeamingInfoParseXML(xmlXPathContextPtr ctxt, virDomainNetTeamingInfo **teaming) -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 130 +++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 62 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dde34404f1..b41e1644a5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8964,6 +8964,23 @@ virDomainNetDefParseXMLRequireSource(virDomainNetDef *def, } +typedef enum { + VIR_DOMAIN_NET_VHOSTUSER_MODE_NONE, + VIR_DOMAIN_NET_VHOSTUSER_MODE_CLIENT, + VIR_DOMAIN_NET_VHOSTUSER_MODE_SERVER, + + VIR_DOMAIN_NET_VHOSTUSER_MODE_LAST +} virDomainNetVhostuserMode; + +VIR_ENUM_DECL(virDomainNetVhostuserMode); +VIR_ENUM_IMPL(virDomainNetVhostuserMode, + VIR_DOMAIN_NET_VHOSTUSER_MODE_LAST, + "", + "client", + "server", +); + + static virDomainNetDef * virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr node, @@ -8981,7 +8998,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr mac_node = NULL; g_autoptr(GHashTable) filterparams = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) - virDomainChrSourceReconnectDef reconnect = {0}; int rv; g_autofree char *macaddr = NULL; g_autofree char *dev = NULL; @@ -8994,9 +9010,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *filter = NULL; g_autofree char *linkstate = NULL; g_autofree char *addrtype = NULL; - g_autofree char *vhostuser_mode = NULL; - g_autofree char *vhostuser_path = NULL; - g_autofree char *vhostuser_type = NULL; g_autofree char *tap = NULL; const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL; @@ -9123,14 +9136,55 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } break; - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - if (source_node) { - vhostuser_type = virXMLPropString(source_node, "type"); - vhostuser_path = virXMLPropString(source_node, "path"); - vhostuser_mode = virXMLPropString(source_node, "mode"); - if (virDomainChrSourceReconnectDefParseXML(&reconnect, source_node, ctxt) < 0) - return NULL; + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: { + g_autofree char *vhostuser_type = NULL; + virDomainNetVhostuserMode vhostuser_mode; + + if (virDomainNetDefParseXMLRequireSource(def, source_node) < 0) + return NULL; + + if (!(vhostuser_type = virXMLPropStringRequired(source_node, "type"))) + return NULL; + + if (STRNEQ_NULLABLE(vhostuser_type, "unix")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Type='%s' unsupported for <interface type='vhostuser'>"), + vhostuser_type); + return NULL; + } + + if (!(def->data.vhostuser = virDomainChrSourceDefNew(xmlopt))) + return NULL; + + def->data.vhostuser->type = VIR_DOMAIN_CHR_TYPE_UNIX; + + if (!(def->data.vhostuser->data.nix.path = virXMLPropStringRequired(source_node, "path"))) + return NULL; + + if (virXMLPropEnum(source_node, "mode", + virDomainNetVhostuserModeTypeFromString, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &vhostuser_mode) < 0) + return NULL; + + switch (vhostuser_mode) { + case VIR_DOMAIN_NET_VHOSTUSER_MODE_CLIENT: + def->data.vhostuser->data.nix.listen = false; + break; + + case VIR_DOMAIN_NET_VHOSTUSER_MODE_SERVER: + def->data.vhostuser->data.nix.listen = true; + break; + + case VIR_DOMAIN_NET_VHOSTUSER_MODE_NONE: + case VIR_DOMAIN_NET_VHOSTUSER_MODE_LAST: + break; } + + if (virDomainChrSourceReconnectDefParseXML(&def->data.vhostuser->data.nix.reconnect, + source_node, ctxt) < 0) + return NULL; + } break; case VIR_DOMAIN_NET_TYPE_VDPA: @@ -9268,58 +9322,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - if (STRNEQ_NULLABLE(vhostuser_type, "unix")) { - if (vhostuser_type) - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Type='%s' unsupported for" - " <interface type='vhostuser'>"), - vhostuser_type); - else - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No <source> 'type' attribute " - "specified for <interface " - "type='vhostuser'>")); - return NULL; - } - - if (vhostuser_path == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'path' attribute " - "specified with <interface " - "type='vhostuser'/>")); - return NULL; - } - - if (vhostuser_mode == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'mode' attribute " - "specified with <interface " - "type='vhostuser'/>")); - return NULL; - } - - if (!(def->data.vhostuser = virDomainChrSourceDefNew(xmlopt))) - return NULL; - - def->data.vhostuser->type = VIR_DOMAIN_CHR_TYPE_UNIX; - def->data.vhostuser->data.nix.path = g_steal_pointer(&vhostuser_path); - - if (STREQ(vhostuser_mode, "server")) { - def->data.vhostuser->data.nix.listen = true; - if (reconnect.enabled == VIR_TRISTATE_BOOL_YES) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'reconnect' attribute unsupported " - "'server' mode for <interface type='vhostuser'>")); - return NULL; - } - } else if (STREQ(vhostuser_mode, "client")) { - def->data.vhostuser->data.nix.listen = false; - def->data.vhostuser->data.nix.reconnect = reconnect; - } else { + if (def->data.vhostuser->data.nix.listen && + def->data.vhostuser->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Wrong <source> 'mode' attribute " - "specified with <interface " - "type='vhostuser'/>")); + _("'reconnect' attribute unsupported 'server' mode for <interface type='vhostuser'>")); return NULL; } break; -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b41e1644a5..c14f3f2910 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9188,9 +9188,11 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_VDPA: - if (source_node) { - dev = virXMLPropString(source_node, "dev"); - } + if (virDomainNetDefParseXMLRequireSource(def, source_node) < 0) + return NULL; + + if (!(def->data.vdpa.devicepath = virXMLPropStringRequired(source_node, "dev"))) + return NULL; break; case VIR_DOMAIN_NET_TYPE_CLIENT: @@ -9331,15 +9333,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_VDPA: - if (dev == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'dev' attribute " - "specified with <interface type='vdpa'/>")); - return NULL; - } - def->data.vdpa.devicepath = g_steal_pointer(&dev); - break; - case VIR_DOMAIN_NET_TYPE_BRIDGE: break; -- 2.37.1

VIR_XPATH_NODE_AUTORESTORE_NAME is a more generic version of the VIR_XPATH_NODE_AUTORESTORE macro used to save the 'node' inside a XPath context struct. The new macro allows specifying the name of the variable used to save the context so that it can be used multiple times inside a function's nested scopes. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/util/virxml.h b/src/util/virxml.h index 3b00d20ea9..09a8d89f1e 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -379,6 +379,20 @@ virXPathContextNodeRestore(virXPathContextNodeSave *save); G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(virXPathContextNodeSave, virXPathContextNodeRestore); +/** + * VIR_XPATH_NODE_AUTORESTORE_NAME: + * @name: name of the temporary variable used to save @ctxt + * @ctxt: XML XPath context pointer + * + * This macro ensures that when the scope where it's used ends, @ctxt's current + * node pointer is reset to the original value when this macro was used. The + * context is saved into a variable named @name; + */ +#define VIR_XPATH_NODE_AUTORESTORE_NAME(_name, _ctxt) \ + VIR_WARNINGS_NO_UNUSED_VARIABLE \ + g_auto(virXPathContextNodeSave) _name = { .ctxt = _ctxt,\ + .node = _ctxt->node}; \ + VIR_WARNINGS_RESET /** * VIR_XPATH_NODE_AUTORESTORE: * @ctxt: XML XPath context pointer @@ -387,10 +401,7 @@ G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(virXPathContextNodeSave, virXPathContextNodeRes * node pointer is reset to the original value when this macro was used. */ #define VIR_XPATH_NODE_AUTORESTORE(_ctxt) \ - VIR_WARNINGS_NO_UNUSED_VARIABLE \ - g_auto(virXPathContextNodeSave) _ctxt ## CtxtSave = { .ctxt = _ctxt,\ - .node = _ctxt->node}; \ - VIR_WARNINGS_RESET + VIR_XPATH_NODE_AUTORESTORE_NAME(_ctxt ## CtxtSave, _ctxt) G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlDoc, xmlFreeDoc); G_DEFINE_AUTOPTR_CLEANUP_FUNC(xmlXPathContext, xmlXPathFreeContext); -- 2.37.1

This also removes the confusing use of variables named 'tmpNode' and 'tmp_node' right next to each other. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 101 +++++++++++++---------------------------- 1 file changed, 31 insertions(+), 70 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c14f3f2910..cc28bc9684 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8994,7 +8994,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr filterref_node = NULL; xmlNodePtr vlan_node = NULL; xmlNodePtr bandwidth_node = NULL; - xmlNodePtr tmpNode; xmlNodePtr mac_node = NULL; g_autoptr(GHashTable) filterparams = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) @@ -9002,10 +9001,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *macaddr = NULL; g_autofree char *dev = NULL; g_autofree char *managed_tap = NULL; - g_autofree char *address = NULL; - g_autofree char *port = NULL; - g_autofree char *localaddr = NULL; - g_autofree char *localport = NULL; g_autofree char *model = NULL; g_autofree char *filter = NULL; g_autofree char *linkstate = NULL; @@ -9199,18 +9194,38 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_MCAST: case VIR_DOMAIN_NET_TYPE_UDP: - if (source_node) { - address = virXMLPropString(source_node, "address"); - port = virXMLPropString(source_node, "port"); - if (def->type == VIR_DOMAIN_NET_TYPE_UDP) { - xmlNodePtr tmp_node = ctxt->node; - ctxt->node = source_node; - if ((tmpNode = virXPathNode("./local", ctxt))) { - localaddr = virXMLPropString(tmpNode, "address"); - localport = virXMLPropString(tmpNode, "port"); - } - ctxt->node = tmp_node; + if (virDomainNetDefParseXMLRequireSource(def, source_node) < 0) + return NULL; + + if (def->type != VIR_DOMAIN_NET_TYPE_SERVER) { + if (!(def->data.socket.address = virXMLPropStringRequired(source_node, "address"))) + return NULL; + } else { + def->data.socket.address = virXMLPropString(source_node, "address"); + } + + if (virXMLPropInt(source_node, "port", 10, VIR_XML_PROP_REQUIRED, + &def->data.socket.port, def->data.socket.port) < 0) + return NULL; + + if (def->type == VIR_DOMAIN_NET_TYPE_UDP) { + VIR_XPATH_NODE_AUTORESTORE_NAME(localCtxt, ctxt) + xmlNodePtr local_node; + + ctxt->node = source_node; + + if (!(local_node = virXPathNode("./local", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'<local>' element missing for 'udp' socket interface")); + return NULL; } + + if (!(def->data.socket.localaddr = virXMLPropStringRequired(local_node, "address"))) + return NULL; + + if (virXMLPropInt(local_node, "port", 10, VIR_XML_PROP_REQUIRED, + &def->data.socket.localport, def->data.socket.localport) < 0) + return NULL; } break; @@ -9334,64 +9349,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, case VIR_DOMAIN_NET_TYPE_VDPA: case VIR_DOMAIN_NET_TYPE_BRIDGE: - break; - case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_MCAST: case VIR_DOMAIN_NET_TYPE_UDP: - if (port == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'port' attribute " - "specified with socket interface")); - return NULL; - } - if (virStrToLong_i(port, NULL, 10, &def->data.socket.port) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <source> 'port' attribute " - "with socket interface")); - return NULL; - } - - if (address == NULL) { - if (def->type == VIR_DOMAIN_NET_TYPE_CLIENT || - def->type == VIR_DOMAIN_NET_TYPE_MCAST || - def->type == VIR_DOMAIN_NET_TYPE_UDP) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <source> 'address' attribute " - "specified with socket interface")); - return NULL; - } - } else { - def->data.socket.address = g_steal_pointer(&address); - } - - if (def->type != VIR_DOMAIN_NET_TYPE_UDP) - break; - - if (localport == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <local> 'port' attribute " - "specified with socket interface")); - return NULL; - } - if (virStrToLong_i(localport, NULL, 10, &def->data.socket.localport) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <local> 'port' attribute " - "with socket interface")); - return NULL; - } - - if (localaddr == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No <local> 'address' attribute " - "specified with socket interface")); - return NULL; - } else { - def->data.socket.localaddr = g_steal_pointer(&localaddr); - } - break; - case VIR_DOMAIN_NET_TYPE_INTERNAL: case VIR_DOMAIN_NET_TYPE_DIRECT: break; -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 44 ++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc28bc9684..3850684707 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8988,7 +8988,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, unsigned int flags) { g_autoptr(virDomainNetDef) def = NULL; - virDomainHostdevDef *hostdev; xmlNodePtr source_node = NULL; xmlNodePtr virtualport_node = NULL; xmlNodePtr filterref_node = NULL; @@ -9004,7 +9003,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *model = NULL; g_autofree char *filter = NULL; g_autofree char *linkstate = NULL; - g_autofree char *addrtype = NULL; g_autofree char *tap = NULL; const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL; @@ -9229,7 +9227,27 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } break; - case VIR_DOMAIN_NET_TYPE_HOSTDEV: + case VIR_DOMAIN_NET_TYPE_HOSTDEV: { + g_autofree char *addrtype = virXPathString("string(./source/address/@type)", ctxt); + + def->data.hostdev.def.parentnet = def; + def->data.hostdev.def.info = &def->info; + def->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; + + /* if not explicitly stated, source/vendor implies usb device */ + if (!addrtype && virXPathNode("./source/vendor", ctxt)) + addrtype = g_strdup("usb"); + + /* The helper function expects type to already be found and + * passed in as a string, since it is in a different place in + * NetDef vs HostdevDef. */ + if (virDomainHostdevDefParseXMLSubsys(node, ctxt, addrtype, + &def->data.hostdev.def, + flags, xmlopt) < 0) + return NULL; + } + break; + case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_NULL: case VIR_DOMAIN_NET_TYPE_LAST: @@ -9355,27 +9373,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, case VIR_DOMAIN_NET_TYPE_UDP: case VIR_DOMAIN_NET_TYPE_INTERNAL: case VIR_DOMAIN_NET_TYPE_DIRECT: - break; - case VIR_DOMAIN_NET_TYPE_HOSTDEV: - hostdev = &def->data.hostdev.def; - hostdev->parentnet = def; - hostdev->info = &def->info; - /* The helper function expects type to already be found and - * passed in as a string, since it is in a different place in - * NetDef vs HostdevDef. - */ - addrtype = virXPathString("string(./source/address/@type)", ctxt); - /* if not explicitly stated, source/vendor implies usb device */ - if (!addrtype && virXPathNode("./source/vendor", ctxt)) - addrtype = g_strdup("usb"); - hostdev->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; - if (virDomainHostdevDefParseXMLSubsys(node, ctxt, addrtype, - hostdev, flags, xmlopt) < 0) { - return NULL; - } - break; - case VIR_DOMAIN_NET_TYPE_VDS: case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_USER: -- 2.37.1

The moved code is pure validation of semantics of the definition and not actual parsed values. Move it to the validation code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 38 -------------------------------------- src/conf/domain_validate.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3850684707..648ce18a3f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9344,44 +9344,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - switch (def->type) { - case VIR_DOMAIN_NET_TYPE_NETWORK: - break; - - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - if (!virDomainNetIsVirtioModel(def)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Wrong or no <model> 'type' attribute " - "specified with <interface type='vhostuser'/>. " - "vhostuser requires the virtio-net* frontend")); - return NULL; - } - - if (def->data.vhostuser->data.nix.listen && - def->data.vhostuser->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'reconnect' attribute unsupported 'server' mode for <interface type='vhostuser'>")); - return NULL; - } - break; - - case VIR_DOMAIN_NET_TYPE_VDPA: - case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_CLIENT: - case VIR_DOMAIN_NET_TYPE_SERVER: - case VIR_DOMAIN_NET_TYPE_MCAST: - case VIR_DOMAIN_NET_TYPE_UDP: - case VIR_DOMAIN_NET_TYPE_INTERNAL: - case VIR_DOMAIN_NET_TYPE_DIRECT: - case VIR_DOMAIN_NET_TYPE_HOSTDEV: - case VIR_DOMAIN_NET_TYPE_VDS: - case VIR_DOMAIN_NET_TYPE_ETHERNET: - case VIR_DOMAIN_NET_TYPE_USER: - case VIR_DOMAIN_NET_TYPE_NULL: - case VIR_DOMAIN_NET_TYPE_LAST: - break; - } - if (virDomainNetIPInfoParseXML(_("guest interface"), node, ctxt, &def->guestIP) < 0) return NULL; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 6ecf6d1c11..81f6d5dbd5 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2134,6 +2134,40 @@ virDomainNetDefValidate(const virDomainNetDef *net) return -1; } + switch (net->type) { + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: + if (!virDomainNetIsVirtioModel(net)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Wrong or no <model> 'type' attribute specified with <interface type='vhostuser'/>. vhostuser requires the virtio-net* frontend")); + return -1; + } + + if (net->data.vhostuser->data.nix.listen && + net->data.vhostuser->data.nix.reconnect.enabled == VIR_TRISTATE_BOOL_YES) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'reconnect' attribute unsupported 'server' mode for <interface type='vhostuser'>")); + return -1; + } + break; + + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_VDPA: + case VIR_DOMAIN_NET_TYPE_BRIDGE: + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_MCAST: + case VIR_DOMAIN_NET_TYPE_UDP: + case VIR_DOMAIN_NET_TYPE_INTERNAL: + case VIR_DOMAIN_NET_TYPE_DIRECT: + case VIR_DOMAIN_NET_TYPE_HOSTDEV: + case VIR_DOMAIN_NET_TYPE_VDS: + case VIR_DOMAIN_NET_TYPE_ETHERNET: + case VIR_DOMAIN_NET_TYPE_USER: + case VIR_DOMAIN_NET_TYPE_NULL: + case VIR_DOMAIN_NET_TYPE_LAST: + break; + } + return 0; } -- 2.37.1

Base whether virtualport is supported for a given interface on a new variable named 'virtualport_flags' which also configures the parser for the virtualports subelement and fill it in the appropriate interface type branches. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 648ce18a3f..bee415b9d5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9004,6 +9004,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *filter = NULL; g_autofree char *linkstate = NULL; g_autofree char *tap = NULL; + unsigned int virtualport_flags = 0; const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL; if (!(def = virDomainNetDefNew(xmlopt))) @@ -9053,6 +9054,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, flags, xmlopt) < 0)) return NULL; } + + virtualport_flags = VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS; break; case VIR_DOMAIN_NET_TYPE_VDS: @@ -9090,6 +9093,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, if (!(def->data.bridge.brname = virXMLPropStringRequired(source_node, "bridge"))) return NULL; + + virtualport_flags = VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS | + VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES | + VIR_VPORT_XML_REQUIRE_TYPE; break; case VIR_DOMAIN_NET_TYPE_DIRECT: @@ -9105,6 +9112,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, &def->data.direct.mode, VIR_NETDEV_MACVLAN_MODE_VEPA) < 0) return NULL; + + virtualport_flags = VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS | + VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES | + VIR_VPORT_XML_REQUIRE_TYPE; break; case VIR_DOMAIN_NET_TYPE_ETHERNET: @@ -9245,6 +9256,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, &def->data.hostdev.def, flags, xmlopt) < 0) return NULL; + + virtualport_flags = VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS | + VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES | + VIR_VPORT_XML_REQUIRE_TYPE; } break; @@ -9255,28 +9270,16 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } if ((virtualport_node = virXPathNode("./virtualport", ctxt))) { - if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - if (!(def->virtPortProfile - = virNetDevVPortProfileParse(virtualport_node, - VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS))) { - return NULL; - } - } else if (def->type == VIR_DOMAIN_NET_TYPE_BRIDGE || - def->type == VIR_DOMAIN_NET_TYPE_DIRECT || - def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { - if (!(def->virtPortProfile - = virNetDevVPortProfileParse(virtualport_node, VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS| - VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES| - VIR_VPORT_XML_REQUIRE_TYPE))) { - return NULL; - } - } else { + if (virtualport_flags == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("<virtualport> element unsupported for" - " <interface type='%s'>"), - virDomainNetTypeToString(def->type)); + _("<virtualport> element unsupported for <interface type='%s'>"), + virDomainNetTypeToString(def->type)); return NULL; } + + if (!(def->virtPortProfile = virNetDevVPortProfileParse(virtualport_node, + virtualport_flags))) + return NULL; } def->ifname = virXPathString("string(./target/@dev)", ctxt); -- 2.37.1

Convert the strut member to proper type. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8fecf0566c..352b88eae5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1152,7 +1152,7 @@ struct _virDomainNetDef { char *downscript; char *domain_name; /* backend domain name */ char *ifname; /* interface name on the host (<target dev='x'/>) */ - int managed_tap; /* enum virTristateBool - ABSENT == YES */ + virTristateBool managed_tap; virNetDevIPInfo hostIP; char *ifname_guest_actual; char *ifname_guest; -- 2.37.1

Specifically rework of parsing of the 'managed' attribute simplifies the code greatly. Signed-off-by: Peter Krempa <pkrempa@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 bee415b9d5..a3f604ec7b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8994,12 +8994,12 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr vlan_node = NULL; xmlNodePtr bandwidth_node = NULL; xmlNodePtr mac_node = NULL; + xmlNodePtr target_node = NULL; g_autoptr(GHashTable) filterparams = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) int rv; g_autofree char *macaddr = NULL; g_autofree char *dev = NULL; - g_autofree char *managed_tap = NULL; g_autofree char *model = NULL; g_autofree char *filter = NULL; g_autofree char *linkstate = NULL; @@ -9282,8 +9282,13 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - def->ifname = virXPathString("string(./target/@dev)", ctxt); - managed_tap = virXPathString("string(./target/@managed)", ctxt); + if ((target_node = virXPathNode("./target", ctxt))) { + def->ifname = virXMLPropString(target_node, "dev"); + + if (virXMLPropTristateBool(target_node, "managed", VIR_XML_PROP_NONE, + &def->managed_tap) < 0) + return NULL; + } def->ifname_guest = virXPathString("string(./guest/@dev)", ctxt); def->ifname_guest_actual = virXPathString("string(./guest/@actual)", ctxt); @@ -9351,17 +9356,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, ctxt, &def->guestIP) < 0) return NULL; - if (managed_tap) { - bool state = false; - if (virStringParseYesNo(managed_tap, &state) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid 'managed' value '%s'"), - managed_tap); - return NULL; - } - def->managed_tap = virTristateBoolFromBool(state); - } - if (def->managed_tap != VIR_TRISTATE_BOOL_NO && def->ifname && (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && (STRPREFIX(def->ifname, VIR_NET_GENERATED_VNET_PREFIX) || -- 2.37.1

Parse the element only when the network type requires it and assign it directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 45 +++++++++++------------------------------- 1 file changed, 11 insertions(+), 34 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3f604ec7b..c680d2bcc7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8990,21 +8990,19 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autoptr(virDomainNetDef) def = NULL; xmlNodePtr source_node = NULL; xmlNodePtr virtualport_node = NULL; - xmlNodePtr filterref_node = NULL; xmlNodePtr vlan_node = NULL; xmlNodePtr bandwidth_node = NULL; xmlNodePtr mac_node = NULL; xmlNodePtr target_node = NULL; - g_autoptr(GHashTable) filterparams = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) int rv; g_autofree char *macaddr = NULL; g_autofree char *dev = NULL; g_autofree char *model = NULL; - g_autofree char *filter = NULL; g_autofree char *linkstate = NULL; g_autofree char *tap = NULL; unsigned int virtualport_flags = 0; + bool parse_filterref = false; const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL; if (!(def = virDomainNetDefNew(xmlopt))) @@ -9056,6 +9054,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } virtualport_flags = VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS; + parse_filterref = true; break; case VIR_DOMAIN_NET_TYPE_VDS: @@ -9097,6 +9096,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, virtualport_flags = VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS | VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES | VIR_VPORT_XML_REQUIRE_TYPE; + parse_filterref = true; break; case VIR_DOMAIN_NET_TYPE_DIRECT: @@ -9138,6 +9138,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, return NULL; } } + parse_filterref = true; break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: { @@ -9298,9 +9299,13 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, def->downscript = virXPathString("string(./downscript/@path)", ctxt); def->domain_name = virXPathString("string(./backenddomain/@name)", ctxt); - if ((filterref_node = virXPathNode("./filterref", ctxt))) { - filter = virXMLPropString(filterref_node, "filter"); - filterparams = virNWFilterParseParamAttributes(filterref_node); + if (parse_filterref) { + xmlNodePtr filterref_node = virXPathNode("./filterref", ctxt); + + if (filterref_node) { + def->filter = virXMLPropString(filterref_node, "filter"); + def->filterparams = virNWFilterParseParamAttributes(filterref_node); + } } if ((bandwidth_node = virXPathNode("./bandwidth", ctxt)) && @@ -9387,34 +9392,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } } - if (filter != NULL) { - switch (def->type) { - case VIR_DOMAIN_NET_TYPE_ETHERNET: - case VIR_DOMAIN_NET_TYPE_NETWORK: - case VIR_DOMAIN_NET_TYPE_BRIDGE: - def->filter = g_steal_pointer(&filter); - def->filterparams = g_steal_pointer(&filterparams); - break; - case VIR_DOMAIN_NET_TYPE_USER: - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - case VIR_DOMAIN_NET_TYPE_SERVER: - case VIR_DOMAIN_NET_TYPE_CLIENT: - case VIR_DOMAIN_NET_TYPE_MCAST: - case VIR_DOMAIN_NET_TYPE_INTERNAL: - case VIR_DOMAIN_NET_TYPE_DIRECT: - case VIR_DOMAIN_NET_TYPE_HOSTDEV: - case VIR_DOMAIN_NET_TYPE_UDP: - case VIR_DOMAIN_NET_TYPE_NULL: - case VIR_DOMAIN_NET_TYPE_VDS: - case VIR_DOMAIN_NET_TYPE_VDPA: - break; - case VIR_DOMAIN_NET_TYPE_LAST: - default: - virReportEnumRangeError(virDomainNetType, def->type); - return NULL; - } - } - if (virDomainNetTeamingInfoParseXML(ctxt, &def->teaming) < 0) return NULL; -- 2.37.1

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c680d2bcc7..2a266ac6c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8994,6 +8994,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr bandwidth_node = NULL; xmlNodePtr mac_node = NULL; xmlNodePtr target_node = NULL; + xmlNodePtr coalesce_node = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) int rv; g_autofree char *macaddr = NULL; @@ -9410,9 +9411,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - node = virXPathNode("./coalesce", ctxt); - if (node) { - if (virDomainNetDefCoalesceParseXML(node, ctxt, &def->coalesce) < 0) + if ((coalesce_node = virXPathNode("./coalesce", ctxt))) { + if (virDomainNetDefCoalesceParseXML(coalesce_node, ctxt, &def->coalesce) < 0) return NULL; } -- 2.37.1

Access the 'mac_node' variable only when it was filled. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 48 +++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a266ac6c9..d3755547c7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9321,37 +9321,37 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, if ((tap = virXPathString("string(./backend/@tap)", ctxt))) def->backend.tap = virFileSanitizePath(tap); - mac_node = virXPathNode("./mac", ctxt); + if ((mac_node = virXPathNode("./mac", ctxt))) { + if ((macaddr = virXMLPropString(mac_node, "address"))) { + if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unable to parse mac address '%s'"), + (const char *)macaddr); + return NULL; + } + if (virMacAddrIsMulticast(&def->mac)) { + virReportError(VIR_ERR_XML_ERROR, + _("expected unicast mac address, found multicast '%s'"), + (const char *)macaddr); + return NULL; + } + } - if ((macaddr = virXMLPropString(mac_node, "address"))) { - if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unable to parse mac address '%s'"), - (const char *)macaddr); + if (virXMLPropEnum(mac_node, "type", + virDomainNetMacTypeTypeFromString, + VIR_XML_PROP_NONZERO, &def->mac_type) < 0) return NULL; - } - if (virMacAddrIsMulticast(&def->mac)) { - virReportError(VIR_ERR_XML_ERROR, - _("expected unicast mac address, found multicast '%s'"), - (const char *)macaddr); + + if (virXMLPropTristateBool(mac_node, "check", VIR_XML_PROP_NONE, + &def->mac_check) < 0) return NULL; - } - } else { + } + + if (!macaddr) { virDomainNetGenerateMAC(xmlopt, &def->mac); def->mac_generated = true; } - if (virXMLPropEnum(mac_node, "type", - virDomainNetMacTypeTypeFromString, - VIR_XML_PROP_NONZERO, - &def->mac_type) < 0) - return NULL; - - if (virXMLPropTristateBool(mac_node, "check", - VIR_XML_PROP_NONE, - &def->mac_check) < 0) - return NULL; - if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT | VIR_DOMAIN_DEF_PARSE_ALLOW_ROM) < 0) { -- 2.37.1

Apart from it being a long time ago the 'openvz' driver is also rarely used. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3755547c7..b7be1bae06 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8998,7 +8998,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, VIR_XPATH_NODE_AUTORESTORE(ctxt) int rv; g_autofree char *macaddr = NULL; - g_autofree char *dev = NULL; g_autofree char *model = NULL; g_autofree char *linkstate = NULL; g_autofree char *tap = NULL; @@ -9120,25 +9119,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, break; case VIR_DOMAIN_NET_TYPE_ETHERNET: - /* This clause is only necessary because from 2010 to 2016 it was - * possible (but never documented) to configure the name of the - * guest-side interface of an openvz domain with <source dev='blah'/>. - * That was blatant misuse of <source>, so was likely (hopefully) - * never used, but just in case there was somebody using it, we - * need to generate an error. If the openvz driver is ever - * deprecated, this clause can be removed from here. - */ - if (source_node) { - if ((dev = virXMLPropString(source_node, "dev"))) { - virReportError(VIR_ERR_XML_ERROR, - _("Invalid attempt to set <interface type='ethernet'> " - "device name with <source dev='%s'/>. " - "Use <target dev='%s'/> (for host-side) " - "or <guest dev='%s'/> (for guest-side) instead."), - dev, dev, dev); - return NULL; - } - } parse_filterref = true; break; -- 2.37.1