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

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(a)linux.vnet.ibm.com&gt; > Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; > --- > 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)