Re: [libvirt] [PATCH 02/20] remote: Don't hard code the feature VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK as available
On 03/14/2018 05:30 PM, John Ferlan wrote:
On 03/08/2018 07:20 AM, Marc Hartmayer wrote:
> Don't assume that the feature VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK is
> available for every driver used for the connection.
>
> Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
> ---
> src/remote/remote_daemon_dispatch.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Something that is not clear about this one - since this was added for
'vz' driver by commit id 'f484310a', then shouldn't
vzConnectSupportsFeature be updated to indicate support?
If I'm right and you add the feature to the vz routine along with a
reference to the commit id that forgot to in your commit message, then
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
If I'm wrong - then help me understand!
John
Once I got to patch 5 I started questioning my (limited) understanding
of what's going on here.
Still if we move the REMOTE_CLOSE_CALLBACK into the "check with the
connection" to see if it's supported, then how does patch 3/virsh
actually utilize the virConnectRegisterCloseCallback unless the vz
driver is enabled?
Won't the check with the connection driver for everyone else return 0?
Maybe I just need to understand this code a bit more <sigh>
John
> diff --git a/src/remote/remote_daemon_dispatch.c
b/src/remote/remote_daemon_dispatch.c
> index 82f6400ca49d..bf6c00348a5e 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -4667,7 +4667,6 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr
server ATTRIBUTE
> switch ((virDrvFeature) args->feature) {
> case VIR_DRV_FEATURE_FD_PASSING:
> case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK:
> - case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK:
> supported = 1;
> break;
> case VIR_DRV_FEATURE_MIGRATION_V1:
> @@ -4681,6 +4680,7 @@ static int remoteDispatchConnectSupportsFeature(virNetServerPtr
server ATTRIBUTE
> case VIR_DRV_FEATURE_XML_MIGRATABLE:
> case VIR_DRV_FEATURE_MIGRATION_OFFLINE:
> case VIR_DRV_FEATURE_MIGRATION_PARAMS:
> + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK:
> default:
> if ((supported = virConnectSupportsFeature(priv->conn, args->feature))
< 0)
> goto cleanup;
>