1. Patchew
  2. Libvirt
  3. [PATCH 0/7] network: implement automatic subnet selection for virtual networks
  4. View patch

[PATCH 7/7] spec: stop trying to find unused network during deamon-network-config %post

Laine Stump posted 7 patches 3 months, 2 weeks ago
[PATCH 7/7] spec: stop trying to find unused network during deamon-network-config %post
Posted by Laine Stump 3 months, 2 weeks ago 
Since the default network now has autoaddr='yes', there is no need to
waste time during %post install looking for an unused network and
modifying the config of the default virtual network.

(It would be even simpler to just install the files directly to /etc
rather than installing to the examples and then copying to /etc, but
if we did that then someone who had manually net-undefined the default
network would keep getting it reinstalled every time there was an
update to libvirt-daemon-config-network. Of course these days someone
who wants to do that can simply dnf rm daemon-config-network if they
didn't want the stock default network config, but nobody will be
expecting that they have to do that, leading to hundreds of bug
reports about "I deleted the default network and it comes back every
time I update my packages!")

Signed-off-by: Laine Stump <laine@redhat.com>
---
 libvirt.spec.in | 36 +++++-------------------------------
 1 file changed, 5 insertions(+), 31 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 51cecfa598..cb62e0dcdb 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1949,38 +1949,12 @@ exit 0
 %libvirt_systemd_config_pre virtnetworkd
 
 %post daemon-config-network
+# Installing during %post via cp rather than directly as a part of the package manifest
+# prevents us from reinstalling on update in cases where the user has net-undefined
+# the default network (and so doesn't *want* it to be reinstalled)
 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
-    # NB: 192.168.122.0/24 is used in the default.xml template file;
-    # do not modify any of those values here without also modifying
-    # them in the template.
-    orig_sub=122
-    sub=${orig_sub}
-    nl='
-'
-    routes="${nl}$(ip route show | cut -d' ' -f1)${nl}"
-    case ${routes} in
-      *"${nl}192.168.${orig_sub}.0/24${nl}"*)
-        # there was a match, so we need to look for an unused subnet
-        for new_sub in $(seq 124 254); do
-          case ${routes} in
-          *"${nl}192.168.${new_sub}.0/24${nl}"*)
-            ;;
-          *)
-            sub=$new_sub
-            break;
-            ;;
-          esac
-        done
-        ;;
-      *)
-        ;;
-    esac
-
-    sed -e "s/${orig_sub}/${sub}/g" \
-         < %{_datadir}/libvirt/networks/default.xml \
-         > %{_sysconfdir}/libvirt/qemu/networks/default.xml
+    cp %{_datadir}/libvirt/networks/default.xml \
+       %{_sysconfdir}/libvirt/qemu/networks/default.xml
     ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
     # libvirt saves this file with mode 0600
     chmod 0600 %{_sysconfdir}/libvirt/qemu/networks/default.xml
-- 
2.45.2
Re: [PATCH 7/7] spec: stop trying to find unused network during deamon-network-config %post
Posted by Daniel P. Berrangé 3 months, 2 weeks ago 
On Wed, Aug 07, 2024 at 01:16:03PM -0400, Laine Stump wrote:
> Since the default network now has autoaddr='yes', there is no need to
> waste time during %post install looking for an unused network and
> modifying the config of the default virtual network.

I think this is possibly still compelling to keep.

Consider the user has something network related that they
frequently manually start post boot (a VPN perhaps). We
have the sequence

   1. Machine boots
   2. User starts something on subset 192.168.122.0/24
   3. User installs libvirt
   4. Machine shuts down

   5. Machine boots
   6. Libvirt starts on boot
   7. VM auto-starts
   8. User starts something on subset 192.168.122.0/24

with steps 5-8 repeating every day.

Currently at step (3), we'll rewrite the default network
to 192.168.123.0, and it'll stay on 192.168.123.0 forever
which gives stability for the install.

With this patch applied, then in step 6 we'll always
start with 192.168.122.0, and then in step 8 change
to 192.168.123.0, breaking the VM from step 7.

That said in the previous patch I suggested that we should
make autoaddr=yes be incompatible with specifying address=
in the XML, which would *require* this patch as we would
no longer have any address in the network XML to modify.

