Re: [libvirt] [PATCH v3 06/36] util: add helper method for re-attaching a tap device to a bridge

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > src/libvirt_private.syms | 1 + > src/util/virnetdevtap.c | 69 ++++++++++++++++++++++++++++++++++++++++ > src/util/virnetdevtap.h | 12 +++++++ > 3 files changed, 82 insertions(+) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index d9494a04bb..6f5a734fdb 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -2461,6 +2461,7 @@ virNetDevTapDelete; > virNetDevTapGetName; > virNetDevTapGetRealDeviceName; > virNetDevTapInterfaceStats; > +virNetDevTapReattachBridge; > > > # util/virnetdevveth.h > diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c > index 972f3405aa..0484c7c5a4 100644 > --- a/src/util/virnetdevtap.c > +++ b/src/util/virnetdevtap.c > @@ -553,6 +553,75 @@ virNetDevTapAttachBridge(const char *tapname, > } > > > +/** > + * virNetDevTapReattachBridge: > + * @tapname: the tap interface name (or name template) > + * @brname: the bridge name > + * @macaddr: desired MAC address > + * @virtPortProfile: bridge/port specific configuration > + * @virtVlan: vlan tag info > + * @mtu: requested MTU for port (or 0 for "default") > + * @actualMTU: MTU actually set for port (after accounting for bridge's MTU) > + * > + * Ensures that the tap device (@tapname) is connected to the bridge > + * (@brname), potentially removing it from any existing bridge that > + * does not match. > + * > + * Returns 0 in case of success or -1 on failure > + */ > +int > +virNetDevTapReattachBridge(const char *tapname, > + const char *brname, > + const virMacAddr *macaddr, > + const unsigned char *vmuuid, > + virNetDevVPortProfilePtr virtPortProfile, > + virNetDevVlanPtr virtVlan, > + unsigned int mtu, > + unsigned int *actualMTU) > +{ > + bool useOVS = false; > + char *master = NULL; > + Could use VIR_AUTOFREE to simplify things a bit > + if (virNetDevGetMaster(tapname, &master) < 0) > + return -1; > + > + /* IFLA_MASTER for a tap on an OVS switch is always "ovs-system" */ > + if (STREQ_NULLABLE(master, "ovs-system")) { > + useOVS = true; > + VIR_FREE(master); > + if (virNetDevOpenvswitchInterfaceGetMaster(tapname, &master) < 0) > + return -1; > + } > + > + /* Nothing more todo if we're on the right bridge already */ > + if (STREQ_NULLABLE(brname, master)) > + return 0; > + > + /* disconnect from current (incorrect) bridge, if any */ > + if (master) { > + int ret; > + VIR_INFO("Removing %s from %s", tapname, master); > + if (useOVS) > + ret = virNetDevOpenvswitchRemovePort(master, tapname); > + else > + ret = virNetDevBridgeRemovePort(master, tapname); > + VIR_FREE(master); > + if (ret < 0) > + return -1; > + } > + These were non-fatal in the original code. Is it okay to change?