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

[Marc Hartmayer]

On Wed, Jan 24, 2018 at 01:03 PM +0100, John Ferlan <jferlan at redhat.com> wrote:
> On 01/24/2018 05:12 AM, Marc Hartmayer wrote:
>> 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
>
> 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