[libvirt] [PATCH 3/7] remote: use a separate connection for network APIs

Daniel P. Berrangé posted 7 patches 7 years, 1 month ago
There is a newer version of this series
[libvirt] [PATCH 3/7] remote: use a separate connection for network APIs
Posted by Daniel P. Berrangé 7 years, 1 month ago
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/remote/remote_daemon.h          |  1 +
 src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
 src/rpc/gendispatch.pl              |  6 ++++++
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 31f433c15d..60be78fe0b 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -75,6 +75,7 @@ struct daemonClientPrivate {
      */
     virConnectPtr conn;
     virConnectPtr interfaceConn;
+    virConnectPtr networkConn;
 
     daemonClientStreamPtr streams;
 };
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 7971646c28..d0bc474850 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1699,7 +1699,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
     DEREG_CB(priv->conn, priv->domainEventCallbacks,
              priv->ndomainEventCallbacks,
              virConnectDomainEventDeregisterAny, "domain");
-    DEREG_CB(priv->conn, priv->networkEventCallbacks,
+    DEREG_CB(priv->networkConn, priv->networkEventCallbacks,
              priv->nnetworkEventCallbacks,
              virConnectNetworkEventDeregisterAny, "network");
     DEREG_CB(priv->conn, priv->storageEventCallbacks,
@@ -1742,6 +1742,8 @@ void remoteClientFree(void *data)
         virConnectClose(priv->conn);
     if (priv->interfaceConn)
         virConnectClose(priv->interfaceConn);
+    if (priv->networkConn)
+        virConnectClose(priv->networkConn);
 
     VIR_FREE(priv);
 }
@@ -1817,6 +1819,7 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
         goto cleanup;
 
     priv->interfaceConn = virObjectRef(priv->conn);
+    priv->networkConn = virObjectRef(priv->conn);
 
     /* force update the @readonly attribute which was inherited from the
      * virNetServerService object - this is important for sockets that are RW
@@ -5713,7 +5716,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN
         virNetServerClientGetPrivateData(client);
     virNetworkPtr net = NULL;
 
-    if (!priv->conn) {
+    if (!priv->networkConn) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
         goto cleanup;
     }
@@ -5721,7 +5724,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN
     virMutexLock(&priv->lock);
 
     if (args->net &&
-        !(net = get_nonnull_network(priv->conn, *args->net)))
+        !(net = get_nonnull_network(priv->networkConn, *args->net)))
         goto cleanup;
 
     if (args->eventID >= VIR_NETWORK_EVENT_ID_LAST || args->eventID < 0) {
@@ -5747,7 +5750,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN
                            callback) < 0)
         goto cleanup;
 
-    if ((callbackID = virConnectNetworkEventRegisterAny(priv->conn,
+    if ((callbackID = virConnectNetworkEventRegisterAny(priv->networkConn,
                                                         net,
                                                         args->eventID,
                                                         networkEventCallbacks[args->eventID],
@@ -5786,7 +5789,7 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_
     struct daemonClientPrivate *priv =
         virNetServerClientGetPrivateData(client);
 
-    if (!priv->conn) {
+    if (!priv->networkConn) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
         goto cleanup;
     }
@@ -5804,7 +5807,7 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_
         goto cleanup;
     }
 
-    if (virConnectNetworkEventDeregisterAny(priv->conn, args->callbackID) < 0)
+    if (virConnectNetworkEventDeregisterAny(priv->networkConn, args->callbackID) < 0)
         goto cleanup;
 
     VIR_DELETE_ELEMENT(priv->networkEventCallbacks, i,
@@ -6467,12 +6470,12 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
     virNetworkPtr net = NULL;
     int nleases = 0;
 
-    if (!priv->conn) {
+    if (!priv->networkConn) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
         goto cleanup;
     }
 
-    if (!(net = get_nonnull_network(priv->conn, args->net)))
+    if (!(net = get_nonnull_network(priv->networkConn, args->net)))
         goto cleanup;
 
     if ((nleases = virNetworkGetDHCPLeases(net,
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 23b17c0815..5bab13bb7b 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -128,6 +128,9 @@ sub get_conn_arg {
         if ($type =~ /remote_nonnull_interface/) {
             return "priv->interfaceConn";
         }
+        if ($type =~ /remote_nonnull_network/) {
+            return "priv->networkConn";
+        }
     }
 
     # This is for the few virConnect APIs that
@@ -136,6 +139,9 @@ sub get_conn_arg {
     if ($proc =~ /Connect.*Interface/ || $proc =~ /InterfaceChange/) {
         return "priv->interfaceConn";
     }
+    if ($proc =~ /Connect.*Network/) {
+        return "priv->networkConn";
+    }
 
     return "priv->conn";
 }
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/7] remote: use a separate connection for network APIs
Posted by John Ferlan 7 years, 1 month ago

On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/remote/remote_daemon.h          |  1 +
>  src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
>  src/rpc/gendispatch.pl              |  6 ++++++
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
> index 31f433c15d..60be78fe0b 100644
> --- a/src/remote/remote_daemon.h
> +++ b/src/remote/remote_daemon.h
> @@ -75,6 +75,7 @@ struct daemonClientPrivate {
>       */
>      virConnectPtr conn;
>      virConnectPtr interfaceConn;
> +    virConnectPtr networkConn;
>  
>      daemonClientStreamPtr streams;
>  };
> diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
> index 7971646c28..d0bc474850 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -1699,7 +1699,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)

Trying to forward think - will there ever come a day when priv->conn ==
NULL, but priv->*Conn != NULL?  The caller is gated on priv->conn...

IOW: Do we need to separate this one out a bit now

If not, then consider this

Reviewed-by: John Ferlan <jferlan@redhat.com>

If so, then probably need to see adjustment...

John

>      DEREG_CB(priv->conn, priv->domainEventCallbacks,
>               priv->ndomainEventCallbacks,
>               virConnectDomainEventDeregisterAny, "domain");
> -    DEREG_CB(priv->conn, priv->networkEventCallbacks,
> +    DEREG_CB(priv->networkConn, priv->networkEventCallbacks,
>               priv->nnetworkEventCallbacks,
>               virConnectNetworkEventDeregisterAny, "network");
>      DEREG_CB(priv->conn, priv->storageEventCallbacks,
[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/7] remote: use a separate connection for network APIs
Posted by Daniel P. Berrangé 7 years, 1 month ago
On Wed, Apr 04, 2018 at 01:45:45PM -0400, John Ferlan wrote:
> 
> 
> On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/remote/remote_daemon.h          |  1 +
> >  src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
> >  src/rpc/gendispatch.pl              |  6 ++++++
> >  3 files changed, 18 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
> > index 31f433c15d..60be78fe0b 100644
> > --- a/src/remote/remote_daemon.h
> > +++ b/src/remote/remote_daemon.h
> > @@ -75,6 +75,7 @@ struct daemonClientPrivate {
> >       */
> >      virConnectPtr conn;
> >      virConnectPtr interfaceConn;
> > +    virConnectPtr networkConn;
> >  
> >      daemonClientStreamPtr streams;
> >  };
> > diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
> > index 7971646c28..d0bc474850 100644
> > --- a/src/remote/remote_daemon_dispatch.c
> > +++ b/src/remote/remote_daemon_dispatch.c
> > @@ -1699,7 +1699,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
> 
> Trying to forward think - will there ever come a day when priv->conn ==
> NULL, but priv->*Conn != NULL?  The caller is gated on priv->conn...
> 
> IOW: Do we need to separate this one out a bit now

I'm not entirely sure at this time. It makes sense to push the if (conn)
check down into this method though, so I'll do that.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list