Re: [libvirt] [PATCH 3/5] libvirtd: Alter refcnt processing for domain server objects

[...] > Now the actual review. > virNetDaemonAddServer is only used when spawning a new daemon or setting up LXC > controller. The function essentially does: > > lock(@dmn) > hash_table_add(@srv) > ref(@srv) > unlock(@dmn) > > and then you unref @dmn right upon the completion of adding the server to the > hash table, what's the purpose of the extra ref when you discard it > immediately? Unless I'm failing to see something, I'd prefer to just drop the > extra ref in virNetDaemonAddServer and leave the proposed hunks unchanged - you > have 1 ref which you got by creating the object, now it should be just simply > inserted into the hash table, the additional ref after insertion doesn't make > sense, if someone managed to unref it before inserting it into the hash table, > hash insert would most likely fail, if not, hashFree surely would, not > mentioning that at that point there's no entity that is touching servers. > Since I was "digging" on a different issue - check out how this is done elsewhere... Start with virNetServerDispatchNewClient. It'll call the ClientNew function which generates a REF. Then it will call AddClient which generates a REF. Then because it's on that list and this code is theoretically done with it - it will UNREF the client before returning.