https://bugzilla.redhat.com/show_bug.cgi?id=1182486
When updating a network and adding new ip-dhcp-host entry, the deamon
may crash. The problem is, we iterate over existing <host/> entries
trying to compare MAC addresses to see if there's already an existing
rule. However, not all entries are required to have MAC address. For
instance, the following is perfectly valid entry:
<host id='00:04:58:fd:e4:15:1b:09:4c:0e:09:af:e4:d3:8c:b8:ca:1e'
name='redhatipv6.redhat.com' ip='2001:db8:ca2:2::119'/>
When the checking loop iterates over this, the entry's MAC address is
accessed directly. Well, the fix is obvious - check if the address is
defined before trying to compare it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
There's this function virNetDevBandwidthParse which parses the
bandwidth XML snippet. But it's not clever much. For the
following XML it allocates the virNetDevBandwidth structure even
though it's completely empty:
<bandwidth>
</bandwidth>
Later in the code there are some places where we check if
bandwidth was set or not. And since we obtained pointer from the
parsing function we think that it is when in fact it isn't.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The macTableManager attribute of a network's bridge subelement tells
libvirt how the bridge's MAC address table (used to determine the
egress port for packets) is managed. In the default mode, "kernel",
management is left to the kernel, which usually determines entries in
part by turning on promiscuous mode on all ports of the bridge,
flooding packets to all ports when the correct destination is unknown,
and adding/removing entries to the fdb as it sees incoming traffic
from particular MAC addresses. In "libvirt" mode, libvirt turns off
learning and flooding on all the bridge ports connected to guest
domain interfaces, and adds/removes entries according to the MAC
addresses in the domain interface configurations. A side effect of
turning off learning and unicast_flood on the ports of a bridge is
that (with Linux kernel 3.17 and newer), the kernel can automatically
turn off promiscuous mode on one or more of the bridge's ports
(usually only the one interface that is used to connect the bridge to
the physical network). The result is better performance (because
packets aren't being flooded to all ports, and can be dropped earlier
when they are of no interest) and slightly better security (a guest
can still send out packets with a spoofed source MAC address, but will
only receive traffic intended for the guest interface's configured MAC
address).
The attribute looks like this in the configuration:
<network>
<name>test</name>
<bridge name='br0' macTableManager='libvirt'/>
...
This patch only adds the config knob, documentation, and test
cases. The functionality behind this knob is added in later patches.
Partially reverts commit 5754dbd.
The code in the specfile adds a MAC address to every <bridge>,
even for <forward mode='bridge'> for which we don't support
changing MAC addresses.
Remove it completely. For new networks, we have been adding
MAC addresses on definition/creation since the commit mentioned above.
For existing networks (pre-0.9.0), the MAC is added by this commit.
https://bugzilla.redhat.com/show_bug.cgi?id=1156367
The function virNetworkObjListExport() in network_conf.c had a call to
the public API virNetworkFree() which was causing a link error:
CCLD libvirt_driver_vbox_network_impl.la
./.libs/libvirt_conf.a(libvirt_conf_la-network_conf.o): In function `virNetworkObjListExport':
/home/laine/devel/libvirt/src/conf/network_conf.c:4496: undefined reference to `virNetworkFree'
This would happen when I added
#include "network_conf.h"
into domain_conf.h, then attempted to call a new function from that
file (and enum converter, similar to virNetworkForwardTypeToString())
In the end, virNetworkFree() ends up just calling virObjectUnref(obj)
anyway (after clearing all pending errors, which we probably *don't*
want to do in the cleanup of a utility function), so this is likely
more correct than the original code as well.
This new attribute will control whether or not libvirt will pay
attention to guest notifications about changes to network device mac
addresses and receive filters. The default for this is 'no' (for
security reasons). If it is set to 'yes' *and* the specified device
model and connection support it (currently only macvtap+virtio) then
libvirt will watch for NIC_RX_FILTER_CHANGED events, and when it
receives one, it will issue a query-rx-filter command, retrieve the
result, and modify the host-side macvtap interface's mac address and
unicast/multicast filters accordingly.
The functionality behind this attribute will be in a later patch. This
patch merely adds the attribute to the top-level of a domain's
<interface> as well as to <network> and <portgroup>, and adds
documentation and schema/xml2xml tests. Rather than adding even more
test files, I've just added the net attribute in various applicable
places of existing test files.
When spanning tree protocol is allowed in bridge settings, forward delay
value is set as well (default is 0 if omitted). Until now, there was no
check for delay value validity. Delay makes sense only as a positive
numerical value.
Note: However, even if you provide positive numerical value, brctl
utility only uses values from range <2,30>, so the number provided can
be modified (kernel most likely) to fall within this range.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1125764
The code compares def->forwarders when deciding to return 0 at a
couple of points, then uses "def->nfwds" as a way to index into
the def->forwarders array. That reference results in Coverity
complaining that def->forwarders being NULL was checked as part
of an arithmetic OR operation where failure could be any one 5
conditions, but that is not checked when entering the loop to
dereference the array. Changing the comparisons to use nfwds
will clear the warnings
Signed-off-by: John Ferlan <jferlan@redhat.com>
When formatting the forward mode addresses or interfaces the switch was
done based on the type of the network rather than of the type of the
individual <interface>/<address> element. In case a user would specify
an incorrect network type ("passhtrough") with <address> elements,
libvirtd would crash as it would attempt to format an <interface>.
Use the type of the individual element to format the XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1132347
Replace:
if (virBufferError(&buf)) {
virBufferFreeAndReset(&buf);
virReportOOMError();
...
}
with:
if (virBufferCheckError(&buf) < 0)
...
This should not be a functional change (unless some callers
misused the virBuffer APIs - a different error would be reported
then)
In "src/conf/" there are many enumeration (enum) declarations.
Similar to the recent cleanup to "src/util" directory, it's
better to use a typedef for variable types, function types and
other usages. Other enumeration and folders will be changed to
typedef's in the future. Most of the files changed in this commit
are reltaed to Network (network_conf.* and interface_conf.*) enums.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
libvirt attempts to determine at startup time which networks are
already active, and set their active flags. Previously it has done
this by assuming that all networks are inactive, then setting the
active flag if the network has a bridge device associated with it and
that bridge device exists. This is not useful for macvtap and hostdev
based networks, since they do not use a bridge device.
Of course the reason that such a check had to be done was that the
presence of a status file in the network "stateDir" couldn't be
trusted as an indicator of whether or not a network was active. This
was due to the network driver mistakenly using
/var/lib/libvirt/network to store the status files, rather than
/var/run/libvirt/network (similar to what is done by every other
libvirt driver that stores status xml for its objects). The difference
is that /var/run is cleared out when the host reboots, so you can be
assured that the state file you are seeing isn't just left over from a
previous boot of the host.
Now that the network driver has been switched to using
/var/run/libvirt/network for status, we can also modify it to assume
that any network with an existing status file is by definition active
- we do this when reading the status file. To fine tune the results,
networkFindActiveConfigs() is changed to networkUpdateAllState(),
and only sets active = 0 if the conditions for particular network
types are *not* met.
The result is that during the first run of libvirtd after the host
boots, there are no status files, so no networks are active. Any time
libvirtd is restarted, any network with a status file will be marked
as active (unless the network uses a bridge device and that device for
some reason doesn't exist).
Experimentation showed that if virNetworkCreateXML() was called for a
network that was already defined, and then the network was
subsequently shutdown, the network would continue to be persistent
after the shutdown (expected/desired), but the original config would
be lost in favor of the transient config sent in with
virNetworkCreateXML() (which would then be the new persistent config)
(obviously unexpected/not desired).
To fix this, virNetworkObjAssignDef() has been changed to
1) properly save/free network->def and network->newDef for all the
various combinations of live/active/persistent, including some
combinations that were previously considered to be an error but didn't
need to be (e.g. setting a "live" config for a network that isn't yet
active but soon will be - that was previously considered an error,
even though in practice it can be very useful).
2) automatically set the persistent flag whenever a new non-live
config is assigned to the network (and clear it when the non-live
config is set to NULL). the libvirt network driver no longer directly
manipulates network->persistent, but instead relies entirely on
virNetworkObjAssignDef() to do the right thing automatically.
After this patch, the following sequence will behave as expected:
virNetworkDefineXML(X)
virNetworkCreateXML(X') (same name but some config different)
virNetworkDestroy(X)
At the end of these calls, the network config will remain as it was
after the initial virNetworkDefine(), whereas previously it would take
on the changes given during virNetworkCreateXML().
Another effect of this tighter coupling between a) setting a !live def
and b) setting/clearing the "persistent" flag, is that future patches
which change the details of network lifecycle management
(e.g. upcoming patches to fix detection of "active" networks when
libvirtd is restarted) will find it much more difficult to break
persistence functionality.
A patch submitted by Steven Malin last week pointed out a problem with
libvirt's DNS SRV record configuration:
https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html
When searching for that message later, I found another series that had
been posted by Guannan Ren back in 2012 that somehow slipped between
the cracks:
https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html
That patch was very much out of date, but also pointed out some real
problems.
This patch fixes all the noted problems by refactoring
virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then
verifies those fixes by added several new records to the test case.
Problems fixed:
* both service and protocol now have an underscore ("_") prepended on
the commandline, as required by RFC2782.
<srv service='sip' protocol='udp' domain='example.com'
target='tests.example.com' port='5060' priority='10'
weight='150'/>
before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150
after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150
* if "domain" wasn't specified in the <srv> element, the extra
trailing "." will no longer be added to the dnsmasq commandline.
<srv service='sip' protocol='udp' target='tests.example.com'
port='5060' priority='10' weight='150'/>
before: srv-host=sip.udp.,tests.example.com,5060,10,150
after: srv-host=_sip._udp,tests.example.com,5060,10,150
* when optional attributes aren't specified, the separating comma is
also now not placed on the dnsmasq commandline. If optional
attributes in the middle of the line are not specified, they are
replaced with a default value in the commandline (1 for port, 0 for
priority and weight).
<srv service='sip' protocol='udp' target='tests.example.com'
port='5060'/>
before: srv-host=sip.udp.,tests.example.com,5060,,
after: srv-host=_sip._udp,tests.example.com,5060
(actually the would have generated an error, because "optional"
attributes weren't really optional.)
* The allowed characters for both service and protocol are now limited
to alphanumerics, plus a few special characters that are found in
existing names in /etc/services and /etc/protocols. (One exception
is that both of these files contain names with an embedded ".", but
"." can't be used in these fields of an SRV record because it is
used as a field separator and there is no method to escape a "."
into a field.) (Previously only the strings "tcp" and "udp" were
allowed for protocol, but this restriction has been removed, since
RFC2782 specifically says that it isn't limited to those, and that
anyway it is case insensitive.)
* the "domain" attribute is no longer required in order to recognize
the port, priority, and weight attributes during parsing. Only
"target" is required for this.
* if "target" isn't specified, port, priority, and weight are not
allowed (since they are meaningless - an empty target means "this
service is *not available* for this domain").
* port, priority, and weight are now truly optional, as the comments
originally suggested, but which was not actually true.
This fixes a possible double free. In virNetworkAssignDef() if
virBitmapNew() fails, then virNetworkObjFree(network) is called.
However, with network->def pointing to actual @def. So if caller
frees @def again, ...
Moreover, this fixes one possible memory leak too. In
virInterfaceAssignDef() if appending to the list of interfaces
fails, we ought to call virInterfaceObjFree() instead of bare
VIR_FREE().
Although, in order to do that some array size variables needs
to be turned into size_t rather than int.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Basically, the idea is copied from domain code, where tainting
exists for a while. Currently, only one taint reason exists -
VIR_NETWORK_TAINT_HOOK to mark those networks which caused invoking
of hook script.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In the next patch I'm going to need the network format function that
takes virBuffer as argument. However, slightly change of name is more
appropriate then: virNetworkDefFormatBuf to match the rest of functions
that format an object to buffer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
In the network status XML we may have the <floor/> element with the
'sum' attribute. The attribute represents sum of all 'floor'-s of
computed over each interface connected to the network (this is needed to
guarantee certain bandwidth for certain domain). The sum is therefore a
number. However, if the number was mangled (e.g. by an user's
interference to network status file), we've just ignored it without
refusing to parse such file. This was all due to 'goto error' missing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The previous patch fixed "forwardPlainNames" so that it really is
doing only what is intended, but left the default to be
"forwardPlainNames='no'". Discussion around the initial version of
that patch led to the decision that the default should instead be
"forwardPlainNames='yes'" (i.e. the original behavior before commit
f3886825). This patch makes that change to the default.
Currently, during XML parsing, when a call to a FromString() function to
get an enum value fails, the error which is reported is either
VIR_ERR_CONFIG_UNSUPPORTED, VIR_ERR_INTERNAL_ERROR or VIR_ERR_XML_ERROR.
This commit makes such conversion failures consistently return
VIR_ERR_CONFIG_UNSUPPORTED.
Most of our code base uses space after comma but not before;
fix the remaining uses before adding a syntax check.
* src/conf/capabilities.c: Consistently use commas.
* src/conf/domain_conf.c: Likewise.
* src/conf/network_conf.c: Likewise.
* src/conf/storage_conf.c: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
'const fooPtr' is the same as 'foo * const' (the pointer won't
change, but it's contents can). But in general, if an interface
is trying to be const-correct, it should be using 'const foo *'
(the pointer is to data that can't be changed).
Fix up remaining offenders in src/conf, and their fallout.
* src/conf/snapshot_conf.h (virDomainSnapshotAssignDef)
(virDomainSnapshotFindByName): Drop attempt at const.
* src/conf/interface_conf.h (virInterfaceObjIsActive)
(virInterfaceDefFormat): Use intended type.
(virInterfaceFindByMACString, virInterfaceFindByName)
(virInterfaceAssignDef, virInterfaceRemove): Drop attempt at
const.
* src/conf/network_conf.h (virNetworkObjIsActive)
(virNetworkDefFormat, virNetworkDefForwardIf)
(virNetworkDefGetIpByIndex, virNetworkIpDefPrefix)
(virNetworkIpDefNetmask): Use intended type.
(virNetworkFindByUUID, virNetworkFindByName, virNetworkAssignDef)
(virNetworkObjAssignDef, virNetworkRemoveInactive)
(virNetworkBridgeInUse, virNetworkSetBridgeName)
(virNetworkAllocateBridge): Drop attempt at const.
* src/conf/netdev_vlan_conf.h (virNetDevVlanFormat): Make
const-correct.
* src/conf/node_device_conf.h (virNodeDeviceHasCap)
(virNodeDeviceDefFormat): Use intended type.
(virNodeDeviceFindByName, virNodeDeviceFindBySysfsPath)
(virNodeDeviceAssignDef, virNodeDeviceObjRemove)
(virNodeDeviceGetParentHost): Drop attempt at const.
* src/conf/secret_conf.h (virSecretDefFormat): Use intended type.
* src/conf/snapshot_conf.c (virDomainSnapshotAssignDef)
(virDomainSnapshotFindByName): Fix fallout.
* src/conf/interface_conf.c (virInterfaceBridgeDefFormat)
(virInterfaceBondDefFormat, virInterfaceVlanDefFormat)
(virInterfaceProtocolDefFormat, virInterfaceDefDevFormat)
(virInterfaceDefFormat, virInterfaceFindByMACString)
(virInterfaceFindByName, virInterfaceAssignDef)
(virInterfaceRemove): Likewise.
* src/conf/network_conf.c
(VIR_ENUM_IMPL, virNetworkFindByName, virNetworkObjAssignDef)
(virNetworkAssignDef, virNetworkRemoveInactive)
(virNetworkDefGetIpByIndex, virNetworkIpDefPrefix)
(virNetworkIpDefNetmask, virNetworkDHCPHostDefParseXML)
(virNetworkIpDefFormat, virNetworkRouteDefFormat)
(virPortGroupDefFormat, virNetworkForwardNatDefFormat)
(virNetworkDefFormatInternal, virNetworkBridgeInUse)
(virNetworkAllocateBridge, virNetworkSetBridgeName)
(virNetworkDNSDefFormat, virNetworkDefFormat): Likewise.
* src/conf/netdev_vlan_conf.c (virNetDevVlanFormat): Likewise.
* src/conf/node_device_conf.c (virNodeDeviceHasCap)
(virNodeDeviceFindBySysfsPath, virNodeDeviceFindByName)
(virNodeDeviceAssignDef, virNodeDeviceObjRemove)
(virNodeDeviceDefFormat, virNodeDeviceGetParentHost): Likewise.
* src/conf/secret_conf.c (virSecretDefFormatUsage)
(virSecretDefFormat): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Useful to set custom forwarders instead of using the contents of
/etc/resolv.conf. It helps me to setup dnsmasq as local nameserver to
resolve VM domain names from domain 0, when domain option is used.
Signed-off-by: Diego Woitasen <diego.woitasen@vhgroup.net>
Signed-off-by: Eric Blake <eblake@redhat.com>
Re-arrange the code so that the returned bitmap is always initialized to
NULL even on early failures and return an error message as some callers
are already expecting it. Fix up the rest not to shadow the error.
This resolves the issue that prompted the filing of
https://bugzilla.redhat.com/show_bug.cgi?id=928638
(although the request there is for something much larger and more
general than this patch).
commit f3868259ca disabled the
forwarding to upstream DNS servers of unresolved DNS requests for
names that had no domain, but were just simple host names (no "."
character anywhere in the name). While this behavior is frowned upon
by DNS root servers (that's why it was changed in libvirt), it is
convenient in some cases, and since dnsmasq can be configured to allow
it, it must not be strictly forbidden.
This patch restores the old behavior, but since it is usually
undesirable, restoring it requires specification of a new option in
the network config. Adding the attribute "forwardPlainNames='yes'" to
the <dns> elemnt does the trick - when that attribute is added to a
network config, any simple hostnames that can't be resolved by the
network's dnsmasq instance will be forwarded to the DNS servers listed
in the host's /etc/resolv.conf for an attempt at resolution (just as
any FQDN would be forwarded).
When that attribute *isn't* specified, unresolved simple names will
*not* be forwarded to the upstream DNS server - this is the default
behavior.
Before, missing attributes were only OK when adding entries;
modification and deletion required all of them.
Now, only deletion works with missing attributes, as long as
the host is uniquely identified.
Decrementing it when it was already 0 causes an invalid free
in virNetworkDefUpdateDNSHost if virNetworkDNSHostDefParseXML
fails and virNetworkDNSHostDefClear gets called twice.
virNetworkForwardDefClear left the number untouched even if it
freed all the elements.
Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
network: static route support for <network>
This patch adds the <route> subelement of <network> to define a static
route. the address and prefix (or netmask) attribute identify the
destination network, and the gateway attribute specifies the next hop
address (which must be directly reachable from the containing
<network>) which is to receive the packets destined for
"address/(prefix|netmask)".
These attributes are translated into an "ip route add" command that is
executed when the network is started. The command used is of the
following form:
ip route add <address>/<prefix> via <gateway> \
dev <virbr-bridge> proto static metric <metric>
Tests are done to validate that the input data are correct. For
example, for a static route ip definition, the address must be a
network address and not a host address. Additional checks are added
to ensure that the specified gateway is directly reachable via this
network (i.e. that the gateway IP address is in the same subnet as one
of the IP's defined for the network).
prefix='0' is supported for both family='ipv4' address='0.0.0.0'
netmask='0.0.0.0' or prefix='0', and for family='ipv6' address='::',
prefix=0', although care should be taken to not override a desired
system default route.
Anytime an attempt is made to define a static route which *exactly*
duplicates an existing static route (for example, address=::,
prefix=0, metric=1), the following error message will be sent to
syslog:
RTNETLINK answers: File exists
This can be overridden by decreasing the metric value for the route
that should be preferred, or increasing the metric for the route that
shouldn't be preferred (and is thus in place only in anticipation that
the preferred route may be removed in the future). Caution should be
used when manipulating route metrics, especially for a default route.
Note: The use of the command-line interface should be replaced by
direct use of libnl so that error conditions can be handled better. But,
that is being left as an exercise for another day.
Signed-off-by: Gene Czarcinski <gene@czarc.net>
Signed-off-by: Laine Stump <laine@laine.org>
The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.
I remembered to document this bit, but somehow forgot to implement it.
This adds <driver name='kvm|vfio'/> as a subelement to the <forward>
element of a network (this puts it parallel to the match between
mode='hostdev' attribute in a network and type='hostdev' in an
<interface>).
Since it's already documented, only the parser, formatter, backend
driver recognition (it just translates/moves the flag into the
<interface> at the appropriate time), and a test case were needed.
(I used a separate enum for the values both because the original is
defined in domain_conf.h, which is unavailable from network_conf.h,
and because in the future it's possible that we may want to support
other non-hostdev oriented driver names in the network parser; this
makes sure that one can be expanded without the other).
1. Handle invalid ULong prefix specified.
When parsing for @prefix as a ULong, a -2 can be returned
if the specification is not a valid ULong.
2. Error out if address= is not specified.
3. Merge netmask process/tests under family tests.
4. Max sure that prefix does not exceed maximum.
.
Signed-off-by: Gene Czarcinski <gene@czarc.net>
Create the utility function virSocketAddrGetIpPrefix() to
determine the prefix for this network. The code in this
function was adapted from virNetworkIpDefPrefix().
Update virNetworkIpDefPrefix() in src/conf/network_conf.c
to use the new utility function.
Signed-off-by: Gene Czarcinski <gene@czarc.net>
Until now tranisent networks weren't really useful as libvirtd wasn't
able to remember them across restarts. This patch adds support for
loading status files of transient networks (that already were generated)
so that the status isn't lost.
This patch chops up virNetworkObjUpdateParseFile and turns it into
virNetworkLoadState and a few friends that will help us to load status
XMLs and refactors the functions that are loading the configs to use
them.
==5306== 8 bytes in 1 blocks are definitely lost in loss record 24 of 277
==5306== at 0x4C28B2F: calloc (vg_replace_malloc.c:593)
==5306== by 0x5293CAF: virAllocN (viralloc.c:152)
==5306== by 0x52DFEAE: virXPathNodeSet (virxml.c:611)
==5306== by 0x5313DD9: virNetworkDefParseXML (network_conf.c:1408)
==5306== by 0x53170F6: virNetworkObjUpdateParseFile (network_conf.c:2031)
==5306== by 0x131DA63C: networkStartup (bridge_driver.c:279)
==5306== by 0x53481DF: virStateInitialize (libvirt.c:822)
==5306== by 0x40DF44: daemonRunStateInit (libvirtd.c:877)
==5306== by 0x52D2FF5: virThreadHelper (virthreadpthread.c:161)
==5306== by 0x5D00C52: start_thread (in /usr/lib64/libpthread-2.17.so)
==5306== by 0x6410ECC: clone (in /usr/lib64/libc-2.17.so)
When libvirtd loads active network configs from network state directory,
it should release the class_id memory block which was allocated
at the time of loading xml from network config directory.
virBitmapParse will create a new memory block of bitmap class_id which
causes a memory leak.
This happens when at least one virtual network is active before.
==12234== 8,216 (24 direct, 8,192 indirect) bytes in 1 blocks are definitely \
lost in loss record 702 of 709
==12234== at 0x4A06B2F: calloc (vg_replace_malloc.c:593)
==12234== by 0x37AB04D77D: virAlloc (in /usr/lib64/libvirt.so.0.1000.3)
==12234== by 0x37AB04EF89: virBitmapNew (in /usr/lib64/libvirt.so.0.1000.3)
==12234== by 0x37AB0BFB37: virNetworkAssignDef (in /usr/lib64/libvirt.so.0.1000.3)
==12234== by 0x37AB0BFD31: ??? (in /usr/lib64/libvirt.so.0.1000.3)
==12234== by 0x37AB0BFE92: virNetworkLoadAllConfigs (in /usr/lib64/libvirt.so.0.1000.3)
==12234== by 0x10650E5A: ??? (in /usr/lib64/libvirt/connection-driver/libvirt_driver_network.so)
==12234== by 0x37AB0EB72F: virStateInitialize (in /usr/lib64/libvirt.so.0.1000.3)
==12234== by 0x40DE04: ??? (in /usr/sbin/libvirtd)
==12234== by 0x37AB0832E8: ??? (in /usr/lib64/libvirt.so.0.1000.3)
==12234== by 0x3796807D14: start_thread (in /usr/lib64/libpthread-2.16.so)
==12234== by 0x37960F246C: clone (in /usr/lib64/libc-2.16.so)
This reverts commit 383ebc4694.
We decided the xml for this feature needed more thought to make sure
we are doing it the best way, in particular wrt option values that
have multiple items.
Originally, only a host name was used to associate a
DHCPv6 request with a specific IPv6 address. Further testing
demonstrates that this is an unreliable method and, instead,
a client-id or DUID needs to be used. According to DHCPv6
standards, this id can be a duid-LLT, duid-LL, or duid-UUID
even though dnsmasq will accept almost any text string.
Although validity checking of a specified string makes sure it is
hexadecimal notation with bytes separated by colons, there is no
rigorous check to make sure it meets the standard.
Documentation and schemas have been updated.
Signed-off-by: Gene Czarcinski <gene@czarc.net>
Signed-off-by: Laine Stump <laine@laine.org>
This patch adds support for a new <option>-Tag in the <dhcp> block of
network configs, based on a subset of the fifth proposal by Laine
Stump in the mailing list discussion at
https://www.redhat.com/archives/libvir-list/2012-November/msg01054.html.
Any such defined option will result in a dhcp-option=<number>,"<value>"
statement in the generated dnsmasq configuration file.
Currently, DHCP options can be specified by number only and there is
no whitelisting or blacklisting of option numbers, which should
probably be added.
Signed-off-by: Pieter Hollants <pieter@hollants.com>
Signed-off-by: Laine Stump <laine@laine.org>
We pass over the address/port start/end values many times so we put
them in structs.
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Laine Stump <laine@laine.org>
Let users set the port range to be used for forward mode NAT:
...
<forward mode='nat'>
<nat>
<port start='1024' end='65535'/>
</nat>
</forward>
...
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Laine Stump <laine@laine.org>
Support setting which public ip to use for NAT via attribute
address in subelement <nat> in <forward>:
...
<forward mode='nat'>
<address start='1.2.3.4' end='1.2.3.10'/>
</forward>
...
This will construct an iptables line using:
'-j SNAT --to-source <start>-<end>'
instead of:
'-j MASQUERADE'
Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Laine Stump <laine@laine.org>
gcc 4.1.2 on RHEL 5 warned:
conf/network_conf.c:3136: warning: 'foundIdx' may be used uninitialized in this function
The warning is spurious, but initializing the variable doesn't hurt.
* src/conf/network_conf.c (virNetworkDefUpdateDNSHost): Silence
unused variable warning.
gcc -O2 complained:
../../src/conf/network_conf.c: In function 'virNetworkDefUpdateDNSSrv':
../../src/conf/network_conf.c:3232: error: 'foundIdx' may be used uninitialized in this function [-Wuninitialized]
It turned out to be a spurious warning (we didn't use foundIdx
unless foundCt was non-zero). But in investigating that, I noticed
a worse problem: we were using 'if (foundCt > 1)', but since foundCt
was bool, it could never be > 1.
* src/conf/network_conf.c (virNetworkDefUpdateDNSHost): Use
correct type.
(virNetworkDefUpdateDNSSrv): Likewise, and silence compiler
warning.
Currently, we are only keeping a inactive XML configuration
in status dir. This is no longer enough as we need to keep
this class_id attribute so we don't overwrite old entries
when the daemon restarts. However, since there has already
been release which has just <network/> as root element,
and we want to keep things compatible, detect that loaded
status file is older one, and don't scream about it.
Network should be notified if we plug in or unplug an
interface, so it can perform some action, e.g. set/unset
network part of QoS. However, we are doing this in very
early stage, so iface->ifname isn't filled in yet. So
whenever we want to report an error, we must use a different
identifier, e.g. the MAC address.
This is however supported only on domain interfaces with
type='network'. Moreover, target network needs to have at least
inbound QoS set. This is required by hierarchical traffic shaping.
From now on, the required attribute for <inbound/> is either 'average'
(old) or 'floor' (new). This new attribute can be used just for
interfaces type of network (<interface type='network'/>) currently.
The DHCPv6 support includes IPV6 dhcp-range and dhcp-host for one
IPv6 subnetwork on one interface. This support will only work
if dnsmasq version >= 2.64; otherwise an error occurs if
dhcp-range or dhcp-host is specified for an IPv6 address.
Essentially, this change provides the same DHCP support for IPv6
that has been available for IPv4.
With dnsmasq >= 2.64, support for the RA service is also now provided
by dnsmasq (radvd is no longer used/started). (Although at least one
version of dnsmasq prior to 2.64 "supported" IPv6 Router
Advertisement, there were bugs (fixed in 2.64) that rendered it
unusable.)
Documentation and the network schema has been updated
to reflect the new support.
virNetworkDefUpdateForward requires separate functions to parse and
clear a virNetworkForwardDef by itself, but they were previously just
inlined in the virNetworkDef parse and free functions. This patch
makes them into separate functions.
The attributes of a <network> element's <forward> element were
previously stored directly in the virNetworkDef object, but
virNetworkUpdateForward() needs to operate on a <forward> in
isolation, so this patchs pulls out all those attributes into a
separate virNetworkForwardDef struct (and shortens their names
appropriately). This new object is contained in the virNetworkDef, not
pointed to by it, so there is no extra memory management.
This patch makes no functional changes, it only changes, e.g.,
"nForwardIfs" to "forward.nifs".
The other clear functions in network_conf.c that clear out arrays of
sub-objects do so by using the n[itemname]s value as a counter going
down to 0. Make this one consistent. There's no functional value, just
makes the style more consistent with the rest of the file.
This makes some function names and arg lists for consistent with other
parse functions in network_conf.c. While modifying
virNetworkIPParseXML(), also change its "error" label to "cleanup",
since the code at that label is executed on success as well as
failure.
These three functions are very similar - none allow a MODIFY
operation; you can only add or delete.
The biggest difference between them (other than the data itself) is in
the criteria for determining a match, and whether or not multiple
matches are possible:
1) for HOST records, it's considered a match if the IP address or any
of the hostnames of an existing record matches.
2) for SRV records, it's a match if all of
domain+service+protocol+target *which have been specified* are
matched.
3) for TXT records, there is only a single field to match - name
(value can be the same for multiple records, and isn't considered a
search term), so by definition there can be no ambiguous matches.
In all three cases, if any matches are found, ADD will fail; if
multiple matches are found, it means the search term was ambiguous,
and a DELETE will fail.
The upper level code in bridge_driver.c is already implemented for
these functions - appropriate conf files will be re-written, and
dnsmasq will be SIGHUPed or restarted as appropriate.
Since there is only a single virNetworkDNSDef for any virNetworkDef,
and it's trivial to determine whether or not it contains any real
data, it's much simpler (and fits more uniformly with the parse
function calling sequence of the parsers for many other objects that
are subordinates of virNetworkDef) if virNetworkDef *contains* an
virNetworkDNSDef rather than pointing to one.
Since it is now just a part of another object rather than its own
object, it no longer makes sense to have a *Free() function, so that
is changed to a *Clear() function.
More importantly though, ParseXML and Clear functions are needed for
the individual items contained in a virNetworkDNSDef (srv, txt, and
host records), but none of them have a *Clear(), and only two of the
three had *ParseXML() functions (both of which used a non-uniform
arglist). Those problems are cleared up by this patch - it splits the
higher-level Clear function into separate functions for each of the
three, creates a parse for txt records, and cleans up the srv and host
parsers, so we now have all the utility functions necessary to
implement virNetworkDefUpdateDNS(Host|Srv|Txt).
This shortens the name of the structs for srv and txt, and their
instances in virNetworkDNSDef, to be more compact and uniform with the
naming of the dns host array. It also changes the type of ntxts, etc
from unsigned int to size_t, so that they can be used directly as args
to VIR_*_ELEMENT.
The already-written backend functions for virNetworkUpdate that add
and delete items into lists within the a network were already debugged
to work properly, but future such functions will use
VIR_(INSERT|DELETE)_ELEMENT instead, so these are changed for
uniformity.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=767057
It was possible to define a network with <forward mode='bridge'> that
had both a bridge device and a forward device defined. These two are
mutually exclusive by definition (if you are using a bridge device,
then this is a host bridge, and if you have a forward dev defined,
this is using macvtap). It was also possible to put <ip>, <dns>, and
<domain> elements in this definition, although those aren't supported
by the current driver (although it's conceivable that some other
driver might support that).
The items that are invalid by definition, are now checked in the XML
parser (since they will definitely *always* be wrong), and the others
are checked in networkValidate() in the network driver (since, as
mentioned, it's possible that some other network driver, or even this
one, could some day support setting those).
This patch adds the capability for virtual guests to do IPv6
communication via a virtual network interface with no IPv6 (gateway)
addresses specified. This capability has always been enabled by
default for IPv4, but disabled for IPv6 for security concerns, and
because it requires the ip6tables command to be operational (which
isn't the case on a system with the ipv6 module completely disabled).
This patch adds a new attribute "ipv6" at the toplevel of a <network>
object. If ipv6='yes', the extra ip6tables rules required to permite
inter-guest communications are added when the network is started. If
it is 'no', or not present, those rules will not be added; thus the
default behavior doesn't change, so there should be no compatibility
issues with any existing installations.
Note that virtual guests cannot communication with the virtualization
host via this interface, because the following kernel tunable has
been set:
net.ipv6.conf.<bridge_interface_name>.disable_ipv6 = 1
This assures that the bridge interface will not have an IPv6
link-local (fe80::) address.
To control this behavior so that it is not enabled by default, the parameter
ipv6='yes' on the <network> statement has been added.
Documentation related to this patch has been updated.
The network schema has also been updated.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=879473
The name attribute is required for portgroup elements (yes, the RNG
specifies that), and there is code in libvirt that assumes it is
non-null. Unfortunately, the portgroup parsing function wasn't
checking for lack of portgroup. One adverse result of this was that
attempts to update a network by adding a portgroup with no name would
cause libvirtd to segfault. For example:
virsh net-update default add portgroup "<portgroup default='yes'/>"
This patch causes virNetworkPortGroupParseXML to fail if no name is
specified, thus avoiding any later problems.
The libvirt coding standard is to use 'function(...args...)'
instead of 'function (...args...)'. A non-trivial number of
places did not follow this rule and are fixed in this patch.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
In the XML warning, we print a virsh command line that can be used to
edit that XML. This patch prints UUIDs if the entity name contains
special characters (like shell metacharacters, or "--" that would break
parsing of the XML comment). If the entity doesn't have a UUID, just
print the virsh command that can be used to edit it.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=868483
virNetworkUpdate, virNetworkDefine, and virNetworkCreate all three
allow network definitions to contain multiple <portgroup> elements
with default='yes'. Only a single default portgroup should be allowed
for each network.
This patch updates networkValidate() (called by both
virNetworkCreate() and virNetworkDefine()) and
virNetworkDefUpdatePortGroup (called by virNetworkUpdate() to not
allow multiple default portgroups.
https://bugzilla.redhat.com/show_bug.cgi?id=866364
pointed out a crash due to virNetworkObjAssignDef free'ing
network->newDef without NULLing it afterward. A fix for this is in
upstream commit b7e9202401. While the
NULLing of newDef was a legitimate fix, newDef should have already
been empty (NULL) anyway (as indicated in the comment that was deleted
by that commit).
The reason that newDef had a non-NULL value (i.e. the root cause) was
that networkStartNetwork() had failed after populating
network->newDef, but then neglected to free/NULL newDef in the
cleanup.
(A bit of background here: network->newDef should contain the
persistent config of a network when a network is active (and of course
only when it is persisten), and NULL at all other times. There is also
a network->def which should contain the persistent definition of the
network when it is inactive, and the current live state at all other
times. The idea is that you can make changes to network->newDef which
will take effect the next time the network is restarted, but won't
mess with the current state of the network (virDomainObj has a similar
pair of virDomainDefs that behave in the same fashion). Personally I
think there should be a network->live and network->config, and the
location of the persistent config should *always* be in
network->config, but that's for a later cleanup).
Since I love things to be symmetric, I created a new function called
virNetworkObjUnsetDefTransient(), which reverses the effects of
virNetworkObjSetDefTransient(). I don't really like the name of the
new function, but then I also didn't really like the name of the old
one either (it's just named that way to match a similar function in
the domain conf code).
