[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] net: update default.xml to match creation by libvirt



On 11/08/2012 01:51 PM, Laine Stump wrote:
> On 11/08/2012 01:06 PM, Eric Blake wrote:
>> I noticed that after a fresh install, the file
>> /etc/libvirt/qemu/networks/default.xml lacked the usual header
>> inserted by modern 'virsh net-edit'; and traced it to the fact
>> that libvirt.spec installs this file by copying a template
>> rather than using libvirt API.  We might as well make our template
>> match what libvirt itself would generate in that location.
>>
>> * src/network/default.xml: Add header and newer XML details.
>> ---
>>  src/network/default.xml | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/network/default.xml b/src/network/default.xml
>> index 9cfc01e..e124621 100644
>> --- a/src/network/default.xml
>> +++ b/src/network/default.xml
>> @@ -1,7 +1,14 @@
>> +<!--
>> +WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE
>> +OVERWRITTEN AND LOST. Changes to this xml configuration should be made using:
>> +  virsh net-edit default
>> +or other application using the libvirt API.
>> +-->
>> +
>>  <network>
>>    <name>default</name>
>> -  <bridge name="virbr0" />
>> -  <forward/>
>> +  <forward mode='nat'/>
> mode='nat' is the defined default, so that isn't strictly necessary...
>
>> +  <bridge name="virbr0" stp='on' delay='0'/>
> The above line shouldn't be in the file. This will create conflicts if
> someone has a libvirt installation that already has networks defined
> (and so is already using virbr0 for a different network), and you go
> through the back door to create a network definition that re-uses it.
> (which makes me wonder if the auto-created bridge name used by a default
> network created in this manner will end up being written to the config
> file in /etc...)

I just checked this out and the results were actually a bit disturbing:

As you'd expect, if you blindly put <bridge name='virbr0'/> in the file
when an existing network already uses virbr0, then one of the two
networks will fail to start (depending on which is started first.

HOWEVER, if you don't put in any <bridge> at all, net-start will take
the lack of a bridge device to mean that you actually want "virbr0", so
the results are identical :-(

Additionally, although <forward/> is always formatted as <forward
mode='nat'/> for net-dumpxml, it isn't actually replaced in the
persistent config.

So, there are really 3(!) bugs:

1) the bug in the BZ
2) if you don't specify a bridge device to use, it will always try to
use virbr0 rather than looking for one that isn't in use,
3) The choice of bridge devices is never updated in the persistent config.

One way to solve all of them would be to use "virsh define default.xml
&& virsh autostart default && virsh start default" rather than directly
inserting the file, but we would need to be certain that libvirtd was
always running by the time the %post script for
libvirt-daemon-config-network was run (and that may be the case, I
haven't looked). In that case, your patch to change the .xml file on
disk wouldn't be necessary.

>
>>    <ip address="192.168.122.1" netmask="255.255.255.0">
>>      <dhcp>
>>        <range start="192.168.122.2" end="192.168.122.254" />
>
> Actually there is another open bug about the way we're creating this
> network:
>
>   https://bugzilla.redhat.com/show_bug.cgi?id=867546
>
> The problem is that when libvirtd is already running and you install the
> libvirt-daemon-config-network-config package (which I believe is what
> happens during the initial install of everything), the default network
> doesn't show up in libvirt's list until the next time you restart
> libvirtd (because we've just copied it into place on the disk, so
> libvirtd hasn't been told about it).
>
>
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list
>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]