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

On Wed, Jan 24, 2018 at 01:03 PM +0100, John Ferlan <jferlan(a)redhat.com&gt; wrote: > On 01/24/2018 05:12 AM, Marc Hartmayer wrote: >> 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 > > Oh right... There really was no reason to set them to NULL since the > only time they're used is in the context they are used. If the > AddProgram was successful, then there would be 2 refs anyway. They're > not used later in the code, so I'll remove the = NULL after the Unref. Is it possible to get rid of these global variables? At least for adminProgram and lxcProgram this should be easily to achieve as they aren’t used anywhere else than in this function context.

libvirtd.h: (Only these variables are declared as extern) extern virNetServerProgramPtr remoteProgram; extern virNetServerProgramPtr qemuProgram; > > John > >> […snip…] -- 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