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