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

[John Ferlan]

On 01/24/2018 07:48 AM, Marc Hartmayer wrote:
> 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.

I suppose so, not sure if that was within the context of what I was
doing. It'd have to be a separate "pre-patch" - in fact doable without
this series.  I also know Dan's been working through adding admin
protocol support for logd/lockd in another series...

John

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