mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-10-07 23:05:48 +00:00
a28d3e485f
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. |
||
|---|---|---|
| .. | ||
| capabilities.c | ||
| capabilities.h | ||
| cpu_conf.c | ||
| cpu_conf.h | ||
| device_conf.c | ||
| device_conf.h | ||
| domain_addr.c | ||
| domain_addr.h | ||
| domain_audit.c | ||
| domain_audit.h | ||
| domain_capabilities.c | ||
| domain_capabilities.h | ||
| domain_conf.c | ||
| domain_conf.h | ||
| domain_event.c | ||
| domain_event.h | ||
| domain_nwfilter.c | ||
| domain_nwfilter.h | ||
| interface_conf.c | ||
| interface_conf.h | ||
| netdev_bandwidth_conf.c | ||
| netdev_bandwidth_conf.h | ||
| netdev_vlan_conf.c | ||
| netdev_vlan_conf.h | ||
| netdev_vport_profile_conf.c | ||
| netdev_vport_profile_conf.h | ||
| network_conf.c | ||
| network_conf.h | ||
| network_event.c | ||
| network_event.h | ||
| networkcommon_conf.c | ||
| networkcommon_conf.h | ||
| node_device_conf.c | ||
| node_device_conf.h | ||
| numa_conf.c | ||
| numa_conf.h | ||
| nwfilter_conf.c | ||
| nwfilter_conf.h | ||
| nwfilter_ipaddrmap.c | ||
| nwfilter_ipaddrmap.h | ||
| nwfilter_params.c | ||
| nwfilter_params.h | ||
| object_event_private.h | ||
| object_event.c | ||
| object_event.h | ||
| secret_conf.c | ||
| secret_conf.h | ||
| snapshot_conf.c | ||
| snapshot_conf.h | ||
| storage_conf.c | ||
| storage_conf.h | ||
| virchrdev.c | ||
| virchrdev.h | ||