[libvirt] [PATCH 03/20] virConnect(Un)RegisterCloseCallback: Throw an error in case the API is not supported

[Nikolay Shirokovskiy]

On 16.03.2018 12:39, Nikolay Shirokovskiy wrote:
> 
> 
> On 08.03.2018 15:20, Marc Hartmayer wrote:
>> Report an error in case the driver does not support
>> connect(Un)registerCloseCallback. The commit 'close callback: move it
>> to driver' (88f09b75eb99) moved the responsibility for the close
>> callback to the driver. But if the driver doesn't support the
>> connectRegisterCloseCallback API this function does nothing, even no
>> unsupported error report. The only case where an error is reported is
>> when the API is supported but the call fails. The same behavior
>> applies to virConnectUnregisterCloseCallback.
>>
>> This behavior is not intended as there are many use cases of this API
>> where the state of for example allocations depends on the result of
>> these functions.
>>
>> To keep the behavior of virsh as before it must silently ignore
>> unsupported error for virConnectRegisterCloseCallback. For the remote
>> driver this change wouldn't be needed, but for the byhve driver, for
>> example. Otherwise the user would see the error message that virsh was
>> unable to register a disconnect callback.
>>
>> Signed-off-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>> ---
>>  src/libvirt-host.c | 24 ++++++++++++++----------
>>  tools/virsh.c      | 11 +++++++++--
>>  2 files changed, 23 insertions(+), 12 deletions(-)
>>
> 
> We can't change public API. As to patch 18 I suggest either to:
> 
> - don't refcount client object, this works though it is dangerous. Or
> - check if driver supports VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK and throw
>   an error if it does not (looks better to me)
> 

Still other clients (other than daemon) can have opaque leaks as they
don't check for VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK before call to 
virConnectRegisterCloseCallback. This is introduced in [1]

[1] 88f09b75e
Author: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
Date:   Tue Mar 1 14:17:38 2016 +0000

    close callback: move it to driver
    
    Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>

To fix this we can fallback to previous scheme (using closeCallback in
virConnectPtr) if connectRegisterCloseCallback is not defined for
the driver. Then we can refcount client object in patch 18 without
any additional checks.

Nikolay