On 09/10/2014 11:54 AM, Laine Stump wrote:
Sometimes libvirt is installed on a host that is already using the
network 192.168.122.0/24. If the libvirt-daemon-config-network package
is installed, this creates a conflict, since that package has been
hard-coded to create a virtual network that also uses
192.168.122.0/24. In the past libvirt has attempted to warn of /
remediate this situation by checking for conflicting routes when the
network is started, but it turns out that isn't always useful (for
example in the case that the *other* interface/network creating the
conflict hasn't yet been started at the time libvirtd start its owm
s/owm/own/
networks).
This patch attempts to catch the problem earlier - at install
time. During the %post install for libvirt-daemon-config-network, we
look through the output of "ip route show" for a route that exactly
matches 192.1 68.122.0/24, and if found we search for a similar route
that *doesn't* match (e.g. 192.168.123.0/24). When we find an
available route, we just replace all occurences of "122" in the
s/occurences/occurrences/
default.xml that is being created with ${new_sub}. This could
obviously be made more complicated - automatically determine the
existing network address and mask from examining the template
default.xml, etc, but this scripting is simpler and gets the job done
as long as we continue to use 192.168.122.0/24 in the template. (If
anyone with mad bash skillz wants to suggest something to do that, by
all means please do).
Is it worth adding comments into the template that the string "122" is
magic and must not be altered without also considering distro packaging?
This is intended to at least "further reduce" the problems detailed in:
https://bugzilla.redhat.com/show_bug.cgi?id=811967
I acknowledge that it doesn't help for cases of pre-built cloud images
(or live images that are created on real hardware and then run in a
virtual machine), but it should at least eliminate the troubles
encountered by individuals doing one-off installs (and could be used
to stifle complaints for live images, as long as libvirtd was running
on the system where the live image compose took place (or the compose
was itself done in a virtual machine that had a 192.168.122.0/24
interface address).
No good suggestions on how to help those situations.
---
The question here is: "Will this help some people's situation without
causing new problems for anyone else?" I wouldn't mind pushing this
patch, but also wouldn't mind if it was just the catalyst for
discussion that leads to a better solution. We do need *some kind* of
solution though, as more and more people are installing OSes that
include the libvirt package in virtual machines, and are running into
this problem with increasing frequency.
libvirt.spec.in | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index a6a58cf..539d9ef 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1728,8 +1728,32 @@ fi
%if %{with_network}
%post daemon-config-network
if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ;
then
+ # see if the network used by default network creates a conflict,
+ # and try to resolve it
+ orig_sub=122
+ sub=${orig_sub}
+ net=192.168.${sub}.0/24
+ routes=$(ip route show | cut -d' ' -f1)
How do we know that 'ip' is installed and available? Do we need any
Requires:, and/or making the script robust to 'ip' failing?
+ for route in $routes; do
+ if [ "${net}" = "${route}" ]; then
+ # there was a match, so we need to look for an unused subnet
Rather than using cut and a shell for loop, why not just use grep? [1]
if ip route show | grep -q "^192\\.168\\.$sub\\.0/24 "; then
+ for new_sub in $(seq 123 254); do
seq is a GNU coreutils extension that can't be used in shell code
designed to be portable everywhere; but this is a spec file for use by
rpms where we know it will be installed. So I'm fine with using it.
(If this were a configure script, I would have suggested using:
new_sub=123
while [ $new_sub -lt 255 ]; do
...
new_sub=$((new_sub + 1))
done
but that's overkill for this scenario.)
+ new_net="192.168.${new_sub}.0/24"
+ usable=yes
+ for route in ${routes}; do
[1] Oh, I see. You captured the ip output once, and are now scanning it
multiple times. In _that_ case, piping ip to grep on each loop is not
as efficient.
But you could still do the lookup in shell instead of spawning child
processes, and without needing a shell for loop:
routes=$(ip route show)
nl='
'
case $nl$routes in
*"${nl}192.168.$new_sub.0/24 "*) # code if found
*) # code if not found
esac
+ [ "${new_net}" = "${route}" ]
&& usable=no
Might be slightly faster if you skip the tail end of the for loop after
a collision, as in:
if [ "$new_net" = "$route" ]; then
usable=no
break
fi
that is, if you don't go with my case statement way to make the shell do
the iteration over the entire input in one pass.
+ done
+ if [ "${usable}" = "yes" ]; then
+ sub=${new_sub}
+ break;
+ fi
+ done
+ fi
+ done
+
UUID=`/usr/bin/uuidgen`
- sed -e "s,</name>,</name>\n <uuid>$UUID</uuid>,"
\
+ sed -e "s/${orig_sub}/${sub}/g" \
+ -e "s,</name>,</name>\n <uuid>$UUID</uuid>,"
\
< %{_datadir}/libvirt/networks/default.xml \
> %{_sysconfdir}/libvirt/qemu/networks/default.xml
ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
Overall, looks like a good idea. Your approach works, without giving me
too much grief, so up to you if you want to spin a v2 incorporating some
of my ideas, or leave it as-is.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org