Re: [libvirt] [PATCH 2/9] libvirtd: Alter refcnt processing for server program objects

On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com&gt; wrote: > Once virNetServerProgramNew returns, the program objects have > either been added to the @srv or @srvAdm list increasing the > refcnt or there was an error. In either case, we can lower the > refcnt from virNetServerProgramNew and set the object to NULL > since it won't be used any more. > > The @srv or @srvAdm dispose function (virNetServerDispose) will > handle the Unref of each object during the virHashFree when the > object is removed from the hash table. > > Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > daemon/libvirtd.c | 28 ++++++++++++++++++++-------- > 1 file changed, 20 insertions(+), 8 deletions(-) > > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c > index 0afd1dd82..b47f875d9 100644 > --- a/daemon/libvirtd.c > +++ b/daemon/libvirtd.c > @@ -1352,7 +1352,11 @@ int main(int argc, char **argv) { > ret = VIR_DAEMON_ERR_INIT; > goto cleanup; > } > - if (virNetServerAddProgram(srv, remoteProgram) < 0) { > + > + rc = virNetServerAddProgram(srv, remoteProgram); > + virObjectUnref(remoteProgram); > + remoteProgram = NULL; I’ve just looked at the code and all those variables are a global variables… libvirtd.c: virNetServerProgramPtr remoteProgram = NULL; virNetServerProgramPtr adminProgram = NULL; virNetServerProgramPtr qemuProgram = NULL; virNetServerProgramPtr lxcProgram = NULL; So this is not a good idea to set them to NULL… As this results in a segmentation fault

[Current thread is 1 (Thread 0x3ff908d7910 (LWP 195236))] (gdb) bt #0 virNetServerProgramGetID (prog=prog@entry=0x0) at ../../src/rpc/virnetserverprogram.c:93 #1 0x000000011b726ac2 in remoteDispatchObjectEventSend (client=<optimized out>, program=0x0, procnr=procnr@entry=319, proc=0x11b767120 <xdr_remote_domain_event_callback_reboot_msg>, data=data@entry=0x3fffee7d948) at ../../daemon/remote.c:3997 #2 0x000000011b7577d2 in remoteRelayDomainEventReboot (conn=<optimized out>, dom=0x143663320, opaque=0x3ff700330c0) at ../../daemon/remote.c:365 #3 0x000003ff904173a8 in virDomainEventDispatchDefaultFunc (conn=0x3fef42ca860, event=0x1436a37b0, cb=0x11b7576d0 <remoteRelayDomainEventReboot>, cbopaque=0x3ff700330c0) at ../../src/conf/domain_event.c:1787 #4 0x000003ff90414e4e in virObjectEventStateDispatchCallbacks (state=state@entry=0x3ff2c0facf0, event=0x1436a37b0, callbacks=callbacks@entry=0x3ff2c106370) at ../../src/conf/object_event.c:715 #5 0x000003ff90414eb4 in virObjectEventStateQueueDispatch (state=state@entry=0x3ff2c0facf0, queue=queue@entry=0x3fffee7dc10, callbacks=0x3ff2c106370) at ../../src/conf/object_event.c:729 #6 0x000003ff904156d4 in virObjectEventStateFlush (state=0x3ff2c0facf0) at ../../src/conf/object_event.c:830 #7 0x000003ff9041574e in virObjectEventTimer (timer=<optimized out>, opaque=<optimized out>) at ../../src/conf/object_event.c:560 #8 0x000003ff90343cc4 in virEventPollDispatchTimeouts () at ../../src/util/vireventpoll.c:457 #9 0x000003ff9034577c in virEventPollRunOnce () at ../../src/util/vireventpoll.c:653 #10 0x000003ff90343852 in virEventRunDefaultImpl () at ../../src/util/virevent.c:327 #11 0x000003ff904e5e78 in virNetDaemonRun (dmn=0x1436303b0) at ../../src/rpc/virnetdaemon.c:880 #12 0x000000011b7256ca in main (argc=<optimized out>, argv=<optimized out>) at ../../daemon/libvirtd.c:1523 Therefore, I’ll withdraw my Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt;. > + if (rc < 0) { > ret = VIR_DAEMON_ERR_INIT; > goto cleanup; > } > @@ -1364,7 +1368,11 @@ int main(int argc, char **argv) { > ret = VIR_DAEMON_ERR_INIT; > goto cleanup; > } > - if (virNetServerAddProgram(srv, lxcProgram) < 0) { > + > + rc = virNetServerAddProgram(srv, lxcProgram); > + virObjectUnref(lxcProgram); > + lxcProgram = NULL; > + if (rc < 0) { > ret = VIR_DAEMON_ERR_INIT; > goto cleanup; > } > @@ -1376,7 +1384,11 @@ int main(int argc, char **argv) { > ret = VIR_DAEMON_ERR_INIT; > goto cleanup; > } > - if (virNetServerAddProgram(srv, qemuProgram) < 0) { > + > + rc = virNetServerAddProgram(srv, qemuProgram); > + virObjectUnref(qemuProgram); > + qemuProgram = NULL; > + if (rc < 0) { > ret = VIR_DAEMON_ERR_INIT; > goto cleanup; > } > @@ -1414,7 +1426,11 @@ int main(int argc, char **argv) { > ret = VIR_DAEMON_ERR_INIT; > goto cleanup; > } > - if (virNetServerAddProgram(srvAdm, adminProgram) < 0) { > + > + rc = virNetServerAddProgram(srvAdm, adminProgram); > + virObjectUnref(adminProgram); > + adminProgram = NULL; > + if (rc < 0) { > ret = VIR_DAEMON_ERR_INIT; > goto cleanup; > } > @@ -1524,10 +1540,6 @@ int main(int argc, char **argv) { > virStateCleanup(); > } > > - virObjectUnref(adminProgram); > - virObjectUnref(qemuProgram); > - virObjectUnref(lxcProgram); > - virObjectUnref(remoteProgram); > virObjectUnref(dmn); > > virNetlinkShutdown(); > -- > 2.13.6 > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294