Re: [libvirt] [PATCH v6 19/23] network: add implementation of network port APIs

On 5/23/19 11:32 AM, Daniel P. Berrangé wrote: > This initial implementation just wires up the APIs and does tracking of > the port XML definitions. It is not yet integrated into the resource > allocation logic. > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > src/network/bridge_driver.c | 400 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 400 insertions(+) > > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index 3cff96ac2c..ce280173e6 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -2766,6 +2766,8 @@ networkStartNetwork(virNetworkDriverStatePtr driver, > VIR_DEBUG("Beginning network startup process"); > + virNetworkObjDeleteAllPorts(obj, driver->stateDir); > + I guess you're just doing this as a failsafe? There shouldn't be any ports in an inactive network, right?

> VIR_DEBUG("Setting current network def as transient"); > if (virNetworkObjSetDefTransient(obj, true) < 0) > goto cleanup; > @@ -3943,6 +3945,9 @@ networkDestroy(virNetworkPtr net) > if ((ret = networkShutdownNetwork(driver, obj)) < 0) > goto cleanup; > + > + virNetworkObjDeleteAllPorts(obj, driver->stateDir); > + (The next paragraph is just me talking to myself. The conclusion is that everything is fine) The other function where you called this (networkStartNetwork()) is called by the public APIs (i.e. it's one level below the public APIs), but in this case you're calling it directly in the public API rather than in the next level down (which would be networkShutdownNetwork()). That may be correct, it just makes me wonder if maybe the port deletion is being missed in some case. I guess there's only one other place where networkShutdownNetwork() is called, which is when an error is encountered during network*Start*Network(). So, it would only make a difference if network ports were added during networkStartNetwork(), but since by definition a newly started network has no active ports, that could never happen. So I guess everything is fine, and my spidey sense tingled for nothing :-) Reviewed-by: Laine Stump <laine(a)laine.org&gt;