This makes me think that the network driver should remember
auto-assigned networks across reboots.

ie require

   <ip autoaddr="yes">
       <dhcp/>
   </ip>

in the default.xml file, but then have something in
/var/lib/libvirt/networks/subnets.db, where we remember
the auto-assigned subnets a virtual network previously
had. IOW, once an auto-subnet is chosen, it should never
change again, unless a new clash arises.


> 
> (It would be even simpler to just install the files directly to /etc
> rather than installing to the examples and then copying to /etc, but
> if we did that then someone who had manually net-undefined the default
> network would keep getting it reinstalled every time there was an
> update to libvirt-daemon-config-network. Of course these days someone
> who wants to do that can simply dnf rm daemon-config-network if they
> didn't want the stock default network config, but nobody will be
> expecting that they have to do that, leading to hundreds of bug
> reports about "I deleted the default network and it comes back every
> time I update my packages!")
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  libvirt.spec.in | 36 +++++-------------------------------
>  1 file changed, 5 insertions(+), 31 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 51cecfa598..cb62e0dcdb 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1949,38 +1949,12 @@ exit 0
>  %libvirt_systemd_config_pre virtnetworkd
>  
>  %post daemon-config-network
> +# Installing during %post via cp rather than directly as a part of the package manifest
> +# prevents us from reinstalling on update in cases where the user has net-undefined
> +# the default network (and so doesn't *want* it to be reinstalled)
>  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
> -    # NB: 192.168.122.0/24 is used in the default.xml template file;
> -    # do not modify any of those values here without also modifying
> -    # them in the template.
> -    orig_sub=122
> -    sub=${orig_sub}
> -    nl='
> -'
> -    routes="${nl}$(ip route show | cut -d' ' -f1)${nl}"
> -    case ${routes} in
> -      *"${nl}192.168.${orig_sub}.0/24${nl}"*)
> -        # there was a match, so we need to look for an unused subnet
> -        for new_sub in $(seq 124 254); do
> -          case ${routes} in
> -          *"${nl}192.168.${new_sub}.0/24${nl}"*)
> -            ;;
> -          *)
> -            sub=$new_sub
> -            break;
> -            ;;
> -          esac
> -        done
> -        ;;
> -      *)
> -        ;;
> -    esac
> -
> -    sed -e "s/${orig_sub}/${sub}/g" \
> -         < %{_datadir}/libvirt/networks/default.xml \
> -         > %{_sysconfdir}/libvirt/qemu/networks/default.xml
> +    cp %{_datadir}/libvirt/networks/default.xml \
> +       %{_sysconfdir}/libvirt/qemu/networks/default.xml
>      ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
>      # libvirt saves this file with mode 0600
>      chmod 0600 %{_sysconfdir}/libvirt/qemu/networks/default.xml
> -- 
> 2.45.2
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 7/7] spec: stop trying to find unused network during deamon-network-config %post
Posted by Laine Stump 3 months, 2 weeks ago 
On 8/7/24 1:54 PM, Daniel P. Berrangé wrote:
> On Wed, Aug 07, 2024 at 01:16:03PM -0400, Laine Stump wrote:
>> Since the default network now has autoaddr='yes', there is no need to
>> waste time during %post install looking for an unused network and
>> modifying the config of the default virtual network.
> 
> I think this is possibly still compelling to keep.
> 
> Consider the user has something network related that they
> frequently manually start post boot (a VPN perhaps). We
> have the sequence
> 
>     1. Machine boots
>     2. User starts something on subset 192.168.122.0/24
>     3. User installs libvirt
>     4. Machine shuts down
> 
>     5. Machine boots
>     6. Libvirt starts on boot
>     7. VM auto-starts
>     8. User starts something on subset 192.168.122.0/24
> 
> with steps 5-8 repeating every day.
> 
> Currently at step (3), we'll rewrite the default network
> to 192.168.123.0, and it'll stay on 192.168.123.0 forever
> which gives stability for the install.
> 
> With this patch applied, then in step 6 we'll always
> start with 192.168.122.0, and then in step 8 change
> to 192.168.123.0, breaking the VM from step 7.

