[libvirt] [PATCH v2 0/2] network_conf: check if bridge exists on host for user created bridges
The patch fixes the below problem. ============================== If the bridge name is not mentioned in the <network> xml, the bridge name is auto generated from virNetworkAllocateBridge(). If the default template named bridge is created manually by a user, the bridge start will fail with "File exists". bash-4.3$ sudo brctl addbr virbr1 bash-4.3$ brctl show bridge name bridge id STP enabled interfaces br0 8000.000000000000 no virbr0 8000.525400a91d03 yes virbr0-nic virbr1 8000.000000000000 no bash-4.3$ sudo virsh net-list --all Name State Autostart Persistent ---------------------------------------------------------- default active no yes bash-4.3$ cat /tmp/isolated # Notice that the <bridge> intentionally not given. <network> <name>isolated</name> <forward/> <ip address="192.168.123.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.123.2" end="192.168.123.254"/> </dhcp> </ip> </network> bash-4.3$ sudo virsh net-create /tmp/isolated error: Failed to create network from isolated error: Unable to create bridge virbr1: File exists =============================== --- Shivaprasad G Bhat (2): cleanup conf/device_conf.h reference from util/virnetdev.h network_conf: check if bridge exists on host for user created bridges src/Makefile.am | 3 +-- src/conf/device_conf.h | 21 +-------------------- src/conf/network_conf.c | 11 +++++++++-- src/util/virnetdev.h | 21 ++++++++++++++++++++- 4 files changed, 31 insertions(+), 25 deletions(-) -- Signature
Its wrong to reference the conf/* files from util/* files. Clean up the Makefile.am. Also, move the struct definitions to utils and include the util file in conf instead. There is a similar incorrect reference to domain_conf.h from virclosecallbacks.h which is to be addressed in a future patch. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/Makefile.am | 3 +-- src/conf/device_conf.h | 21 +-------------------- src/util/virnetdev.h | 21 ++++++++++++++++++++- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index d38432e..85d6b44 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1014,8 +1014,7 @@ libvirt_util_la_SOURCES = \ libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \ $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \ - $(SYSTEMD_DAEMON_CFLAGS) $(POLKIT_CFLAGS) \ - -I$(srcdir)/conf + $(SYSTEMD_DAEMON_CFLAGS) $(POLKIT_CFLAGS) libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \ diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 7256cdc..a650189 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -31,19 +31,7 @@ # include "virutil.h" # include "virthread.h" # include "virbuffer.h" - -typedef enum { - VIR_INTERFACE_STATE_UNKNOWN = 1, - VIR_INTERFACE_STATE_NOT_PRESENT, - VIR_INTERFACE_STATE_DOWN, - VIR_INTERFACE_STATE_LOWER_LAYER_DOWN, - VIR_INTERFACE_STATE_TESTING, - VIR_INTERFACE_STATE_DORMANT, - VIR_INTERFACE_STATE_UP, - VIR_INTERFACE_STATE_LAST -} virInterfaceState; - -VIR_ENUM_DECL(virInterfaceState) +# include "virnetdev.h" typedef struct _virDevicePCIAddress virDevicePCIAddress; typedef virDevicePCIAddress *virDevicePCIAddressPtr; @@ -55,13 +43,6 @@ struct _virDevicePCIAddress { int multi; /* virTristateSwitch */ }; -typedef struct _virInterfaceLink virInterfaceLink; -typedef virInterfaceLink *virInterfaceLinkPtr; -struct _virInterfaceLink { - virInterfaceState state; /* link state */ - unsigned int speed; /* link speed in Mbits per second */ -}; - int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr); int virDevicePCIAddressParseXML(xmlNodePtr node, diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index de8b480..42cc1f8 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -29,7 +29,6 @@ # include "virnetlink.h" # include "virmacaddr.h" # include "virpci.h" -# include "device_conf.h" # ifdef HAVE_STRUCT_IFREQ typedef struct ifreq virIfreq; @@ -73,6 +72,26 @@ struct _virNetDevRxFilter { } vlan; }; +typedef enum { + VIR_INTERFACE_STATE_UNKNOWN = 1, + VIR_INTERFACE_STATE_NOT_PRESENT, + VIR_INTERFACE_STATE_DOWN, + VIR_INTERFACE_STATE_LOWER_LAYER_DOWN, + VIR_INTERFACE_STATE_TESTING, + VIR_INTERFACE_STATE_DORMANT, + VIR_INTERFACE_STATE_UP, + VIR_INTERFACE_STATE_LAST +} virInterfaceState; + +VIR_ENUM_DECL(virInterfaceState) + +typedef struct _virInterfaceLink virInterfaceLink; +typedef virInterfaceLink *virInterfaceLinkPtr; +struct _virInterfaceLink { + virInterfaceState state; /* link state */ + unsigned int speed; /* link speed in Mbits per second */ +}; + int virNetDevSetupControl(const char *ifname, virIfreq *ifr) ATTRIBUTE_RETURN_CHECK;
Hi All, This patch needs some rework as the Makefile.am change would result in build failures on virhostdev.h and virclosecallbacks.h. It didnt fail for me earlier as the autogen was run prior to the changes. I'll be sending a v2 with the fixes for build failures. Thanks, Shiva On Wed, Mar 11, 2015 at 2:40 PM, Shivaprasad G Bhat <shivaprasadbhat@gmail.com> wrote:
Its wrong to reference the conf/* files from util/* files. Clean up the Makefile.am. Also, move the struct definitions to utils and include the util file in conf instead. There is a similar incorrect reference to domain_conf.h from virclosecallbacks.h which is to be addressed in a future patch.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/Makefile.am | 3 +-- src/conf/device_conf.h | 21 +-------------------- src/util/virnetdev.h | 21 ++++++++++++++++++++- 3 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index d38432e..85d6b44 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1014,8 +1014,7 @@ libvirt_util_la_SOURCES = \ libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \ $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \ - $(SYSTEMD_DAEMON_CFLAGS) $(POLKIT_CFLAGS) \ - -I$(srcdir)/conf + $(SYSTEMD_DAEMON_CFLAGS) $(POLKIT_CFLAGS) libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \ diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 7256cdc..a650189 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -31,19 +31,7 @@ # include "virutil.h" # include "virthread.h" # include "virbuffer.h" - -typedef enum { - VIR_INTERFACE_STATE_UNKNOWN = 1, - VIR_INTERFACE_STATE_NOT_PRESENT, - VIR_INTERFACE_STATE_DOWN, - VIR_INTERFACE_STATE_LOWER_LAYER_DOWN, - VIR_INTERFACE_STATE_TESTING, - VIR_INTERFACE_STATE_DORMANT, - VIR_INTERFACE_STATE_UP, - VIR_INTERFACE_STATE_LAST -} virInterfaceState; - -VIR_ENUM_DECL(virInterfaceState) +# include "virnetdev.h"
typedef struct _virDevicePCIAddress virDevicePCIAddress; typedef virDevicePCIAddress *virDevicePCIAddressPtr; @@ -55,13 +43,6 @@ struct _virDevicePCIAddress { int multi; /* virTristateSwitch */ };
-typedef struct _virInterfaceLink virInterfaceLink; -typedef virInterfaceLink *virInterfaceLinkPtr; -struct _virInterfaceLink { - virInterfaceState state; /* link state */ - unsigned int speed; /* link speed in Mbits per second */ -}; - int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr);
int virDevicePCIAddressParseXML(xmlNodePtr node, diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index de8b480..42cc1f8 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -29,7 +29,6 @@ # include "virnetlink.h" # include "virmacaddr.h" # include "virpci.h" -# include "device_conf.h"
# ifdef HAVE_STRUCT_IFREQ typedef struct ifreq virIfreq; @@ -73,6 +72,26 @@ struct _virNetDevRxFilter { } vlan; };
+typedef enum { + VIR_INTERFACE_STATE_UNKNOWN = 1, + VIR_INTERFACE_STATE_NOT_PRESENT, + VIR_INTERFACE_STATE_DOWN, + VIR_INTERFACE_STATE_LOWER_LAYER_DOWN, + VIR_INTERFACE_STATE_TESTING, + VIR_INTERFACE_STATE_DORMANT, + VIR_INTERFACE_STATE_UP, + VIR_INTERFACE_STATE_LAST +} virInterfaceState; + +VIR_ENUM_DECL(virInterfaceState) + +typedef struct _virInterfaceLink virInterfaceLink; +typedef virInterfaceLink *virInterfaceLinkPtr; +struct _virInterfaceLink { + virInterfaceState state; /* link state */ + unsigned int speed; /* link speed in Mbits per second */ +}; + int virNetDevSetupControl(const char *ifname, virIfreq *ifr) ATTRIBUTE_RETURN_CHECK;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
virNetworkBridgeInUse() doesn't check if the bridge is manually created outside of libvirt. Check if the bridge actually exist on host using the virNetDevExists(). Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/conf/network_conf.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index d0e7e1b..3ab087c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3074,16 +3074,23 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets, { size_t i; unsigned int ret = 0; + int defined_bridge = 0; for (i = 0; i < nets->count; i++) { virNetworkObjLock(nets->objs[i]); if (nets->objs[i]->def->bridge && - STREQ(nets->objs[i]->def->bridge, bridge) && - !(skipname && STREQ(nets->objs[i]->def->name, skipname))) + STREQ(nets->objs[i]->def->bridge, bridge)) { + defined_bridge = 1; + if (!(skipname && STREQ(nets->objs[i]->def->name, skipname))) ret = 1; + } virNetworkObjUnlock(nets->objs[i]); } + /* Bridge might have been created by a user manually outside libvirt */ + if (!defined_bridge && !ret) + ret = virNetDevExists(bridge) ? 1 : 0; + return ret; }
participants (2)
-
Shivaprasad bhat -
Shivaprasad G Bhat