On 25.10.2017 12:06, John Ferlan wrote: > > > On 10/24/2017 06:47 AM, Nikolay Shirokovskiy wrote: >> After virNetDaemonAddServerPostExec call in virtlogd we should have >> netserver refcount set to 2. One goes to netdaemon servers hashtable >> and one goes to virtlogd own reference to netserver. Let's add >> missing increment in virNetDaemonAddServerPostExec itself while holding >> daemon lock. >> >> We also have to unref new extra ref after virtlockd call to virNetDaemonAddServerPostExec. >> --- >> src/locking/lock_daemon.c | 1 + >> src/rpc/virnetdaemon.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c >> index fe3eaf9..41a06b2 100644 >> --- a/src/locking/lock_daemon.c >> +++ b/src/locking/lock_daemon.c >> @@ -275,6 +275,7 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) >> virLockDaemonClientFree, >> (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) >> goto error; >> + virObjectUnref(srv); > > I need to think a bit more about this one... different model in lockd > vs. logd over @srv usage especially w/r/t this particular path. > > I see that in this path eventually something calls virNetDaemonGetServer > instead of storing the @srv in order to add lockProgram... In any case,> I guess I'd be worried/concerned that something could remove @srv > causing the Hash table code to unref/delete the srv... Also, in the > cleanup: path of main() wouldn't the Unref(srv) cause problems? But this unref only balance newly added ref. If there are other problems with reference conting in virtlockd - its a different concern. > > Again- need to think a bit longer. > > John > >> >> return lockd; >> >> diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c >> index 495bc4b..f3e3ffe 100644 >> --- a/src/rpc/virnetdaemon.c >> +++ b/src/rpc/virnetdaemon.c >> @@ -312,6 +312,7 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, >> >> if (virHashAddEntry(dmn->servers, serverName, srv) < 0) >> goto error; >> + virObjectRef(srv); >> >> virJSONValueFree(object); >> virObjectUnlock(dmn); >> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
