[libvirt PATCH 1/2] network: Drop UUID handling for default network

Mon Nov 16 15:00:46 UTC 2020

On 11/16/20 5:33 AM, Andrea Bolognani wrote:
> On Sun, 2020-11-15 at 19:19 -0500, Laine Stump wrote:
>> On 11/15/20 3:43 PM, Andrea Bolognani wrote:
>>> We are generating a fresh UUID and storing it in the XML for the
>>> default network, but this is unnecessary because the network
>>> driver will automatically generate one if it's missing from the
>>> XML;
>> But that automatically generated uuid will not be stored in the original
>> xml in /etc/libvirt, so a new and different uuid will be generated every
>> time libvirt is restarted.
> That doesn't seem to be the case:
>
>    $ sudo cat /etc/libvirt/qemu/networks/default.xml
>    <network>
>      <name>default</name>
>      <forward mode='nat'/>
>      <bridge name='virbr0'/>
>      <ip address='192.168.122.1' netmask='255.255.255.0'>
>        <dhcp>
>          <range start='192.168.122.2' end='192.168.122.254'/>
>        </dhcp>
>      </ip>
>    </network>
>    $ sudo systemctl start libvirtd
>    $ sudo systemctl stop libvirtd
>    $ sudo cat /etc/libvirt/qemu/networks/default.xml
>    <!--
>    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>
>      <uuid>212a684a-4ca4-4c42-b389-aecd676868f8</uuid>
>      <forward mode='nat'/>
>      <bridge name='virbr0' stp='on' delay='0'/>
>      <mac address='52:54:00:ff:27:85'/>
>      <ip address='192.168.122.1' netmask='255.255.255.0'>
>        <dhcp>
>          <range start='192.168.122.2' end='192.168.122.254'/>
>        </dhcp>
>      </ip>
>    </network>
>    $ sudo systemctl start libvirtd
>    $ sudo systemctl stop libvirtd
>    $ sudo cat /etc/libvirt/qemu/networks/default.xml
>    <!--
>    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>
>      <uuid>212a684a-4ca4-4c42-b389-aecd676868f8</uuid>
>      <forward mode='nat'/>
>      <bridge name='virbr0' stp='on' delay='0'/>
>      <mac address='52:54:00:ff:27:85'/>
>      <ip address='192.168.122.1' netmask='255.255.255.0'>
>        <dhcp>
>          <range start='192.168.122.2' end='192.168.122.254'/>
>        </dhcp>
>      </ip>
>    </network>

But... I did a nearly identical experiment last night before posting my 
reply, and came up with different results, which is why I posted...

Ah! So the difference is that in your example, you have no MAC address 
either, and the code you point out down below is noticing that change, 
and causing the entire network config to be re-written to the disk. I 
hadn't removed the MAC address, so in my case it didn't re-write the config.

(...and now I've read to the bottom of your reply and see that you've 
already figured that out :-P)

I guess if we're okay with re-writing the config at daemon start to add 
a MAC address, then we should be okay with doing that to add a uuid 
(they're really just differently-used examples of the same thing). But 
if we're going to do that, the check to see if the config should be 
modified to check both uuid and mac address (even if in current practice 
both are absent from the proto-default.xml, that may not be the case in 
the future).

Alternately, if we're *not* okay with rewriting the config at daemon 
start time (even though, as you've pointed out, we've been doing that 
for 6 years already), then we would need to *stop* doing that for MAC 
address. (Which would mean we would need to figure out a different way 
to get a fixed MAC address into the config file, and seeing that the 
commit you reference actually *removes* my original code that added it 
in at install time, we can't really go back there (also because that's 
moving the code in the wrong direction - Fedora SilverBlue people said 
in the BZ I referenced that they don't want %post_install scripts 
modifying the files that are installed).

I think I'm okay with the former (adding a check for changed uuid onto 
your patches); it gives us a stable uuid (as long as the modified disk 
contents are saved for the next reboot) while eliminating another bit of 
the undesirable code that runs during %post_install.

Is there anything else wrt. pre-built images that we're not taking into 
consideration though?

>> I don't know if the solution to this is to modify libvirt so that it
>> rewrites the XML for a network if it has to auto-generate any
>> attributes[*], or to put a canned/static uuid value in the installed
>> xml, or just to declare that we don't care if the uuid changes from one
>> run of libvirtd to the next. But definitely these patches change
>> behavior, so we probably need to point that out and maybe discuss it.
>>
>> [*] I have a vague memory that there is (or was?) at least one case
>> where we would automatically update the persistent definition of a
>> domain or network on disk when libvirt started (it was needed due to a
>> change in some attribute or something), but I don't recall the details,
>> don't feel like looking for it right now, and it anyway is not the norm.
> AFAICT the behavior you're talking about, and which can be seen in
> action in the transcript above, is caused by the following snippet in
> virNetworkLoadConfig():
>
>    switch ((virNetworkForwardType) def->forward.type) {
>    case VIR_NETWORK_FORWARD_NONE:
>    case VIR_NETWORK_FORWARD_NAT:
>    case VIR_NETWORK_FORWARD_ROUTE:
>    case VIR_NETWORK_FORWARD_OPEN:
>        if (!def->mac_specified) {
>            virNetworkSetBridgeMacAddr(def);
>            virNetworkSaveConfig(configDir, def, xmlopt);
>        }
>        break;
>
> which can be tracked down to
>
>    https://gitlab.com/libvirt/libvirt/-/commit/a47ae7c004e92f959b45808ca82326e8559c2f61
>
> So, it's far from a new behavior.
>
> Note that we only persist the new configuration if the bridge's MAC
> address has been generated, not if the network UUID has, which is
> something that we might want to change. But, for the use case at
> hand, we know that the XML template for the default network will have
> neither of those, so in practice they will both be generated during
> the first startup and the result will be persisted to disk.
>