On Tue, Nov 07, 2017 at 09:39:55PM -0500, John Ferlan wrote:
Instead of holding onto the reference for each program added to a NetServer, let's Unref the program reference as soon as the attempt is made to add to the @srv->programs list. This then allows the virNetServerDispose to handle the final Unref of the object for actual deletion and cleans up the cleanup: path.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/libvirtd.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 5c47e49d48..87c5b22710 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1352,7 +1352,10 @@ int main(int argc, char **argv) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } - if (virNetServerAddProgram(srv, remoteProgram) < 0) { + + rc = virNetServerAddProgram(srv, remoteProgram); + virObjectUnref(remoteProgram);
Again, why can't we drop the ref from the AddProgram, if we as the @srv mutex owners got rescheduled and someone unreff'd @program before we inserted it to the array (while holding mutex), we'd fail to ref a dangling pointer consequently, so I don't see any added protection here and I think ^these hunks are unnecessary, once we agree to drop virObjectRef from virNet<Foo>Add<Bar> during the init phase.
+ if (rc < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } @@ -1364,7 +1367,10 @@ int main(int argc, char **argv) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } - if (virNetServerAddProgram(srv, lxcProgram) < 0) { + + rc = virNetServerAddProgram(srv, lxcProgram); + virObjectUnref(lxcProgram); + if (rc < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } @@ -1376,7 +1382,10 @@ int main(int argc, char **argv) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } - if (virNetServerAddProgram(srv, qemuProgram) < 0) { + + rc = virNetServerAddProgram(srv, qemuProgram); + virObjectUnref(qemuProgram); + if (rc < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } @@ -1414,7 +1423,10 @@ int main(int argc, char **argv) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } - if (virNetServerAddProgram(srvAdm, adminProgram) < 0) { + + rc = virNetServerAddProgram(srvAdm, adminProgram); + virObjectUnref(adminProgram); + if (rc < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } @@ -1513,10 +1525,6 @@ int main(int argc, char **argv) {
cleanup: virNetlinkEventServiceStopAll(); - virObjectUnref(remoteProgram); - virObjectUnref(lxcProgram); - virObjectUnref(qemuProgram); - virObjectUnref(adminProgram);
This hunk would stay either way. Erik