a28d3e4 network: move auto-assign of bridge name from XML parser to net driver

Authored and Committed by Laine Stump 8 years ago
    network: move auto-assign of bridge name from XML parser to net driver
    
    We already check that any auto-assigned bridge device name for a
    virtual network (e.g. "virbr1") doesn't conflict with the bridge name
    for any existing libvirt network (via virNetworkSetBridgeName() in
    conf/network_conf.c).
    
    We also want to check that the name doesn't conflict with any bridge
    device created on the host system outside the control of libvirt
    (history: possibly due to the ploriferation of references to libvirt's
    bridge devices in HOWTO documents all around the web, it is not
    uncommon for an admin to manually create a bridge in their host's
    system network config and name it "virbrX"). To add such a check to
    virNetworkBridgeInUse() (which is called by virNetworkSetBridgeName())
    we would have to call virNetDevExists() (from util/virnetdev.c); this
    function calls ioctl(SIOCGIFFLAGS), which everyone on the mailing list
    agreed should not be done from an XML parsing function in the conf
    directory.
    
    To remedy that problem, this patch removes virNetworkSetBridgeName()
    from conf/network_conf.c and puts an identically functioning
    networkBridgeNameValidate() in network/bridge_driver.c (because it's
    reasonable for the bridge driver to call virNetDevExists(), although
    we don't do that yet because I wanted this patch to have as close to 0
    effect on function as possible).
    
    There are a couple of inevitable changes though:
    
    1) We no longer check the bridge name during
       virNetworkLoadConfig(). Close examination of the code shows that
       this wasn't necessary anyway - the only *correct* way to get XML
       into the config files is via networkDefine(), and networkDefine()
       will always call networkValidate(), which previously called
       virNetworkSetBridgeName() (and now calls
       networkBridgeNameValidate()). This means that the only way the
       bridge name can be unset during virNetworkLoadConfig() is if
       someone edited the config file on disk by hand (which we explicitly
       prohibit).
    
    2) Just on the off chance that somebody *has* edited the file by hand,
       rather than crashing when they try to start their malformed
       network, a check for non-NULL bridge name has been added to
       networkStartNetworkVirtual().
    
       (For those wondering why I don't instead call
       networkValidateBridgeName() there to set a bridge name if one
       wasn't present - the problem is that during
       networkStartNetworkVirtual(), the lock for the network being
       started has already been acquired, but the lock for the network
       list itself *has not* (because we aren't adding/removing a
       network). But virNetworkBridgeInuse() iterates through *all*
       networks (including this one) and locks each network as it is
       checked for a duplicate entry; it is necessary to lock each network
       even before checking if it is the designated "skip" network because
       otherwise some other thread might acquire the list lock and delete
       the very entry we're examining. In the end, permitting a setting of
       the bridge name during network start would require that we lock the
       entire network list during any networkStartNetwork(), which
       eliminates a *lot* of parallelism that we've worked so hard to
       achieve (it can make a huge difference during libvirtd startup). So
       rather than try to adjust for someone playing against the rules, I
       choose to instead give them the error they deserve.)
    
    3) virNetworkAllocateBridge() (now removed) would leak any "template"
       string set as the bridge name. Its replacement
       networkFindUnusedBridgeName() doesn't leak the template string - it
       is properly freed.
    
        
file modified
+0 -60
file modified
+1 -8
file modified
+1 -1
file modified
+90 -1