[libvirt] [PATCH v5 02/24] conf: introduce virNetworkPortDefPtr struct and XML support
Laine Stump
laine at laine.org
Tue May 21 01:44:04 UTC 2019
On 5/14/19 11:48 AM, Daniel P. Berrangé wrote:
> Introduce a virNetworkPortDefPtr struct to represent the data associated
> with a virtual network port. Add APIs for parsing/formatting XML docs
> with the data.
>
> Signed-off-by: Daniel P. Berrangé<berrange at redhat.com>
> ---
> docs/docs.html.in | 1 +
> docs/formatnetworkport.html.in | 212 ++++++++
> docs/schemas/networkport.rng | 165 ++++++
> src/conf/Makefile.inc.am | 2 +
> src/conf/virnetworkportdef.c | 509 ++++++++++++++++++
> src/conf/virnetworkportdef.h | 113 ++++
> src/libvirt_private.syms | 10 +
> tests/Makefile.am | 7 +
> .../plug-bridge-mactbl.xml | 9 +
> .../virnetworkportxml2xmldata/plug-bridge.xml | 15 +
> .../virnetworkportxml2xmldata/plug-direct.xml | 12 +
> .../plug-hostdev-pci.xml | 12 +
> .../plug-network.xml | 16 +
> tests/virnetworkportxml2xmldata/plug-none.xml | 8 +
> tests/virnetworkportxml2xmltest.c | 104 ++++
> tests/virschematest.c | 1 +
> 16 files changed, 1196 insertions(+)
> create mode 100644 docs/formatnetworkport.html.in
> create mode 100644 docs/schemas/networkport.rng
> create mode 100644 src/conf/virnetworkportdef.c
> create mode 100644 src/conf/virnetworkportdef.h
> create mode 100644 tests/virnetworkportxml2xmldata/plug-bridge-mactbl.xml
> create mode 100644 tests/virnetworkportxml2xmldata/plug-bridge.xml
> create mode 100644 tests/virnetworkportxml2xmldata/plug-direct.xml
> create mode 100644 tests/virnetworkportxml2xmldata/plug-hostdev-pci.xml
> create mode 100644 tests/virnetworkportxml2xmldata/plug-network.xml
> create mode 100644 tests/virnetworkportxml2xmldata/plug-none.xml
> create mode 100644 tests/virnetworkportxml2xmltest.c
>
> diff --git a/docs/docs.html.in b/docs/docs.html.in
> index d0ff844d0c..c8674e1457 100644
> --- a/docs/docs.html.in
> +++ b/docs/docs.html.in
> @@ -72,6 +72,7 @@
> <dd>Description of the XML schemas for
> <a href="formatdomain.html">domains</a>,
> <a href="formatnetwork.html">networks</a>,
> + <a href="formatnetworkport.html">network ports</a>,
> <a href="formatnwfilter.html">network filtering</a>,
> <a href="formatstorage.html">storage</a>,
> <a href="formatstorageencryption.html">storage encryption</a>,
> diff --git a/docs/formatnetworkport.html.in b/docs/formatnetworkport.html.in
> new file mode 100644
> index 0000000000..0211518a6a
> --- /dev/null
> +++ b/docs/formatnetworkport.html.in
> @@ -0,0 +1,212 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!DOCTYPE html>
> +<html xmlns="http://www.w3.org/1999/xhtml">
> + <body>
> + <h1>Network XML format</h1>
> +
> + <ul id="toc">
> + </ul>
> +
> + <p>
> + This page provides an introduction to the network port XML format.
> + This stores information about the connection between an virtual
> + interface on a virtual domain's, and the virtual network it is
> + attached to.
> + </p>
> +
> + <h2><a id="elements">Element and attribute overview</a></h2>
> +
> + <p>
> + The root element required for all virtual network ports is
> + named <code>networkport</code> and has no configurable attributes
> + The network port XML format is available <span class="since">since
> + 5.4.0</span>
> + </p>
> +
> + <h3><a id="elementsMetadata">General metadata</a></h3>
> +
> + <p>
> + The first elements provide basic metadata about the virtual
> + network port.
> + </p>
> +
> + <pre>
> +<networkport
> + <uuid>7ae63b5f-fe96-4af0-a7c3-da04ba1b3f54</uuid>
> + <owner>
> + <uuid>06578fc1-c686-46fa-bc2c-220893b466a6</uuid>
> + <name>myguest<name>
> + </owner>
> + <group>webfront<group>
> + <mac address='52:54:0:7b:35:93'/>
> + ...</pre>
> +
> + <dl>
> + <dt><code>uuid</code></dt>
> + <dd>The content of the <code>uuid</code> element provides
> + a globally unique identifier for the virtual network port.
> + The format must be RFC 4122 compliant, eg <code>3e3fce45-4f53-4fa7-bb32-11f34168b82b</code>.
> + If omitted when defining/creating a new network port, a random
> + UUID is generated.</dd>
> + <dd>The <code>owner</code> node records the domain object that
> + is the owner of the network port. It contains two child nodes:
> + <dl>
> + <dt><code>uuid</code></dt>
> + <dd>The content of the <code>uuid</code> element provides
> + a globally unique identifier for the virtual domain.</dd>
> + <dt><code>name</code></dt>
> + <dd>The unique name of the virtual domain</dd>
> + </dl>
> + </dd>
> + <dt><code>group</code></dt>
> + <dd>The port group in the virtual network to which the
> + port belongs. Can be omitted if no port groups are
> + defined on the network.</dd>
> + <dt><code>mac</code></dt>
> + <dd>The <code>address</code> attribute provides the MAC
> + address of the virtual port that will be see by the
> + guest. The MAC address must not start with 0xFE as this
> + byte is reserved for use on the host side of the port.
> + </dd>
> + </dl>
> +
> + <h3><a id="elementsCommon">Common elements</a></h3>
> +
> + <p>
> + The following elements are common to one of more of the plug
> + types listed later
> + </p>
> +
> + <pre>
> + ...
> + <bandwidth>
> + <inbound average='1000' peak='5000' floor='200' burst='1024'/>
> + <outbound average='128' peak='256' burst='256'/>
> + </bandwidth>
> + <rxfilters trustGuest='yes'/>
> + <virtualport type='802.1Qbg'>
> + <parameters managerid='11' typeid='1193047' typeidversion='2'/>
> + </virtualport>
> + ...</pre>
> +
> + <dl>
> + <dt><code>bandwidth</code></dt>
> + <dd>This part of the network port XML provides setting quality of service.
> + Incoming and outgoing traffic can be shaped independently.
> + The <code>bandwidth</code> element and its child elements are described
> + in the <a href="formatnetwork.html#elementQoS">QoS</a> section of
> + the Network XML. In addition the <code>classID</code> attribute may
> + exist provide the ID of the traffic shaping class that is active.
> + </dd>
> + <dt><code>rxfilters</code></dt>
> + <dd>The <code>rxfilters</code> element property
> + <code>trustGuestRxFilters</code> provides the
Earlier you referred to this as "trustGuest" (also looks like that's
what the parser/formatter use)
> + capability for the host to detect and trust reports from the
> + guest regarding changes to the interface mac address and receive
> + filters by setting the attribute to <code>yes</code>. The default
> + setting for the attribute is <code>no</code> for security
> + reasons and support depends on the guest network device model as
> + well as the type of connection on the host - currently it is
> + only supported for the virtio device model and for macvtap
> + connections on the host.
> + </dd>
> + <dt><code>virtualport</code></dt>
> + <dd>The <code>virtualport</code> element describes metadata that
> + needs to be provided to the underlying network subsystem. It
> + is described in the domain XML
> + <a href="formatdomain.html#elementsNICS">interface documentation</a>.
> + </dd>
> + </dl>
> +
> +
> + <h3><a id="elementsPlug">Plugs</a></h3>
> +
> + <p>
> + The <code>plug</code> element has varying content depending
> + on the value of the <code>type</code> attribute.
> + </p>
> +
> + <h4><a id="elementsPlugNetwork">Network</a></h4>
> +
> + <p>
> + The <code>network</code> plug type refers to a managed virtual
> + network plug that is based on a traditional software bridge
> + device privately managed by libvirt.
> + </p>
Your patches essentially take the same items that are in the
virActualNetDef and put them into virNetworkPortDef. I think this is the
right way to approach the task of changing the infrastructure - makes it
more straightforward to get all the same functionality working with the
new intra-driver communication method. The one thing that bothers me,
though, is that this ends up replicating design mistakes that we ("I"
:-P - most of it was good, I just added in some things that were kind of
screwed up...) made with the original. I'm hoping that there will also
be a path to correcting those mistakes after the fact so that we don't
have to live with them forever.
In particular, we've been using "type='network' to imply that the
network supports a bandwidth floor setting (and who knows what else).
Although we made this mistake with <interface> status, I would really
prefer if we don't (permanently at least) propagate the mistake to
networkport. What "type='network' really means is a tap device connected
to a bridge that probably has a routed connection and supports bandwidth
floor setting). But what if network port was instead something like:
<plug type='tap' forward='route' bridge='virbr0'/>
<plug type='tap' forward='bridge' bridge='br0'/>
<plug type='direct' dev='ens3' mode='vepa'/>
(maybe if we use "type='tap'", we should also use "type='macvtap'"?.
Hmm, and of course if we're going to spell out "tap", then for
container-ish connections that use a veth pair, we would have to say
"type='veth'"...)
(in the future if networkport was used for what we call
"type='ethernet'" in <interface>, it would maybe look like <plug
type='tap' forward='unknown'/> (with no bridge attribute). Or something
like that.)
The idea really is to deconstruct all of the attributes implicit in
"type='network'" (and "type='bridge'", etc) into several independent
attributes that do a better job of correctly describing the connection).
Also, I'm thinking to a time in the future when the network driver might
create and connect the device on behalf of an unprivileged hypervisor.
The above could be extended to:
<plug type='tap' tap='vnet0' forward='route' bridge='virbr0'/>
etc.
Anyway, I haven't thought this all through to the logical end, and
there's definitely holes in it, but I'm concerned of repeating the
mistake of trying to use a single attribute to convey multiple pieces of
information when we have the chance to rethink it and get it right.
Anyway, like I said, I think it *is* better to change the infrastructure
without changing the attributes right now, but will we be able to change
it in the future?
+
> + <pre>
> + ...
> + <plug type='network' bridge='virbr0'>
> + ...</pre>
> +
> + <p>
> + The <code>bridge</code> attribute provides the name of the
> + privately managed bridge device associated with the virtual
> + network.
> + </p>
> +
> + <h4><a id="elementsPlugNetwork">Bridge</a></h4>
> +
> + <p>
> + The <code>bridge</code> plug type refers to an externally
> + managed traditional software bridge.
> + </p>
> +
> + <pre>
> + ...
> + <plug type='bridge' bridge='br2'>
> + ...</pre>
> +
> + <p>
> + The <code>bridge</code> attribute provides the name of the
> + externally managed bridge device associated with the virtual
> + network.
> + </p>
> +
> + <h4><a id="elementsPlugNetwork">Direct</a></h4>
> +
> + <p>
> + The <code>direct</code> plug type refers to an connection
"a connection"
> + directly to a physical network interface.
> + </p>
> +
> + <pre>
> + ...
> + <plug type='direct' dev='ens3' mode='vepa'/>
> + ...</pre>
> +
> + <p>
> + The <code>dev</code> attribute provides the name of the
> + physical network interface to which the port will be
> + connected. The <code>mode</code> attribute describes
> + how the connection will be setup and takes the same
> + values described in the
> + <a href="formatdomain.html#elementsNICSDirect">domain XML</a>.
> + </p>
> +
> + <h4><a id="elementsPlugNetwork">Host PCI</a></h4>
> +
> + <p>
> + The <code>hostdev-pci</code> plug type refers to the
> + passthrough of a physical PCI device rather than emulation.
> + </p>
> +
> + <pre>
> + ...
> + <plug type='hostdev-pci' managed='yes'>
> + <driver name='vfio'/>
> + <address domain='0x0001' bus='0x02' slot='0x03' function='0x4'/>
> + </plug>
> + ...</pre>
> +
> + <p>
> + The <code>managed</code> attribute indicates who is responsible for
> + managing the PCI device in the host. When set to the value <code>yes</code>
> + libvirt is responsible for automatically detaching the device from host
> + drivers and resetting it if needed. If the value is <code>no</code>,
> + some other party must ensure the device is not attached to any
> + host drivers.
Actually it specifically must be bound to the vfio-pci driver (legacy
KVM assignment, using the pcistub driver) has been deprecated since
kernel 3.1.x days, and completely removed in RHEL kernels (at least,
maybe also upstream) since RHEL7.0. But I'm not going to be persnickety
about details in the docs :-)
> + </p>
> +
> + </body>
> +</html>
> diff --git a/docs/schemas/networkport.rng b/docs/schemas/networkport.rng
> new file mode 100644
> index 0000000000..8192f6efc4
> --- /dev/null
> +++ b/docs/schemas/networkport.rng
> @@ -0,0 +1,165 @@
> +<?xml version="1.0"?>
> +<!-- A Relax NG schema for the libvirt network port XML format -->
> +<grammar xmlns="http://relaxng.org/ns/structure/1.0"
> + datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes">
> + <include href='basictypes.rng'/>
> + <include href='networkcommon.rng'/>
> +
> + <start>
> + <ref name="networkport"/>
> + </start>
> +
> + <define name="networkport">
> + <element name="networkport">
> + <interleave>
> + <element name="uuid">
> + <ref name="UUID"/>
> + </element>
> + <ref name="owner"/>
> + <ref name="mac"/>
> + <optional>
> + <ref name="group"/>
> + </optional>
> + <optional>
> + <ref name="class"/>
> + </optional>
I thought in the previous patch you had integrated <class id='blah'/>
into the bandwidth element as a classID attribute. Did you just forget
to remove it from the rng?
> + <optional>
> + <ref name="rxfilters"/>
> + </optional>
> + <optional>
> + <ref name="virtualPortProfile"/>
> + </optional>
> + <optional>
> + <ref name="bandwidth"/>
> + </optional>
> + <optional>
> + <ref name="plug"/>
> + </optional>
> + </interleave>
> + </element>
> + </define>
> +
> + <define name="owner">
> + <element name="owner">
> + <element name="name">
> + <text/>
> + </element>
> + <element name="uuid">
> + <ref name="UUID"/>
> + </element>
> + </element>
> + </define>
> +
> + <define name="mac">
> + <element name="mac">
> + <attribute name="address">
> + <ref name="uniMacAddr"/>
> + </attribute>
> + <empty/>
> + </element>
> + </define>
> +
> + <define name="group">
> + <element name="group">
> + <ref name="deviceName"/>
> + </element>
> + </define>
> +
> + <define name="class">
> + <element name="class">
> + <attribute name="id">
> + <ref name="positiveInteger"/>
> + </attribute>
> + </element>
> + </define>
> +
> + <define name="rxfilters">
> + <element name="rxfilters">
> + <attribute name="trustGuest">
> + <ref name="virYesNo"/>
> + </attribute>
> + </element>
> + </define>
> +
> + <define name="plug">
> + <element name="plug">
> + <choice>
> + <ref name="plugnetwork"/>
> + <ref name="plugbridge"/>
> + <ref name="plugdirect"/>
> + <ref name="plughostdevpci"/>
> + </choice>
> + </element>
> + </define>
> +
> + <define name="plugnetwork">
> + <attribute name="type">
> + <value>network</value>
> + </attribute>
> + <attribute name="bridge">
> + <ref name="deviceName"/>
> + </attribute>
> + <optional>
> + <attribute name="macTableManager">
> + <ref name="macTableManager"/>
> + </attribute>
> + </optional>
> + </define>
> +
> + <define name="plugbridge">
> + <attribute name="type">
> + <value>bridge</value>
> + </attribute>
> + <attribute name="bridge">
> + <ref name="deviceName"/>
> + </attribute>
> + <optional>
> + <attribute name="macTableManager">
> + <ref name="macTableManager"/>
> + </attribute>
> + </optional>
> + </define>
> +
> + <define name="plugdirect">
> + <attribute name="type">
> + <value>direct</value>
> + </attribute>
> + <attribute name="dev">
> + <ref name="deviceName"/>
> + </attribute>
> + <attribute name="mode">
> + <choice>
> + <value>bridge</value>
> + <value>passthrough</value>
> + <value>private</value>
> + <value>vepa</value>
> + </choice>
> + </attribute>
> + </define>
> +
> + <define name="plughostdevpci">
> + <attribute name="type">
> + <value>hostdev-pci</value>
> + </attribute>
> + <optional>
> + <attribute name="managed">
> + <ref name="virYesNo"/>
> + </attribute>
> + </optional>
> + <optional>
> + <element name="driver">
> + <attribute name="name">
> + <choice>
> + <value>kvm</value>
We *really* need to get rid of <driver name='kvm'/> for hostdev - it's
been deprecated for 5 years, and removing it would eliminate ugly code.
(I suppose since this is distinct from the <driver name='blah'/> in
<hostdev> or <interface type='hostdev'> you could safely leave it out.
For that matter, since the only driver supported is vfio, the entire
<driver> element is unneeded - maybe we don't need it in networkport at
all?)
(I remember when we added it, we didn't really want to (specifically
because it could mean supporting it essentially forever), but there were
some people concerned that we might have to fallback to legacy KVM
assignment if VFIO was found inadequate in some way. Turns out their
fears were unfounded (although it could easily have gone the other way :-))
> + <value>vfio</value>
> + </choice>
> + </attribute>
> + <empty/>
> + </element>
> + </optional>
> + <element name='address'>
> + <ref name="pciaddress"/>
> + </element>
> + </define>
> +
> +</grammar>
> diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am
> index 3e9fdd1aea..6b52ba674b 100644
> --- a/src/conf/Makefile.inc.am
> +++ b/src/conf/Makefile.inc.am
> @@ -7,6 +7,8 @@ NETDEV_CONF_SOURCES = \
> conf/netdev_vport_profile_conf.c \
> conf/netdev_vlan_conf.h \
> conf/netdev_vlan_conf.c \
> + conf/virnetworkportdef.h \
> + conf/virnetworkportdef.c \
> $(NULL)
>
> DOMAIN_CONF_SOURCES = \
> diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c
> new file mode 100644
> index 0000000000..6c896968ce
> --- /dev/null
> +++ b/src/conf/virnetworkportdef.c
> @@ -0,0 +1,509 @@
> +/*
> + * virnetworkportdef.c: network port XML processing
> + *
> + * Copyright (C) 2018 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + *<http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +
> +#include "viralloc.h"
> +#include "virerror.h"
> +#include "virstring.h"
> +#include "virfile.h"
> +#include "virnetworkportdef.h"
> +#include "network_conf.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NETWORK
> +
> +VIR_ENUM_IMPL(virNetworkPortPlug,
> + VIR_NETWORK_PORT_PLUG_TYPE_LAST,
> + "none", "network", "bridge", "direct", "hostdev-pci");
> +
> +void
> +virNetworkPortDefFree(virNetworkPortDefPtr def)
> +{
> + if (!def)
> + return;
> +
> + VIR_FREE(def->ownername);
> + VIR_FREE(def->group);
> +
> + virNetDevBandwidthFree(def->bandwidth);
> + virNetDevVlanClear(&def->vlan);
> + VIR_FREE(def->virtPortProfile);
> +
> + switch ((virNetworkPortPlugType)def->plugtype) {
> + case VIR_NETWORK_PORT_PLUG_TYPE_NONE:
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_NETWORK:
> + case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE:
> + VIR_FREE(def->plug.bridge.brname);
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT:
> + VIR_FREE(def->plug.direct.linkdev);
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI:
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_LAST:
> + default:
> + break;
> + }
> +
> + VIR_FREE(def);
> +}
> +
> +
> +
> +static virNetworkPortDefPtr
> +virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
> +{
> + virNetworkPortDefPtr def;
> + char *uuid = NULL;
> + xmlNodePtr virtPortNode;
> + xmlNodePtr vlanNode;
> + xmlNodePtr bandwidthNode;
> + xmlNodePtr addressNode;
> + char *trustGuestRxFilters = NULL;
> + char *mac = NULL;
> + char *macmgr = NULL;
> + char *mode = NULL;
> + char *plugtype = NULL;
> + char *managed = NULL;
> + char *driver = NULL;
> + char *class_id = NULL;
> +
> + if (VIR_ALLOC(def) < 0)
> + return NULL;
> +
> + uuid = virXPathString("string(./uuid)", ctxt);
> + if (!uuid) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("network port has no uuid"));
> + goto error;
> + }
> + if (virUUIDParse(uuid, def->uuid) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to parse UUID '%s'"), uuid);
> + goto error;
> + }
> +
> + def->ownername = virXPathString("string(./owner/name)", ctxt);
> + if (!def->ownername) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("network port has no owner name"));
> + goto error;
> + }
I'm curious why you made name and uuid subelements of owner, rather than
attributes. Just for consistency with the name element we use in other
places? Or are you expecting it might need qualifying attributes? Or
just because the <owner> line would be too long if they were attributes?
> +
> + VIR_FREE(uuid);
> + uuid = virXPathString("string(./owner/uuid)", ctxt);
> + if (!uuid) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("network port has no owner UUID"));
> + goto error;
> + }
> +
> + if (virUUIDParse(uuid, def->owneruuid) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to parse UUID '%s'"), uuid);
> + goto error;
> + }
> +
> + def->group = virXPathString("string(./group)", ctxt);
> +
> + virtPortNode = virXPathNode("./virtualport", ctxt);
> + if (virtPortNode &&
> + (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode, 0)))) {
> + goto error;
> + }
> +
> + mac = virXPathString("string(./mac/@address)", ctxt);
> + if (!mac) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("network port has no mac"));
> + goto error;
> + }
> + if (virMacAddrParse(mac, &def->mac) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to parse MAC '%s'"), mac);
> + goto error;
> + }
> +
> + bandwidthNode = virXPathNode("./bandwidth", ctxt);
> + /*
> + * We don't know if the port will allow the "floor" param or
> + * not at this stage, so we must just tell virNetDevBandwidthParse
> + * to allow it regardless. Any bad config must be reported at
> + * time of use instead.
> + */
(BTW, in case it got lost in other stuff, I agree with you that we
should just always allow specifying floor in the XML, and only give an
error if it turns out it's not supported at runtime.)
> + if (bandwidthNode &&
> + virNetDevBandwidthParse(&def->bandwidth, &def->class_id,
> + bandwidthNode, true) < 0)
> + goto error;
> +
> + vlanNode = virXPathNode("./vlan", ctxt);
> + if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0)
> + goto error;
> +
> +
> + trustGuestRxFilters
> + = virXPathString("string(./rxfilters/@trustGuest)", ctxt);
> + if (trustGuestRxFilters) {
> + if ((def->trustGuestRxFilters
> + = virTristateBoolTypeFromString(trustGuestRxFilters)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid guest rx filters trust setting '%s' "),
> + trustGuestRxFilters);
> + goto error;
> + }
> + }
> +
> + plugtype = virXPathString("string(./plug/@type)", ctxt);
> +
> + if (plugtype &&
> + (def->plugtype = virNetworkPortPlugTypeFromString(plugtype)) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid network prt plug type '%s'"), plugtype);
> + }
> +
> + switch (def->plugtype) {
> + case VIR_NETWORK_PORT_PLUG_TYPE_NONE:
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_NETWORK:
> + case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE:
> + if (!(def->plug.bridge.brname = virXPathString("string(./plug/@bridge)", ctxt))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing network port bridge name"));
> + goto error;
> + }
> + macmgr = virXPathString("string(./plug/@macTableManager)", ctxt);
> + if (macmgr &&
> + (def->plug.bridge.macTableManager =
> + virNetworkBridgeMACTableManagerTypeFromString(macmgr)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid macTableManager setting '%s' "
> + "in network port"), macmgr);
> + goto error;
> + }
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT:
> + if (!(def->plug.direct.linkdev = virXPathString("string(./plug/@dev)", ctxt))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing network port link device name"));
> + goto error;
> + }
> + mode = virXPathString("string(./plug/@mode)", ctxt);
> + if (mode &&
> + (def->plug.direct.mode =
> + virNetDevMacVLanModeTypeFromString(mode)) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid mode setting '%s' in network port"), mode);
> + goto error;
> + }
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI:
> + managed = virXPathString("string(./plug/@managed)", ctxt);
> + if (managed &&
> + (def->plug.hostdevpci.managed =
> + virTristateBoolTypeFromString(managed)) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid managed setting '%s' in network port"), mode);
> + goto error;
> + }
> + driver = virXPathString("string(./plug/driver/@name)", ctxt);
> + if (driver &&
> + (def->plug.hostdevpci.driver =
> + virNetworkForwardDriverNameTypeFromString(driver)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing network port driver name"));
> + goto error;
> + }
Yeah, the more I think about this, the more I think it's unnecessary
(and that we should also consider removing it from other places it's
used, since non-VFIO device assignment hasn't worked in Linux since
RHEL6 days).
> + if (!(addressNode = virXPathNode("./plug/address", ctxt))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing network port PCI address"));
> + goto error;
> + }
> +
> + if (virPCIDeviceAddressParseXML(addressNode, &def->plug.hostdevpci.addr) < 0)
> + goto error;
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_LAST:
> + default:
> + virReportEnumRangeError(virNetworkPortPlugType, def->plugtype);
> + goto error;
> + }
> +
> + cleanup:
> + VIR_FREE(class_id);
> + VIR_FREE(uuid);
> + VIR_FREE(plugtype);
> + VIR_FREE(mac);
> + VIR_FREE(mode);
> + VIR_FREE(macmgr);
> + VIR_FREE(driver);
> + VIR_FREE(managed);
> + return def;
> +
> + error:
> + virNetworkPortDefFree(def);
> + def = NULL;
> + goto cleanup;
> +}
> +
> +
> +virNetworkPortDefPtr
> +virNetworkPortDefParseNode(xmlDocPtr xml,
> + xmlNodePtr root)
> +{
> + xmlXPathContextPtr ctxt = NULL;
> + virNetworkPortDefPtr def = NULL;
> +
> + if (STRNEQ((const char *)root->name, "networkport")) {
> + virReportError(VIR_ERR_XML_ERROR,
> + "%s",
> + _("unknown root element for network port"));
> + goto cleanup;
> + }
> +
> + ctxt = xmlXPathNewContext(xml);
> + if (ctxt == NULL) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + ctxt->node = root;
> + def = virNetworkPortDefParseXML(ctxt);
> +
> + cleanup:
> + xmlXPathFreeContext(ctxt);
> + return def;
> +}
> +
> +
> +static virNetworkPortDefPtr
> +virNetworkPortDefParse(const char *xmlStr,
> + const char *filename)
> +{
> + virNetworkPortDefPtr def = NULL;
> + xmlDocPtr xml;
> +
> + if ((xml = virXMLParse(filename, xmlStr, _("(networkport_definition)")))) {
> + def = virNetworkPortDefParseNode(xml, xmlDocGetRootElement(xml));
> + xmlFreeDoc(xml);
> + }
> +
> + return def;
> +}
> +
> +
> +virNetworkPortDefPtr
> +virNetworkPortDefParseString(const char *xmlStr)
> +{
> + return virNetworkPortDefParse(xmlStr, NULL);
> +}
> +
> +
> +virNetworkPortDefPtr
> +virNetworkPortDefParseFile(const char *filename)
> +{
> + return virNetworkPortDefParse(NULL, filename);
> +}
> +
> +
> +char *
> +virNetworkPortDefFormat(const virNetworkPortDef *def)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + if (virNetworkPortDefFormatBuf(&buf, def) < 0) {
> + virBufferFreeAndReset(&buf);
> + return NULL;
> + }
> +
> + if (virBufferCheckError(&buf) < 0)
> + return NULL;
> +
> + return virBufferContentAndReset(&buf);
> +}
> +
So much boilerplate... :-P
> +
> +int
> +virNetworkPortDefFormatBuf(virBufferPtr buf,
> + const virNetworkPortDef *def)
> +{
> + char uuid[VIR_UUID_STRING_BUFLEN];
> + char macaddr[VIR_MAC_STRING_BUFLEN];
> +
> + virBufferAddLit(buf, "<networkport>\n");
> +
> + virBufferAdjustIndent(buf, 2);
> +
> + virUUIDFormat(def->uuid, uuid);
> + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuid);
> +
> + virBufferAddLit(buf, "<owner>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferEscapeString(buf, "<name>%s</name>\n", def->ownername);
> + virUUIDFormat(def->owneruuid, uuid);
> + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuid);
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</owner>\n");
> +
> + if (def->group)
> + virBufferEscapeString(buf, "<group>%s</group>\n", def->group);
Because virBufferEscapeString() outputs nothing if the 2nd arg is NULL,
it's not necessary to have the "if (def->group)". (I've never been
totally comfortable with that behavior, but it's being used in a lot of
places in the code, so if it ever changed then lots of things would also
break, so I think it's safe to rely on it :-))
> +
> + virMacAddrFormat(&def->mac, macaddr);
> + virBufferAsprintf(buf, "<mac address='%s'/>\n", macaddr);
> +
> + if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
> + return -1;
> + if (def->bandwidth)
> + virNetDevBandwidthFormat(def->bandwidth, def->class_id, buf);
> + if (virNetDevVlanFormat(&def->vlan, buf) < 0)
> + return -1;
> + if (def->trustGuestRxFilters)
> + virBufferAsprintf(buf, "<rxfilters trustGuest='%s'/>\n",
> + virTristateBoolTypeToString(def->trustGuestRxFilters));
> +
> + if (def->plugtype != VIR_NETWORK_PORT_PLUG_TYPE_NONE) {
> + virBufferAsprintf(buf, "<plug type='%s'",
> + virNetworkPortPlugTypeToString(def->plugtype));
> +
> + switch (def->plugtype) {
> + case VIR_NETWORK_PORT_PLUG_TYPE_NONE:
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_NETWORK:
> + case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE:
> + virBufferEscapeString(buf, " bridge='%s'", def->plug.bridge.brname);
> + if (def->plug.bridge.macTableManager)
> + virBufferAsprintf(buf, " macTableManager='%s'",
> + virNetworkBridgeMACTableManagerTypeToString(
> + def->plug.bridge.macTableManager));
> + virBufferAddLit(buf, "/>\n");
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT:
> + virBufferEscapeString(buf, " dev='%s'", def->plug.direct.linkdev);
> + virBufferAsprintf(buf, " mode='%s'",
> + virNetDevMacVLanModeTypeToString(
> + def->plug.direct.mode));
> + virBufferAddLit(buf, "/>\n");
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI:
> + virBufferAsprintf(buf, " managed='%s'>\n",
> + def->plug.hostdevpci.managed ? "yes" : "no");
> + virBufferAdjustIndent(buf, 2);
> + if (def->plug.hostdevpci.driver)
> + virBufferEscapeString(buf, "<driver name='%s'/>\n",
> + virNetworkForwardDriverNameTypeToString(
> + def->plug.hostdevpci.driver));
> +
> + virPCIDeviceAddressFormat(buf, def->plug.hostdevpci.addr, false);
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</plug>\n");
> + break;
> +
> + case VIR_NETWORK_PORT_PLUG_TYPE_LAST:
> + default:
> + virReportEnumRangeError(virNetworkPortPlugType, def->plugtype);
> + return -1;
> + }
> + }
> +
> +
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</networkport>\n");
> +
> + return 0;
> +}
> +
> +
> +static char *
> +virNetworkPortDefConfigFile(const char *dir,
> + const char *name)
> +{
> + char *ret = NULL;
> +
> + ignore_value(virAsprintf(&ret, "%s/%s.xml", dir, name));
> + return ret;
> +}
> +
> +
> +int
> +virNetworkPortDefSaveStatus(virNetworkPortDef *def,
> + const char *dir)
> +{
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + char *path;
> + char *xml = NULL;
> + int ret = -1;
> +
> + virUUIDFormat(def->uuid, uuidstr);
> +
> + if (virFileMakePath(dir) < 0)
> + goto cleanup;
> +
> + if (!(path = virNetworkPortDefConfigFile(dir, uuidstr)))
> + goto cleanup;
> +
> + if (!(xml = virNetworkPortDefFormat(def)))
> + goto cleanup;
> +
> + if (virXMLSaveFile(path, uuidstr, "net-port-create", xml) < 0)
> + goto cleanup;
For other objects, when they are saved to a "status" file, the xml is
enclosed in a "<blahstatus>" element, e.g. <domstatus>, <networkstatus>,
etc. I guess this is done in case there are other things about the state
of the object that aren't contained directly in virBlahDef. Did you not
do that here on purpose (maybe because the entire point of this object
is to keep track of the state of a particular connection, so of course
there is nothing extra?), or was it an oversite?
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(xml);
> + VIR_FREE(path);
> + return ret;
> +}
> +
> +
> +int
> +virNetworkPortDefDeleteStatus(virNetworkPortDef *def,
> + const char *dir)
> +{
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + char *path;
> + int ret = -1;
> +
> + virUUIDFormat(def->uuid, uuidstr);
> +
> + if (!(path = virNetworkPortDefConfigFile(dir, uuidstr)))
> + goto cleanup;
> +
> + if (unlink(path) < 0 && errno != ENOENT) {
> + virReportSystemError(errno,
> + _("Unable to delete %s"), path);
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(path);
> + return ret;
> +}
> diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h
> new file mode 100644
> index 0000000000..94b2fabf6b
> --- /dev/null
> +++ b/src/conf/virnetworkportdef.h
> @@ -0,0 +1,113 @@
> +/*
> + * virnetworkportdef.h: network port XML processing
> + *
> + * Copyright (C) 2018 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + *<http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#ifndef LIBVIRT_VIRNETWORKPORTDEF_H
> +# define LIBVIRT_VIRNETWORKPORTDEF_H
> +
> +# include "internal.h"
> +# include "viruuid.h"
> +# include "virnetdevvlan.h"
> +# include "virnetdevvportprofile.h"
> +# include "virnetdevbandwidth.h"
> +# include "virpci.h"
> +# include "virxml.h"
> +# include "netdev_vport_profile_conf.h"
> +# include "netdev_bandwidth_conf.h"
> +# include "netdev_vlan_conf.h"
> +
> +typedef struct _virNetworkPortDef virNetworkPortDef;
> +typedef virNetworkPortDef *virNetworkPortDefPtr;
> +
> +typedef enum {
> + VIR_NETWORK_PORT_PLUG_TYPE_NONE,
> + VIR_NETWORK_PORT_PLUG_TYPE_NETWORK,
> + VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE,
> + VIR_NETWORK_PORT_PLUG_TYPE_DIRECT,
> + VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI,
> +
> + VIR_NETWORK_PORT_PLUG_TYPE_LAST,
> +} virNetworkPortPlugType;
> +
> +VIR_ENUM_DECL(virNetworkPortPlug);
> +
> +struct _virNetworkPortDef {
> + unsigned char uuid[VIR_UUID_BUFLEN];
> + char *ownername;
> + unsigned char owneruuid[VIR_UUID_BUFLEN];
> +
> + char *group;
> + virMacAddr mac;
> +
> + virNetDevVPortProfilePtr virtPortProfile;
> + virNetDevBandwidthPtr bandwidth;
> + unsigned int class_id; /* class ID for bandwidth 'floor' */
> + virNetDevVlan vlan;
> + int trustGuestRxFilters; /* enum virTristateBool */
> +
> + int plugtype; /* virNetworkPortPlugType */
> + union {
> + struct {
> + char *brname;
> + int macTableManager; /* enum virNetworkBridgeMACTableManagerType */
> + } bridge; /* For TYPE_NETWORK & TYPE_BRIDGE */
> + struct {
> + char *linkdev;
> + int mode; /* enum virNetDevMacVLanMode from util/virnetdevmacvlan.h */
> + } direct;
> + struct {
> + virPCIDeviceAddress addr; /* PCI Address of device */
> + int driver; /* virNetworkForwardDriverNameType */
> + int managed;
> + } hostdevpci;
> + } plug;
> +};
> +
> +
> +void
> +virNetworkPortDefFree(virNetworkPortDefPtr port);
> +
> +virNetworkPortDefPtr
> +virNetworkPortDefParseNode(xmlDocPtr xml,
> + xmlNodePtr root);
> +
> +virNetworkPortDefPtr
> +virNetworkPortDefParseString(const char *xml);
> +
> +virNetworkPortDefPtr
> +virNetworkPortDefParseFile(const char *filename);
> +
> +char *
> +virNetworkPortDefFormat(const virNetworkPortDef *def);
> +
> +int
> +virNetworkPortDefFormatBuf(virBufferPtr buf,
> + const virNetworkPortDef *def);
> +
> +int
> +virNetworkPortDefSaveStatus(virNetworkPortDef *def,
> + const char *dir);
> +
> +int
> +virNetworkPortDefDeleteStatus(virNetworkPortDef *def,
> + const char *dir);
> +
> +
> +#endif /* LIBVIRT_VIRNETWORKPORTDEF_H */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 909975750c..62a07c094e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1075,6 +1075,16 @@ virNetworkObjUpdate;
> virNetworkObjUpdateAssignDef;
>
>
> +# conf/virnetworkportdef.h
> +virNetworkPortDefFormat;
> +virNetworkPortDefFormatBuf;
> +virNetworkPortDefFree;
> +virNetworkPortDefParseFile;
> +virNetworkPortDefParseNode;
> +virNetworkPortDefParseString;
> +virNetworkPortDefSaveStatus;
> +
> +
> # conf/virnodedeviceobj.h
> virNodeDeviceObjEndAPI;
> virNodeDeviceObjGetDef;
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 46d94d2236..6865ee946e 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -149,6 +149,7 @@ EXTRA_DIST = \
> virmockstathelpers.c \
> virnetdaemondata \
> virnetdevtestdata \
> + virnetworkportxml2xmldata \
> virnwfilterbindingxml2xmldata \
> virpcitestdata \
> virscsidata \
> @@ -335,6 +336,7 @@ endif WITH_YAJL
> test_programs += \
> networkxml2xmltest \
> networkxml2xmlupdatetest \
> + virnetworkportxml2xmltest \
> $(NULL)
>
> if WITH_NETWORK
> @@ -832,6 +834,11 @@ networkxml2xmlupdatetest_SOURCES = \
> testutils.c testutils.h
> networkxml2xmlupdatetest_LDADD = $(LDADDS)
>
> +virnetworkportxml2xmltest_SOURCES = \
> + virnetworkportxml2xmltest.c \
> + testutils.c testutils.h
> +virnetworkportxml2xmltest_LDADD = $(LDADDS)
> +
> if WITH_NETWORK
> networkxml2conftest_SOURCES = \
> networkxml2conftest.c \
> diff --git a/tests/virnetworkportxml2xmldata/plug-bridge-mactbl.xml b/tests/virnetworkportxml2xmldata/plug-bridge-mactbl.xml
> new file mode 100644
> index 0000000000..8036bc2e1c
> --- /dev/null
> +++ b/tests/virnetworkportxml2xmldata/plug-bridge-mactbl.xml
> @@ -0,0 +1,9 @@
> +<networkport>
> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid>
> + <owner>
> + <name>memtest</name>
> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
> + </owner>
> + <mac address='52:54:00:7b:35:93'/>
> + <plug type='bridge' bridge='virbr0' macTableManager='libvirt'/>
> +</networkport>
> diff --git a/tests/virnetworkportxml2xmldata/plug-bridge.xml b/tests/virnetworkportxml2xmldata/plug-bridge.xml
> new file mode 100644
> index 0000000000..e09fc45a9d
> --- /dev/null
> +++ b/tests/virnetworkportxml2xmldata/plug-bridge.xml
> @@ -0,0 +1,15 @@
> +<networkport>
> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid>
> + <owner>
> + <name>memtest</name>
> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
> + </owner>
> + <group>web1</group>
> + <mac address='52:54:00:7b:35:93'/>
> + <bandwidth classID='1729'>
> + <inbound average='1000' peak='4000' floor='2000' burst='1024'/>
> + <outbound average='128' peak='256' burst='32768'/>
> + </bandwidth>
> + <rxfilters trustGuest='yes'/>
> + <plug type='bridge' bridge='virbr0'/>
> +</networkport>
> diff --git a/tests/virnetworkportxml2xmldata/plug-direct.xml b/tests/virnetworkportxml2xmldata/plug-direct.xml
> new file mode 100644
> index 0000000000..81554b4579
> --- /dev/null
> +++ b/tests/virnetworkportxml2xmldata/plug-direct.xml
> @@ -0,0 +1,12 @@
> +<networkport>
> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid>
> + <owner>
> + <name>memtest</name>
> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
> + </owner>
> + <mac address='52:54:00:7b:35:93'/>
> + <virtualport type='802.1Qbg'>
> + <parameters managerid='11' typeid='1193047' typeidversion='2'/>
> + </virtualport>
> + <plug type='direct' dev='ens3' mode='vepa'/>
> +</networkport>
> diff --git a/tests/virnetworkportxml2xmldata/plug-hostdev-pci.xml b/tests/virnetworkportxml2xmldata/plug-hostdev-pci.xml
> new file mode 100644
> index 0000000000..cc4419f3fd
> --- /dev/null
> +++ b/tests/virnetworkportxml2xmldata/plug-hostdev-pci.xml
> @@ -0,0 +1,12 @@
> +<networkport>
> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid>
> + <owner>
> + <name>memtest</name>
> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
> + </owner>
> + <mac address='52:54:00:7b:35:93'/>
> + <plug type='hostdev-pci' managed='yes'>
> + <driver name='vfio'/>
> + <address domain='0x0001' bus='0x02' slot='0x03' function='0x4'/>
> + </plug>
> +</networkport>
> diff --git a/tests/virnetworkportxml2xmldata/plug-network.xml b/tests/virnetworkportxml2xmldata/plug-network.xml
> new file mode 100644
> index 0000000000..7b08ca295a
> --- /dev/null
> +++ b/tests/virnetworkportxml2xmldata/plug-network.xml
> @@ -0,0 +1,16 @@
> +<networkport>
> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid>
> + <owner>
> + <name>memtest</name>
> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
> + </owner>
> + <group>web1</group>
> + <mac address='52:54:00:7b:35:93'/>
> + <bandwidth>
> + <inbound average='1000' peak='4000' floor='2000' burst='1024'/>
> + <outbound average='128' peak='256' burst='32768'/>
> + </bandwidth>
> + <class id='1729'/>
> + <rxfilters trustGuest='yes'/>
> + <plug type='network' bridge='virbr0'/>
> +</networkport>
> diff --git a/tests/virnetworkportxml2xmldata/plug-none.xml b/tests/virnetworkportxml2xmldata/plug-none.xml
> new file mode 100644
> index 0000000000..ed7199ec8c
> --- /dev/null
> +++ b/tests/virnetworkportxml2xmldata/plug-none.xml
> @@ -0,0 +1,8 @@
> +<networkport>
> + <uuid>5d744f21-ba4a-4d6e-bdb2-30a35ff3207d</uuid>
> + <owner>
> + <name>memtest</name>
> + <uuid>d54df46f-1ab5-4a22-8618-4560ef5fac2c</uuid>
> + </owner>
> + <mac address='52:54:00:7b:35:93'/>
> +</networkport>
> diff --git a/tests/virnetworkportxml2xmltest.c b/tests/virnetworkportxml2xmltest.c
> new file mode 100644
> index 0000000000..bb0ae8a8d5
> --- /dev/null
> +++ b/tests/virnetworkportxml2xmltest.c
> @@ -0,0 +1,104 @@
> +/*
> + * virnetworkportxml2xmltest.c: network port XML processing test suite
> + *
> + * Copyright (C) 2018 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + *<http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +
> +#include <unistd.h>
> +
> +#include <sys/types.h>
> +#include <fcntl.h>
> +
> +#include "internal.h"
> +#include "testutils.h"
> +#include "virnetworkportdef.h"
> +#include "virstring.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +
> +static int
> +testCompareXMLToXMLFiles(const char *expected)
> +{
> + char *actual = NULL;
> + int ret = -1;
> + virNetworkPortDefPtr dev = NULL;
> +
> + if (!(dev = virNetworkPortDefParseFile(expected)))
> + goto cleanup;
> +
> + if (!(actual = virNetworkPortDefFormat(dev)))
> + goto cleanup;
> +
> + if (virTestCompareToFile(actual, expected) < 0)
> + goto cleanup;
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(actual);
> + virNetworkPortDefFree(dev);
> + return ret;
> +}
> +
> +struct testInfo {
> + const char *name;
> +};
> +
> +static int
> +testCompareXMLToXMLHelper(const void *data)
> +{
> + const struct testInfo *info = data;
> + int ret = -1;
> + char *xml = NULL;
> +
> + if (virAsprintf(&xml, "%s/virnetworkportxml2xmldata/%s.xml",
> + abs_srcdir, info->name) < 0)
> + goto cleanup;
> +
> + ret = testCompareXMLToXMLFiles(xml);
> +
> + cleanup:
> + VIR_FREE(xml);
> +
> + return ret;
> +}
> +
> +static int
> +mymain(void)
> +{
> + int ret = 0;
> +
> +#define DO_TEST(name) \
> + do { \
> + const struct testInfo info = {name}; \
> + if (virTestRun("virnetworkportdeftest " name, \
> + testCompareXMLToXMLHelper, &info) < 0) \
> + ret = -1; \
> + } while (0)
> +
> + DO_TEST("plug-none");
> + DO_TEST("plug-bridge");
> + DO_TEST("plug-bridge-mactbl");
> + DO_TEST("plug-direct");
> + DO_TEST("plug-hostdev-pci");
> +
> + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +VIR_TEST_MAIN(mymain)
> diff --git a/tests/virschematest.c b/tests/virschematest.c
> index 56bdcb2f88..13c30acc30 100644
> --- a/tests/virschematest.c
> +++ b/tests/virschematest.c
> @@ -227,6 +227,7 @@ mymain(void)
> DO_TEST_DIR("interface.rng", "interfaceschemadata");
> DO_TEST_DIR("network.rng", "../src/network", "networkxml2xmlin",
> "networkxml2xmlout", "networkxml2confdata");
> + DO_TEST_DIR("networkport.rng", "virnetworkportxml2xmldata");
> DO_TEST_DIR("nodedev.rng", "nodedevschemadata");
> DO_TEST_DIR("nwfilter.rng", "nwfilterxml2xmlout", "../examples/xml/nwfilter");
> DO_TEST_DIR("nwfilterbinding.rng", "virnwfilterbindingxml2xmldata");
I'm inclined to ACK this (after a few questions are resolved) even
though I would like to replace the type='network|bridge' with something
more deconstructed - we have to take a dive and do it with *something*
(as long as we're able to fine tune it later). Let me look through the
rest of the patches though.
More information about the libvir-list
mailing list