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

Marc Hartmayer mhartmay at linux.vnet.ibm.com
Wed Jan 24 10:12:17 UTC 2018


On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan at redhat.com> 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 at redhat.com>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  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 at entry=0x0) at ../../src/rpc/virnetserverprogram.c:93
#1  0x000000011b726ac2 in remoteDispatchObjectEventSend (client=<optimized out>, program=0x0, procnr=procnr at entry=319, proc=0x11b767120 <xdr_remote_domain_event_callback_reboot_msg>, data=data at 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 at entry=0x3ff2c0facf0, event=0x1436a37b0, callbacks=callbacks at entry=0x3ff2c106370) at ../../src/conf/object_event.c:715
#5  0x000003ff90414eb4 in virObjectEventStateQueueDispatch (state=state at entry=0x3ff2c0facf0, queue=queue at 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 at linux.vnet.ibm.com>.


> +    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 at 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





More information about the libvir-list mailing list