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

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()

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