Re: [libvirt] [PATCH v6 18/23] conf: support recording ports against virNetworkObjPtr

On Thu, Jun 06, 2019 at 09:51:52PM -0400, Laine Stump wrote: > On 5/23/19 11:32 AM, Daniel P. Berrangé wrote: > > The virNetworkObjPtr state will need to maintain a record of all > > virNetworkPortDefPtr objects associated with the network. Record these > > in a hash and add APIs for manipulating them. > > > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > > --- > > src/conf/virnetworkobj.c | 303 +++++++++++++++++++++++++++++++++++++++ > > src/conf/virnetworkobj.h | 34 +++++ > > src/libvirt_private.syms | 6 + > > 3 files changed, 343 insertions(+) > > > > diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c > > index c9336e0472..47c142998e 100644 > > --- a/src/conf/virnetworkobj.c > > +++ b/src/conf/virnetworkobj.c > > @@ -58,6 +58,8 @@ struct _virNetworkObj { > > /* Immutable pointer, self locking APIs */ > > virMacMapPtr macmap; > > + > > + virHashTablePtr ports; /* uuid -> virNetworkPortDefPtr */ > > }; > > struct _virNetworkObjList { > > @@ -86,6 +88,17 @@ virNetworkObjOnceInit(void) > > VIR_ONCE_GLOBAL_INIT(virNetworkObj); > > +static int > > +virNetworkObjLoadAllPorts(virNetworkObjPtr net, > > + const char *stateDir); > > + > > + > > +static void > > +virNetworkObjPortFree(void *val, const void *key ATTRIBUTE_UNUSED) > > +{ > > + virNetworkPortDefFree(val); > > +} > > + > > virNetworkObjPtr > > virNetworkObjNew(void) > > { > > @@ -106,6 +119,10 @@ virNetworkObjNew(void) > > virBitmapSetBitExpand(obj->classIdMap, 2) < 0) > > goto error; > > + if (!(obj->ports = virHashCreate(10, > > + virNetworkObjPortFree))) > > + goto error; > > + > > virObjectLock(obj); > > return obj; > > @@ -458,6 +475,7 @@ virNetworkObjDispose(void *opaque) > > { > > virNetworkObjPtr obj = opaque; > > + virHashFree(obj->ports); > > virNetworkDefFree(obj->def); > > virNetworkDefFree(obj->newDef); > > virBitmapFree(obj->classIdMap); > > @@ -1072,9 +1090,16 @@ virNetworkObjLoadAllState(virNetworkObjListPtr nets, > > continue; > > obj = virNetworkLoadState(nets, stateDir, entry->d_name); > > + > > + if (obj && > > + virNetworkObjLoadAllPorts(obj, stateDir) < 0) { > > + virNetworkObjEndAPI(&obj); > > + goto cleanup; > > + } > > > Why do you do this here instead of adding it to virNetworkLoadState()? > > > ACK as-is if there's a reason for it. Otherwise, ACK with that chunk moved > into virNetworkLoadState() Just felt like it would be nice to have them separated, but not a strong reason.

> Reviewed-by: Laine Stump <laine(a)laine.org&gt;