Not when all of the other patches are also applied - when the network is 
started the first time and chooses 192.168.123.0, that will be saved in 
the network's config XML (just like we re-save the config when mac 
addresses and uuids are changed by libvirt), and so the next time it is 
started (in your example, the next time the host boots), it would start 
out by trying 192.168.123.0 again, and only switch to a different 
network if that was already in use.

And with the NM dispatcher script, even if another network starts at 
192.168.123.0 after this network starts, this network will switch to yet 
another network (and, again, save the config with that new address so it 
will be used again next time).

The only remaining issue (which I pointed out in one of the commit logs) 
is that if guests have already started by the time the NM script forces 
the network to change, then those guests would have been started with 
192.168.123.0, but then the network is changed to 192.168.124.0

> 
> That said in the previous patch I suggested that we should
> make autoaddr=yes be incompatible with specifying address=
> in the XML, which would *require* this patch as we would
> no longer have any address in the network XML to modify.
> 
> This makes me think that the network driver should remember
> auto-assigned networks across reboots.
> 
> ie require
> 
>     <ip autoaddr="yes">
>         <dhcp/>
>     </ip>
> 
> in the default.xml file, but then have something in
> /var/lib/libvirt/networks/subnets.db, where we remember
> the auto-assigned subnets a virtual network previously
> had. IOW, once an auto-subnet is chosen, it should never
> change again, unless a new clash arises.

Yeah, that's the way it works. Except without a new file to keep track of.

> 
> 
>>
>> (It would be even simpler to just install the files directly to /etc
>> rather than installing to the examples and then copying to /etc, but
>> if we did that then someone who had manually net-undefined the default
>> network would keep getting it reinstalled every time there was an
>> update to libvirt-daemon-config-network. Of course these days someone
>> who wants to do that can simply dnf rm daemon-config-network if they
>> didn't want the stock default network config, but nobody will be
>> expecting that they have to do that, leading to hundreds of bug
>> reports about "I deleted the default network and it comes back every
>> time I update my packages!")
>>
>> Signed-off-by: Laine Stump <laine@redhat.com>
>> ---
>>   libvirt.spec.in | 36 +++++-------------------------------
>>   1 file changed, 5 insertions(+), 31 deletions(-)
>>
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index 51cecfa598..cb62e0dcdb 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -1949,38 +1949,12 @@ exit 0
>>   %libvirt_systemd_config_pre virtnetworkd
>>   
>>   %post daemon-config-network
>> +# Installing during %post via cp rather than directly as a part of the package manifest
>> +# prevents us from reinstalling on update in cases where the user has net-undefined
>> +# the default network (and so doesn't *want* it to be reinstalled)
>>   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
>> -    # NB: 192.168.122.0/24 is used in the default.xml template file;
>> -    # do not modify any of those values here without also modifying
>> -    # them in the template.
>> -    orig_sub=122
>> -    sub=${orig_sub}
>> -    nl='
>> -'
>> -    routes="${nl}$(ip route show | cut -d' ' -f1)${nl}"
>> -    case ${routes} in
>> -      *"${nl}192.168.${orig_sub}.0/24${nl}"*)
>> -        # there was a match, so we need to look for an unused subnet
>> -        for new_sub in $(seq 124 254); do
>> -          case ${routes} in
>> -          *"${nl}192.168.${new_sub}.0/24${nl}"*)
>> -            ;;
>> -          *)
>> -            sub=$new_sub
>> -            break;
>> -            ;;
>> -          esac
>> -        done
>> -        ;;
>> -      *)
>> -        ;;
>> -    esac
>> -
>> -    sed -e "s/${orig_sub}/${sub}/g" \
>> -         < %{_datadir}/libvirt/networks/default.xml \
>> -         > %{_sysconfdir}/libvirt/qemu/networks/default.xml
>> +    cp %{_datadir}/libvirt/networks/default.xml \
>> +       %{_sysconfdir}/libvirt/qemu/networks/default.xml
>>       ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
>>       # libvirt saves this file with mode 0600
>>       chmod 0600 %{_sysconfdir}/libvirt/qemu/networks/default.xml
>> -- 
>> 2.45.2
>>
> 
> With regards,
> Daniel

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.