[libvirt] [PATCH RFC] network: try to eliminate default network conflict during package install
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 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 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). 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). --- 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) + for route in $routes; do + if [ "${net}" = "${route}" ]; then + # there was a match, so we need to look for an unused subnet + for new_sub in $(seq 123 254); do + new_net="192.168.${new_sub}.0/24" + usable=yes + for route in ${routes}; do + [ "${new_net}" = "${route}" ] && usable=no + 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 -- 1.9.3
On Wed, Sep 10, 2014 at 01:54:00PM -0400, 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 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 default.xml that is being created with ${new_sub}.
Thanks for writing this patch, Laine.
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).
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). ---
The question here is: "Will this help some people's situation without causing new problems for anyone else?"
I, for one, certainly welcome this change. I used to hit this case when setting up nested virtualization with libvirt -- although, by now I instinctively create a non-default libvirt network on my physical host (L0), so my guest hypervisor (L1) can use the default libvirt network without conflicts while running the nested guest (L2). ACK for the idea, FWIW.
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.
-- /kashyap
On 09/10/2014 01:54 PM, 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 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 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).
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). ---
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.
Agree with all the above FWIW. This isn't the perfect 'fix' but no one has come up with one yet. So I say we give this a spin in rawhide/f21 and see how it works out.
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) + for route in $routes; do + if [ "${net}" = "${route}" ]; then + # there was a match, so we need to look for an unused subnet + for new_sub in $(seq 123 254); do + new_net="192.168.${new_sub}.0/24" + usable=yes + for route in ${routes}; do + [ "${new_net}" = "${route}" ] && usable=no + 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
My shell is pretty weak, but that looks okay to me. ACK. But I'd feel more comfortable if Eric checked it out :) Thanks, Cole
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
On 09/11/2014 05:15 PM, Eric Blake wrote:
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?
I thought of that as I was sending the patch, but didn't want to ruin the momentum by going back to check if I needed to add a Requires: (I was also curious if anyone else would think of it :-). As it turns out, the iproute package (containing the ip command) is included in the "core" set of packages used even for a minimal Fedora install. However, we do have a "Requires: iproute", for the libvirt-daemon package, and libvirt-daemon-config-network has "Requires: libvirt-daemon", so we are guaranteed that the ip command will be present.
+ 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
Yeah, things like this are the reason I asked for suggestions :-)
+ 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.
What if I captured the ip output, then did: if echo ${routes} | grep -q " 192\\.168\\.$sub\\.0/24 "; then ...
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.
I was really just going for brevity rather than efficiency, since it's only going to run once, and the list of routes will likely not be very long (although I suppose if someone installed libvirt on a machine that was in use as a core router (with full list of routes for the entire internet) it could be a consideration :-)
+ 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.
I'm sending a new version that captures the output of ip route show, then uses it in nested case statements.
participants (4)
-
Cole Robinson -
Eric Blake -
Kashyap Chamarthy -
Laine Stump