On 03/08/2018 07:20 AM, 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 ^^^^^^^ I think you forgot to end your sentence for example what about bhyve?
unable to register a disconnect callback.
So the reason to keep the virsh behavior is because failure to do so could be problematic if connecting to an older server that doesn't support virConnectRegisterCloseCallback? I almost wonder if we should blat a vshDebug type message to "inform" the consumer that it's not possible to catch disconnect... Since the code path doesn't return -1 anyway (even if "Unable to register..." is printed), it may not hurt. Your call though - it's not that important other than the oh, OK so there's something newer in the newer server that might be useful. Reviewed-by: John Ferlan <jferlan@redhat.com> John