On Fri, Jun 07, 2019 at 07:44:21AM -0400, Laine Stump wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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?
Yeah, should not happen when everything is working correctly. At least
during dev, bugs did cause it so I felt it worth keeping this safety
net in here.
>
> > 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@xxxxxxxxx>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
