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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux