This patch is a fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=743176
which was discovered by Dan Berrange while making bandwidth
configuration work for LXC guests.
Background: Although virtportprofile data from a network portgroup is
only applicable for direct mode interfaces, the code that copies
bandwidth data from the portgroup was also only being executed in the
case of direct mode interfaces. The result was that interfaces using
traditional virtual networks (forward mode='nat|route|none'), and
those using a host bridge for forwarding, would not pick up bandwidth
data from a portgroup defined in the network.
This patch moves that code outside the conditional, so that bandwidth
information is *alway* copied from the appropriate portgroup (unless
the <interface> definition itself already has bandwidth information,
which would take precedence over what's in the portgroup anyway).
Code altered so that it is consistent with the associated comment. The
'autoconf' variable is forced to zero.
Signed-off-by: Neil Wilson <neil@brightbox.co.uk>
In some cases the caller of virPidFileRead might like extra checks
to determine whether the pid just read is really the one they are
expecting. This adds virPidFileReadIfAlive which will check whether
the pid is still alive with kill(0, -1), and (on linux only) will
look at /proc/$PID/path
* libvirt_private.syms, util/virpidfile.c, util/virpidfile.h: Add
virPidFileReadIfValid and virPidFileReadPathIfValid
* network/bridge_driver.c: Use new APIs to check PID validity
The functions for manipulating pidfiles are in util/util.{c,h}.
We will shortly be adding some further pidfile related functions.
To avoid further growing util.c, this moves the pidfile related
functions into a dedicated virpidfile.{c,h}. The functions are
also all renamed to have 'virPidFile' as their name prefix
* util/util.h, util/util.c: Remove all pidfile code
* util/virpidfile.c, util/virpidfile.h: Add new APIs for pidfile
handling.
* lxc/lxc_controller.c, lxc/lxc_driver.c, network/bridge_driver.c,
qemu/qemu_process.c: Add virpidfile.h include and adapt for API
renames
This addresses https://bugzilla.redhat.com/show_bug.cgi?id=713728
When "defining" a new network (or one that exists but isn't currently
active) the new definition is stored in network->def, but for a
network that already exists and is active, the new definition is
stored in network->newDef, and then moved over to network->def as soon
as the network is destroyed.
However, the code that writes the dhcp and dns hosts files used by
dnsmasq was always using network->def for its information, even when
the new data was actually in network->newDef, so the hosts files
always lagged one edit behind the definition.
This patch changes the code to keep the pointer to the new definition
after it's been assigned into the network, and use it directly
(regardless of whether it's stored in network->newDef or network->def)
to construct the hosts files.
Coverity complained that 395 out of 409 virAsprintf calls are
checked, and therefore assumed that the remaining cases are bugs
waiting to happen. But in each of these cases, a failed virAsprintf
will properly set the target string to NULL, and pass on that
failure to the caller, without wasting efforts to check the call.
Adding the ignore_value silences Coverity.
* src/conf/domain_audit.c (virDomainAuditGetRdev): Ignore
virAsprintf return value, when it behaves like we need.
* src/network/bridge_driver.c (networkDnsmasqLeaseFileNameDefault)
(networkRadvdConfigFileName, networkBridgeDummyNicName)
(networkRadvdPidfileBasename): Likewise.
* src/util/storage_file.c (absolutePathFromBaseFile): Likewise.
* src/openvz/openvz_driver.c (openvzGenerateContainerVethName):
Likewise.
* src/util/command.c (virCommandTranslateStatus): Likewise.
This is in response to:
https://bugzilla.redhat.com/show_bug.cgi?id=723862
which points out that a guest on an "isolated" network could
potentially exploit the DNS forwarding provided by dnsmasq to create a
communication channel to the outside.
This patch eliminates that possibility by adding the "--no-resolv"
argument to the dnsmasq commandline, which tells dnsmasq to not
forward on any requests that it can't resolve itself (by looking at
its own static hosts files and runtime list of dhcp clients), but to
instead return a failure for those requests.
This shouldn't cause any undesirable change from current
behavior, even in the case where a guest is currently configured with
multiple interfaces, one of them being connected to an isolated
network, and another to a network that does have connectivity to the
outside. If the isolated network's DNS server is queried for a name
it doesn't know, it will return "Refused" rather than "Unknown", which
indicates to the guest that it should query other servers, so it then
queries the connected DNS server, and gets the desired response.
Every DomainNetDef has a bandwidth, as does every portgroup.
Whenever a DomainNetDef of type NETWORK is about to be used, a call is
made to networkAllocateActualDevice(). This function chooses the "best"
bandwidth object and places it in the DomainActualNetDef.
From that point on, whenever some code needs to use the bandwidth data
for the interface, it's retrieved with virDomainNetGetActualBandwidth(),
which will always return the "best" info as determined in the
previous step.
Although most functions in libvirt return 0 on success and < 0 on
failure, there are a few functions lingering around that return errno
(a positive value) on failure, and sometimes code calling those
functions incorrectly assumes the <0 standard. I noticed one of these
the other day when auditing networkStartDhcpDaemon after Guido Gunther
found a place where success was improperly returned on failure (that
patch has been acked and is pending a push). The problem was that it
expected the return value from virFileReadPid to be < 0 on failure,
but it was actually positive (it was also neglected to set the return
code in this case, similar to the bug found by Guido).
This all led to the fact that *all* of the virFile*Pid functions in
util.c are returning errno on failure. This patch remedies that
problem by changing them all to return -errno on failure, and makes
any necessary changes to callers of the functions. (In the meantime, I
also properly set the return code on failure of virFileReadPid in
networkStartDhcpDaemon).
The new listenNetwork attribute needs to learn an IP address based on a
named network. This patch provides a function networkGetNetworkAddress
which provides that.
Some networks have an IP address explicitly in their configuration
(ie, those with a forward type of "none", "route", or "nat"). For
those, we can just return the IP address from the config.
The rest will have a physical device associated with them (either via
<bridge name='...'/>, <forward ... dev='...'/>, or possibly via a pool
of interfaces inside the network's <forward> element) and we will need
to ask the kernel for a current IP address of that device (via the
newly added ifaceGetIPAddress)
If networkGetNetworkAddress encounters an error while trying to learn
the address for a network, it will return -1. In the case that libvirt
has been compiled without the network driver, the call is a macro
which reduces to -2. This allows differentiating between a failure of
the network driver, and its complete absence.
The network driver needs to assign physical devices for use by modes
that use macvtap, keeping track of which physical devices are in use
(and how many instances, when the devices can be shared). Three calls
are added:
networkAllocateActualDevice - finds a physical device for use by the
domain, and sets up the virDomainActualNetDef accordingly.
networkNotifyActualDevice - assumes that the domain was already
running, but libvirtd was restarted, and needs to be notified by each
already-running domain about what interfaces they are using.
networkReleaseActualDevice - decrements the usage count of the
allocated physical device, and frees the virDomainActualNetDef to
avoid later accidentally using the device.
bridge_driver.[hc] - the new APIs. When WITH_NETWORK is false, these
functions are all #defined to be "0" in the .h file (effectively
becoming a NOP) to prevent link errors.
qemu_(command|driver|hotplug|process).c - add calls to the above APIs
in the appropriate places.
tests/Makefile.am - we need to include libvirt_driver_network.la
whenever libvirt_driver_qemu.la is linked, to avoid unreferenced
symbols (in functions that are never called by the test
programs...)
Previously all networks were composed of bridge devices created and
managed by libvirt, and the same operations needed to be done for all
of them when they were started and stopped (create and start the
bridge device, configure its MAC address and IP address, add iptables
rules). The new network types are (for now at least) managed outside
of libvirt, and the network object is used only to contain information
about the network, which is then used as each individual guest
connects itself.
This means that when starting/stopping one of these new networks, we
really want to do nothing, aside from marking the network as
active/inactive.
This has been setup as toplevel Start/Shutdown functions that do the
small bit of common stuff, then have a switch statement to execute
network type-specific start/shutdown code, then do a bit more common
code. The type-specific functions called for the new host bridge and
macvtap based types are currently empty.
In the future these functions may actually do something, and we will
surely add more functions that are similarly patterned. Once
everything has settled, we can make a table of "sub-driver" function
pointers for each network type, and store a pointer to that table in
the network object, then we can replace the switch statements with
calls to functions in the table.
The final step in this will be to add a new table (and corresponding
new functions) for new network types as they are added.
The network XML is updated in the following ways:
1) The <forward> element can now contain a list of forward interfaces:
<forward .... >
<interface dev='eth10'/>
<interface dev='eth11'/>
<interface dev='eth12'/>
<interface dev='eth13'/>
</forward>
The first of these takes the place of the dev attribute that is
normally in <forward> - when defining a network you can specify
either one, and on output both will be present. If you specify
both on input, they must match.
2) In addition to forward modes of 'nat' and 'route', these new modes
are supported:
private, passthrough, vepa - when this network is referenced by a
domain's interface, it will have the same effect as if the
interface had been defined as type='direct', e.g.:
<interface type='direct'>
<source mode='${mode}' dev='${dev}>
...
</interface>
where ${mode} is one of the three new modes, and ${dev} is an interface
selected from the list given in <forward>.
bridge - if a <forward> dev (or multiple devs) is defined, and
forward mode is 'bridge' this is just like the modes 'private',
'passthrough', and 'vepa' above. If there is no forward dev
specified but a bridge name is given (e.g. "<bridge
name='br0'/>"), then guest interfaces using this network will use
libvirt's "host bridge" mode, equivalent to this:
<interface type='bridge'>
<source bridge='${bridge-name}'/>
...
</interface>
3) A network can have multiple <portgroup> elements, which may be
selected by the guest interface definition (by adding
"portgroup='${name}'" in the <source> element along with the
network name). Currently a portgroup can only contain a
virtportprofile, but the intent is that other configuration items
may be put there int the future (e.g. bandwidth config). When
building a guest's interface, if the <interface> XML itself has no
virtportprofile, and if the requested network has a portgroup with
a name matching the name given in the <interface> (or if one of the
network's portgroups is marked with the "default='yes'" attribute),
the virtportprofile from that portgroup will be used by the
interface.
4) A network can have a virtportprofile defined at the top level,
which will be used by a guest interface when connecting in one of
the 'direct' modes if the guest interface XML itself hasn't
specified any virtportprofile, and if there are also no matching
portgroups on the network.
Some callers expected virFileMakePath to set errno, some expected
it to return an errno value. Unify this to return 0 on success and
-1 on error. Set errno to report detailed error information.
Also optimize virFileMakePath if stat fails with an errno different
from ENOENT.
networkSaveDnsmasqHostsfile was added in 8fa9c22142 (Apr 2010).
It has a force flag. If the dnsmasq hostsfile already exists force
needs to be true to overwrite it. networkBuildDnsmasqArgv sets force
to false, networkDefine sets it to true. This results in the
hostsfile being written only in networkDefine in the common case.
If no error occurred networkSaveDnsmasqHostsfile returns true and
networkBuildDnsmasqArgv adds the --dhcp-hostsfile to the dnsmasq
command line.
networkSaveDnsmasqHostsfile was changed in 89ae9849f7 (24 Jun 2011)
to return a new dnsmasqContext instead of reusing one. This change broke
the logic of the force flag as now networkSaveDnsmasqHostsfile returns
NULL on error, but the early return -- if force was not set and the
hostsfile exists -- returns 0. This turned the early return in an error
case and networkBuildDnsmasqArgv didn't add the --dhcp-hostsfile option
anymore if the hostsfile already exists. It did because networkDefine
created the hostsfile already.
Then 9d4e2845d4 fixed the return 0 case in networkSaveDnsmasqHostsfile
but didn't apply the force option correctly to the new addnhosts file.
Now force doesn't control an early return anymore, but influences the
handling of the hostsfile context creation and dnsmasqSave is always
called now. This commit also added test cases that reveal several
problems. First, the tests now calls functions that try to write the
dnsmasq config files to disk. If someone runs this tests as root this
might overwrite actively used dnsmasq config files, this is a no-go. Also
the tests depend on configure --localstatedir, this needs to be fixed as
well, because it makes the tests fail when localstatedir is different
from /var.
This patch does several things to fix this:
1) Move dnsmasqContext creation and saving out of networkBuildDnsmasqArgv
to the caller to separate the command line generation from the config
file writing. This makes the command line generation testable without the
risk of interfering with system files, because the tests just don't call
dnsmasqSave.
2) This refactoring of networkSaveDnsmasqHostsfile makes the force flag
useless as the saving happens somewhere else now. This fixes the wrong
usage of the force flag in combination with then newly added addnhosts
file by removing the force flag.
3) Adapt the wrong test cases to the correct behavior, by adding the
missing --dhcp-hostsfile option. Both affected tests contain DHCP host
elements but missed the necessary --dhcp-hostsfile option.
4) Rename networkSaveDnsmasqHostsfile to networkBuildDnsmasqHostsfile,
because it doesn't save the dnsmasqContext anymore.
5) Move all directory creations in dnsmasq context handling code from
the *New functions to dnsmasqSave to avoid directory creations in system
paths in the test cases.
6) Now that networkBuildDnsmasqArgv doesn't create the dnsmasqContext
anymore the test case can create one with the localstatedir that is
expected by the tests instead of the configure --localstatedir given one.
If a domain name is defined for a network, add the --expand-hosts
option to the dnsmasq commandline. This results in the domain being
added to any hostname that is defined in a dns <host> element and
contains no '.' characters (i.e. it is an "unqualified"
hostname). Since PTR records are automatically created for any name
defined in <host>, the result of a PTR request will change from the
unqualified name to the qualified name.
This also has the same effect on any hostnames that dnsmasq reads
from the host's /etc/hosts file.
(In the case of guest hostnames that were learned by dnsmasq via DHCP
requests, they were already getting the domain name added on, even
without --expand-hosts).
Convert networkDnsmasqLeaseFileName to a replaceable function pointer
that allow the testsuite to use a version of that function that is not
depending on configure --localstatedir.
This fixes 5 of 6 test failures, when configure --localstatedir isn't
set to /var.
This commit introduces names definition for the DNS hosts file using
the following syntax:
<dns>
<host ip="192.168.1.1">
<name>alias1</name>
<name>alias2</name>
</host>
</dns>
Some of the improvements and fixes were done by Laine Stump so
I'm putting him into the SOB clause again ;-)
Signed-off-by: Michal Novotny <minovotn@redhat.com>
Signed-off-by: Laine Stump <laine@laine.org>
The dnsmasq commandline was being built as a part of running
dnsmasq. This patch puts the commandline build into a separate
function (and exports it as a private API) making it possible to build
a dnsmasq commandline without executing it, so that we can write a
test program to verify that the proper commandlines are being created.
Signed-off-by: Michal Novotny <minovotn@redhat.com>
This commit introduces the <dns> element and <txt> record for the
virtual DNS network. The DNS TXT record can be defined using following
syntax in the network XML file:
<dns>
<txt name="example" value="example value" />
</dns>
Also, the Relax-NG scheme has been altered to allow the texts without
spaces only for the name element and some nitpicks about memory
free'ing have been fixed by Laine so therefore I'm adding Laine to the
SOB clause ;-)
Signed-off-by: Michal Novotny <minovotn@redhat.com>
Signed-off-by: Laine Stump <laine@laine.org>
Since we virEventRegisterDefaultImpl is now a public API, callers need
a way to invoke the default registered Handle and Timeout functions. We
already have general functions for these internally, so promote
them to the public API.
v2:
Actually add APIs to libvirt.h
Change all the driver struct initializers to use the
C99 style, leaving out unused fields. This will make
it possible to add new APIs without changing every
driver. eg change:
qemudDomainResume, /* domainResume */
qemudDomainShutdown, /* domainShutdown */
NULL, /* domainReboot */
qemudDomainDestroy, /* domainDestroy */
to
.domainResume = qemudDomainResume,
.domainShutdown = qemudDomainShutdown,
.domainDestroy = qemudDomainDestroy,
And get rid of any existing C99 style initializersr which
set NULL, eg change
.listPools = vboxStorageListPools,
.numOfDefinedPools = NULL,
.listDefinedPools = NULL,
.findPoolSources = NULL,
.poolLookupByName = vboxStoragePoolLookupByName,
to
.listPools = vboxStorageListPools,
.poolLookupByName = vboxStoragePoolLookupByName,
We were 31/73 on whether to translate; since less than 50% translated
and since VIR_INFO is less than VIR_WARN which also doesn't translate,
this makes sense.
* cfg.mk (sc_prohibit_gettext_markup): Add VIR_INFO, since it
falls between WARN and DEBUG.
* daemon/libvirtd.c (qemudDispatchSignalEvent, remoteCheckAccess)
(qemudDispatchServer): Adjust offenders.
* daemon/remote.c (remoteDispatchAuthPolkit): Likewise.
* src/network/bridge_driver.c (networkReloadIptablesRules)
(networkStartNetworkDaemon, networkShutdownNetworkDaemon)
(networkCreate, networkDefine, networkUndefine): Likewise.
* src/qemu/qemu_driver.c (qemudDomainDefine)
(qemudDomainUndefine): Likewise.
* src/storage/storage_driver.c (storagePoolCreate)
(storagePoolDefine, storagePoolUndefine, storagePoolStart)
(storagePoolDestroy, storagePoolDelete, storageVolumeCreateXML)
(storageVolumeCreateXMLFrom, storageVolumeDelete): Likewise.
* src/util/bridge.c (brProbeVnetHdr): Likewise.
* po/POTFILES.in: Drop src/util/bridge.c.
These VIR_XXXX0 APIs make us confused, use the non-0-suffix APIs instead.
How do these coversions works? The magic is using the gcc extension of ##.
When __VA_ARGS__ is empty, "##" will swallow the "," in "fmt," to
avoid compile error.
example: origin after CPP
high_level_api("%d", a_int) low_level_api("%d", a_int)
high_level_api("a string") low_level_api("a string")
About 400 conversions.
8 special conversions:
VIR_XXXX0("") -> VIR_XXXX("msg") (avoid empty format) 2 conversions
VIR_XXXX0(string_literal_with_%) -> VIR_XXXX(%->%%) 0 conversions
VIR_XXXX0(non_string_literal) -> VIR_XXXX("%s", non_string_literal)
(for security) 6 conversions
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
This matches the public API and helps to get rid of some special
case code in the remote generator.
Rename driver API functions and XDR protocol structs.
No functional change included outside of the remote generator.
We already have virAsprintf, so picking a similar name helps for
seeing a similar purpose. Furthermore, the prefix V before printf
generally implies 'va_list', even though this variant was '...', and
the old name got in the way of adding a new va_list version.
global rename performed with:
$ git grep -l virBufferVSprintf \
| xargs -L1 sed -i 's/virBufferVSprintf/virBufferAsprintf/g'
then revert the changes in ChangeLog-old.
This patch addresses:
https://bugzilla.redhat.com/show_bug.cgi?id=694382
In order to give each libvirt-created bridge a fixed MAC address,
commit 5754dbd56d, added code to create
a dummy tap device with guaranteed lowest MAC address and attach it to
the bridge. This tap device was given the name "${bridgename}-nic".
However, an interface device name must be IFNAMSIZ (15) characters or
less, so a valid ${bridgename} such as "verylongname123" (15
characters) would lead to an invalid tap device name
("verylongname123-nic" - 19 characters), and that in turn led to a
failure to bring up the network.
The solution is to shorten the part of the original name used to
generate the tap device name. However, simply truncating it is
insufficient, because the last few characters of an interface name are
often a number used to indicate one of a list of several similar
devices (for example, "verylongname123", "verylongname124", etc) and
simple truncation would lead to duplicate names (eg "verlongnam-nic"
and "verylongnam-nic"). So instead we take the first 8 characters of
$bridgename ("verylong" in the example), add on the final 3 bytes
("123"), then add "-nic" (so "verylong123-nic"). Not pretty, but it
is much more likely to generate a unique name, and is reproducible
(unlike, say, a random number).
This fixes: https://bugzilla.redhat.com/show_bug.cgi?id=696660
While starting a network, if brSetForwardDelay() fails, we go to err1
where we want to access macTapIfName variable which was just
VIR_FREE'd a few lines above. Instead, keep macTapIfName until we are
certain of success.
Currently libvirt's default logging is limited and it is difficult to
determine what was happening when a proglem occurred (especially on a
machines where one don't know the detail.) This patch helps to do that
by making additional logging available for the following events:
creating/defining/undefining domains
creating/defining/undefining/starting/stopping networks
creating/defining/undefining/starting/stopping storage pools
creating/defining/undefining/starting/stopping storage volumes.
* AUTHORS: add Naoya Horiguchi
* src/network/bridge_driver.c src/qemu/qemu_driver.c
src/storage/storage_driver.c: provide more VIR_INFO logging
This simplifies several callers that were repeating checks already
guaranteed by util.c, and makes other callers more robust to now
reject directories. remote_driver.c was over-strict - access(,R_OK)
is only needed to execute a script file; a binary only needs
access(,X_OK) (besides, it's unusual to see a file with x but not
r permissions, whether script or binary).
* cfg.mk (sc_prohibit_access_xok): New syntax-check rule.
(exclude_file_name_regexp--sc_prohibit_access_xok): Exempt one use.
* src/network/bridge_driver.c (networkStartRadvd): Fix offenders.
* src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes)
(qemuCapsInitGuest, qemuCapsInit, qemuCapsExtractVersionInfo):
Likewise.
* src/remote/remote_driver.c (remoteFindDaemonPath): Likewise.
* src/uml/uml_driver.c (umlStartVMDaemon): Likewise.
* src/util/hooks.c (virHookCheck): Likewise.
This is detailed in:
https://bugzilla.redhat.com/show_bug.cgi?id=688957
Since radvd is executed by daemonizing it, the attempt to exec the
radvd binary doesn't happen until after libvirtd has already received
an exit code from the intermediate forked process, so no error is
detected or logged by __virExec().
We can't require radvd as a prerequisite for the libvirt package (many
installations don't use IPv6, so they don't need it), so instead we
add in a check to verify there is an executable radvd binary prior to
trying to exec it.
Normally dnsmasq will send a default route (the address of the host in
the network definition) to any client requesting an address via
DHCP. On an isolated network this makes no sense, as we have iptables
to prevent any traffic going out via that interface, so anything sent
that way would be dropped anyway.
This extra/unusable default route becomes problematic if you have
setup a guest with multiple network interfaces, with one connected to
an isolated network and another that provides connectivity to the
outside (example - one interface directly connecting to a physical
interface via macvtap, with a second connected to an isolated network
so that the host and guest can communicate (macvtap doesn't support
guest<->host communication without an external switch that supports
vepa, or reflecting all traffic back)). In this case, if the guest
chooses the default route of the isolated network, the guest will not
be able to get network traffic beyond the host.
To prevent dnsmasq from sending a default route, you can tell it to
send 0 bytes of data for the default route option (option number 3)
with --dhcp-option=3 (normally the data to send for the option would
follow the option number; no extra data means "don't send this option").
I have checked on RHEL5 (a good representative of the oldest supported
libvirt platforms) and its version of dnsmasq (2.45) does support
--dhcp-option, so this shouldn't create any compatibility problems.
By default, all dnsmasq processes share the same leases file. libvirt
also uses the --dhcp-lease-max option to control the maximum number of
leases allowed. The problem is that libvirt puts in a number equal to
the number of addresses in the range for the one network handled by a
single instance of dnsmasq, but dnsmasq checks the total number of
leases in the file (which could potentially contain many more).
The solution is to tell each instance of dnsmasq to create and use its
own leases file. (/var/lib/libvirt/network/<net-name>.leases).
This file is created by dnsmasq when it starts, but not deleted when
it exists. This is fine when the network is just being stopped, but if
the leases file was left around when a network was undefined, we could
end up with an ever-increasing number of dead files - instead, we
explicitly unlink the leases file when a network is undefined.
Note that Ubuntu carries a patch against an older version of libvirt for this:
hhttps://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/713071
ttp://bazaar.launchpad.net/~serge-hallyn/ubuntu/maverick/libvirt/bugall/revision/109
I was certain I'd also seen discussion of this on libvir-list or
libvirt-users, but couldn't find it.
This fixes a regression introduced in commit ad48df, and reported on
the libvirt-users list:
https://www.redhat.com/archives/libvirt-users/2011-March/msg00018.html
The problem in that commit was that we began searching a list of ip
address definitions (rather than just having one) to look for a dhcp
range or static host; when we didn't find any, our pointer (ipdef) was
left at NULL, and when ipdef was NULL, we returned without starting up
dnsmasq.
Previously dnsmasq was started even without any dhcp ranges or static
entries, because it's still useful for DNS services.
Another problem I noticed while investigating was that, if there are
IPv6 addresses, but no IPv4 addresses of any kind, we would jump out
at an ever higher level in the call chain.
This patch does the following:
1) networkBuildDnsmasqArgv() = all uses of ipdef are protected from
NULL dereference. (this patch doesn't change indentation, to make
review easier. The next patch will change just the
indentation). ipdef is intended to point to the first IPv4 address
with DHCP info (or the first IPv4 address if none of them have any
dhcp info).
2) networkStartDhcpDaemon() = if the loop looking for an ipdef with
DHCP info comes up empty, we then grab the first IPv4 def from the
list. Also, instead of returning if there are no IPv4 defs, we just
return if there are no IP defs at all (either v4 or v6). This way a
network that is IPv6-only will still get dnsmasq listening for DNS
queries.
3) in networkStartNetworkDaemon() - we will startup dhcp not just if there
are any IPv4 addresses, but also if there are any IPv6 addresses.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=609463
The problem was that, since a bridge always acquires the MAC address
of the connected interface with the numerically lowest MAC, as guests
are started and stopped, it was possible for the MAC address to change
over time, and this change in the network was being detected by
Windows 7 (it sees the MAC of the default route change), so on each
reboot it would bring up a dialog box asking about this "new network".
The solution is to create a dummy tap interface with a MAC guaranteed
to be lower than any guest interface's MAC, and attach that tap to the
bridge as soon as it's created. Since all guest MAC addresses start
with 0xFE, we can just generate a MAC with the standard "0x52, 0x54,
0" prefix, and it's guaranteed to always win (physical interfaces are
never connected to these bridges, so we don't need to worry about
competing numerically with them).
Note that the dummy tap is never set to IFF_UP state - that's not
necessary in order for the bridge to take its MAC, and not setting it
to UP eliminates the clutter of having an (eg) "virbr0-nic" displayed
in the output of the ifconfig command.
I chose to not auto-generate the MAC address in the network XML
parser, as there are likely to be consumers of that API that don't
need or want to have a MAC address associated with the
bridge.
Instead, in bridge_driver.c when the network is being defined, if
there is no MAC, one is generated. To account for virtual network
configs that already exist when upgrading from an older version of
libvirt, I've added a %post script to the specfile that searches for
all network definitions in both the config directory
(/etc/libvirt/qemu/networks) and the state directory
(/var/lib/libvirt/network) that are missing a mac address, generates a
random address, and adds it to the config (and a matching address to
the state file, if there is one).
docs/formatnetwork.html.in: document <mac address.../>
docs/schemas/network.rng: add nac address to schema
libvirt.spec.in: %post script to update existing networks
src/conf/network_conf.[ch]: parse and format <mac address.../>
src/libvirt_private.syms: export a couple private symbols we need
src/network/bridge_driver.c:
auto-generate mac address when needed,
create dummy interface if mac address is present.
tests/networkxml2xmlin/isolated-network.xml
tests/networkxml2xmlin/routed-network.xml
tests/networkxml2xmlout/isolated-network.xml
tests/networkxml2xmlout/routed-network.xml: add mac address to some tests
