[Libvir] PATCH: Fix remote driver to handle network API for local non-QEMU

Daniel P. Berrange berrange at redhat.com
Wed Jun 27 18:04:00 UTC 2007


The remote_internal.c code is basically a no-op in its remoteNetworkOpen
method. This means the remote driver will only handle the networking
APIs, if it is also handling the main domain APIs. This works fine if
connecting to a remote URI, or if using QEMU URIs, but it does not work
if using the test or Xen drivers.

To make this work the remoteNetworkOpen method needs to open a connection
to the remote daemon IIF the remoteOpen method did not already open one.

So the attached patch adds the neccessary logic in remoteNetworkOpen. It
also adds code to remoteNetworkClose to make it shutdown& free the connection
IIF it was opened by the remoteNetworkOpen method. This is what the extra
'networkOnly' field in the 'struct private_data' is used to check.

Second, all of the implementations of virNetwork* APIs in the remote_internal.c
driver must use the  'struct private_data *' from networkPrivateData field
in virConnectPtr, not the 'privateData' field. This is because the 'privateData'
field is probably storing data related to the Xen or Test drivers.

Third, this also fixes the qemu_driver.c which incorrectly used the 
privateData field insteadof the networkPrivateData field for 2 of the
networking APIs, and finally makes sure the qemu_driver.c also acccepts
the use of qemu:///session APIs for root user.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
-------------- next part --------------
Index: src/qemu_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.1
diff -u -p -r1.1 qemu_driver.c
--- src/qemu_driver.c	27 Jun 2007 00:12:29 -0000	1.1
+++ src/qemu_driver.c	27 Jun 2007 17:46:38 -0000
@@ -1464,7 +1464,8 @@ virDrvOpenStatus qemudOpen(virConnectPtr
         if (strcmp(name, "qemu:///session"))
             return VIR_DRV_OPEN_DECLINED;
     } else {
-        if (strcmp(name, "qemu:///system"))
+        if (strcmp(name, "qemu:///system") &&
+            strcmp(name, "qemu:///session"))
             return VIR_DRV_OPEN_DECLINED;
     }
 
@@ -2143,7 +2144,7 @@ int qemudDomainSetAutostart(virDomainPtr
 
 virNetworkPtr qemudNetworkLookupByUUID(virConnectPtr conn ATTRIBUTE_UNUSED,
                                      const unsigned char *uuid) {
-    struct qemud_driver *driver = (struct qemud_driver *)conn->privateData;
+    struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData;
     struct qemud_network *network = qemudFindNetworkByUUID(driver, uuid);
     virNetworkPtr net;
 
@@ -2161,7 +2162,7 @@ virNetworkPtr qemudNetworkLookupByUUID(v
 }
 virNetworkPtr qemudNetworkLookupByName(virConnectPtr conn ATTRIBUTE_UNUSED,
                                      const char *name) {
-    struct qemud_driver *driver = (struct qemud_driver *)conn->privateData;
+    struct qemud_driver *driver = (struct qemud_driver *)conn->networkPrivateData;
     struct qemud_network *network = qemudFindNetworkByName(driver, name);
     virNetworkPtr net;
 
Index: src/remote_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/remote_internal.c,v
retrieving revision 1.11
diff -u -p -r1.11 remote_internal.c
--- src/remote_internal.c	26 Jun 2007 23:48:47 -0000	1.11
+++ src/remote_internal.c	27 Jun 2007 17:46:40 -0000
@@ -60,6 +60,7 @@ struct private_data {
     char *type;                 /* Cached return from remoteType. */
     int counter;                /* Generates serial numbers for RPC. */
     char *uri;                  /* Original (remote) URI. */
+    int networkOnly;            /* Only used for network API */
 };
 
 #define GET_PRIVATE(conn,retcode)                                       \
@@ -72,6 +73,16 @@ struct private_data {
     }                                                                   \
     assert (priv->magic == MAGIC)
 
+#define GET_NETWORK_PRIVATE(conn,retcode)                                       \
+    struct private_data *priv = (struct private_data *) (conn)->networkPrivateData; \
+    assert (priv);                                                      \
+    if (priv->magic == DEAD) {                                          \
+        error (conn, VIR_ERR_INVALID_ARG,                               \
+               "tried to use a closed or uninitialised handle");        \
+        return (retcode);                                               \
+    }                                                                   \
+    assert (priv->magic == MAGIC)
+
 static int call (virConnectPtr conn, struct private_data *priv, int in_open, int proc_nr, xdrproc_t args_filter, char *args, xdrproc_t ret_filter, char *ret);
 static void error (virConnectPtr conn, virErrorNumber code, const char *info);
 static void server_error (virConnectPtr conn, remote_error *err);
@@ -105,8 +116,15 @@ static void query_free (struct query_fie
 static int initialise_gnutls (virConnectPtr conn);
 static gnutls_session_t negotiate_gnutls_on_connection (virConnectPtr conn, int sock, int no_verify, const char *hostname);
 
+
+/* Must not overlap with virDrvOpenFlags */
+enum {
+    VIR_DRV_OPEN_REMOTE_UNIX = (1 << 8),
+    VIR_DRV_OPEN_REMOTE_USER = (1 << 9),
+} virDrvOpenRemoteFlags;
+
 static int
-remoteOpen (virConnectPtr conn, const char *uri_str, int flags)
+doRemoteOpen (virConnectPtr conn, struct private_data *priv, const char *uri_str, int flags)
 {
     if (!uri_str) return VIR_DRV_OPEN_DECLINED;
 
@@ -146,11 +164,12 @@ remoteOpen (virConnectPtr conn, const ch
         return VIR_DRV_OPEN_ERROR;
     }
 
-    if (!strcmp(uri_str, "qemu:///system") ||
-        !strcmp(uri_str, "qemu:///session"))
-        transport = trans_unix;
-    else if (!uri->server && !transport_str)
-        return VIR_DRV_OPEN_DECLINED; /* Decline - not a remote URL. */
+    if (!uri->server && !transport_str) {
+        if (flags & VIR_DRV_OPEN_REMOTE_UNIX)
+            transport = trans_unix;
+        else
+            return VIR_DRV_OPEN_DECLINED; /* Decline - not a remote URL. */
+    }
 
     /* Local variables which we will initialise. These can
      * get freed in the failed: path.
@@ -162,7 +181,6 @@ remoteOpen (virConnectPtr conn, const ch
 
     /* Return code from this function, and the private data. */
     int retcode = VIR_DRV_OPEN_ERROR;
-    struct private_data priv = { .magic = DEAD, .sock = -1 };
 
     /* Remote server defaults to "localhost" if not specified. */
     server = strdup (uri->server ? uri->server : "localhost");
@@ -282,7 +300,7 @@ remoteOpen (virConnectPtr conn, const ch
     switch (transport) {
     case trans_tls:
         if (initialise_gnutls (conn) == -1) goto failed;
-        priv.uses_tls = 1;
+        priv->uses_tls = 1;
 
         /*FALLTHROUGH*/
     case trans_tcp: {
@@ -312,30 +330,30 @@ remoteOpen (virConnectPtr conn, const ch
         for (r = res; r; r = r->ai_next) {
             int no_slow_start = 1;
 
-            priv.sock = socket (r->ai_family, SOCK_STREAM, 0);
-            if (priv.sock == -1) {
+            priv->sock = socket (r->ai_family, SOCK_STREAM, 0);
+            if (priv->sock == -1) {
                 error (NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno));
                 continue;
             }
 
             /* Disable Nagle - Dan Berrange. */
-            setsockopt (priv.sock,
+            setsockopt (priv->sock,
                         IPPROTO_TCP, TCP_NODELAY, (void *)&no_slow_start,
                         sizeof no_slow_start);
 
-            if (connect (priv.sock, r->ai_addr, r->ai_addrlen) == -1) {
+            if (connect (priv->sock, r->ai_addr, r->ai_addrlen) == -1) {
                 error (NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno));
-                close (priv.sock);
+                close (priv->sock);
                 continue;
             }
 
-            if (priv.uses_tls) {
-                priv.session =
+            if (priv->uses_tls) {
+                priv->session =
                     negotiate_gnutls_on_connection
-                      (conn, priv.sock, no_verify, server);
-                if (!priv.session) {
-                    close (priv.sock);
-                    priv.sock = -1;
+                      (conn, priv->sock, no_verify, server);
+                if (!priv->session) {
+                    close (priv->sock);
+                    priv->sock = -1;
                     continue;
                 }
             }
@@ -355,9 +373,7 @@ remoteOpen (virConnectPtr conn, const ch
 
     case trans_unix: {
         if (!sockname) {
-            if (!strcmp(uri->scheme, "qemu") &&
-                uri->path &&
-                !strcmp(uri->path, "/session")) {
+            if (flags & VIR_DRV_OPEN_REMOTE_USER) {
                 struct passwd *pw;
                 uid_t uid = getuid();
  
@@ -388,12 +404,12 @@ remoteOpen (virConnectPtr conn, const ch
         if (addr.sun_path[0] == '@')
             addr.sun_path[0] = '\0';
 
-        priv.sock = socket (AF_UNIX, SOCK_STREAM, 0);
-        if (priv.sock == -1) {
+        priv->sock = socket (AF_UNIX, SOCK_STREAM, 0);
+        if (priv->sock == -1) {
             error (NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno));
             goto failed;
         }
-        if (connect (priv.sock, (struct sockaddr *) &addr, sizeof addr) == -1){
+        if (connect (priv->sock, (struct sockaddr *) &addr, sizeof addr) == -1){
             error (NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno));
             goto failed;
         }
@@ -465,45 +481,35 @@ remoteOpen (virConnectPtr conn, const ch
 
         /* Parent continues here. */
         close (sv[1]);
-        priv.sock = sv[0];
+        priv->sock = sv[0];
     }
     } /* switch (transport) */
 
     /* Finally we can call the remote side's open function. */
     remote_open_args args = { &name, flags };
 
-    if (call (conn, &priv, 1, REMOTE_PROC_OPEN,
+    if (call (conn, priv, 1, REMOTE_PROC_OPEN,
               (xdrproc_t) xdr_remote_open_args, (char *) &args,
               (xdrproc_t) xdr_void, (char *) NULL) == -1)
         goto failed;
 
-    /* Finally allocate private data. */
-    conn->privateData = malloc (sizeof priv);
-    if (!conn->privateData) {
-        error (NULL, VIR_ERR_NO_MEMORY, "malloc");
-        goto failed;
-    }
     /* Duplicate and save the uri_str. */
-    priv.uri = strdup (uri_str);
-    if (!priv.uri) {
+    priv->uri = strdup (uri_str);
+    if (!priv->uri) {
         error (NULL, VIR_ERR_NO_MEMORY, "allocating priv->uri");
-        free (conn->privateData);
         goto failed;
     }
 
-    priv.magic = MAGIC;
-    memcpy (conn->privateData, &priv, sizeof priv);
-
     /* Successful. */
     retcode = VIR_DRV_OPEN_SUCCESS;
 
     /*FALLTHROUGH*/
  failed:
     /* Close the socket if we failed. */
-    if (retcode != VIR_DRV_OPEN_SUCCESS && priv.sock >= 0) {
-        if (priv.uses_tls && priv.session)
-            gnutls_bye (priv.session, GNUTLS_SHUT_RDWR);
-        close (priv.sock);
+    if (retcode != VIR_DRV_OPEN_SUCCESS && priv->sock >= 0) {
+        if (priv->uses_tls && priv->session)
+            gnutls_bye (priv->session, GNUTLS_SHUT_RDWR);
+        close (priv->sock);
     }
 
     /* Free up the URL and strings. */
@@ -527,6 +533,40 @@ remoteOpen (virConnectPtr conn, const ch
     return retcode;
 }
 
+static int
+remoteOpen (virConnectPtr conn, const char *uri_str, int flags)
+{
+    struct private_data *priv = malloc (sizeof(struct private_data));
+    int ret;
+
+    if (!priv) {
+        error (NULL, VIR_ERR_NO_MEMORY, "struct private_data");
+        return VIR_DRV_OPEN_ERROR;
+    }
+
+    if (!strcmp(uri_str, "qemu:///system")) {
+        flags |= VIR_DRV_OPEN_REMOTE_UNIX;
+    } else if (!strcmp(uri_str, "qemu:///session")) {
+        flags |= VIR_DRV_OPEN_REMOTE_UNIX;
+        if (getuid() > 0)
+            flags |= VIR_DRV_OPEN_REMOTE_USER;
+    }
+
+    memset(priv, 0, sizeof(struct private_data));
+    priv->magic = DEAD;
+    priv->sock = -1;
+    ret = doRemoteOpen(conn, priv, uri_str, flags);
+    if (ret != VIR_DRV_OPEN_SUCCESS) {
+        conn->privateData = NULL;
+        free(priv);
+    } else {
+        priv->magic = MAGIC;
+        conn->privateData = priv;
+    }
+    return ret;
+}
+
+
 /* In a string "driver+transport" return a pointer to "transport". */
 static char *
 get_transport_from_scheme (char *scheme)
@@ -948,11 +988,10 @@ verify_certificate (virConnectPtr conn A
 
 /*----------------------------------------------------------------------*/
 
+
 static int
-remoteClose (virConnectPtr conn)
+doRemoteClose (virConnectPtr conn, struct private_data *priv)
 {
-    GET_PRIVATE (conn, -1);
-
     if (call (conn, priv, 0, REMOTE_PROC_CLOSE,
               (xdrproc_t) xdr_void, (char *) NULL,
               (xdrproc_t) xdr_void, (char *) NULL) == -1)
@@ -971,11 +1010,23 @@ remoteClose (virConnectPtr conn)
 
     /* Free private data. */
     priv->magic = DEAD;
-    free (conn->privateData);
 
     return 0;
 }
 
+static int
+remoteClose (virConnectPtr conn)
+{
+    int ret;
+    GET_PRIVATE (conn, -1);
+
+    ret = doRemoteClose(conn, priv);
+    free (priv);
+    conn->privateData = NULL;
+
+    return ret;
+}
+
 /* Unfortunately this function is defined to return a static string.
  * Since the remote end always answers with the same type (for a
  * single connection anyway) we cache the type in the connection's
@@ -1954,33 +2005,65 @@ remoteNetworkOpen (virConnectPtr conn,
                    const char *uri_str ATTRIBUTE_UNUSED,
                    int flags ATTRIBUTE_UNUSED)
 {
-    /* If the main connection is a remote, then just catch the
-     * network open too.  Nothing is forwarded because the
-     * main remoteOpen call above will have already opened
-     * network on the remote side.
-     */
     if (conn &&
         conn->driver &&
-        strcmp (conn->driver->name, "remote") == 0)
+        strcmp (conn->driver->name, "remote") == 0) {
+        /* Main connection is already using remote driver,
+           so just hook up network APIs to use same driver */
+        conn->networkPrivateData = conn->privateData;
         return VIR_DRV_OPEN_SUCCESS;
-    else
-        return VIR_DRV_OPEN_DECLINED;
+    } else {
+        struct private_data *priv = malloc (sizeof(struct private_data));
+        int ret;
+        /* Using a non-remote driver, so we need to open a
+         * new connection for network APIs, forcing it to
+         * use the UNIX transport. This handles Xen / Test
+         * drivers which don't have their own impl of the
+         * network APIs.
+         */
+        if (!priv) {
+            error (NULL, VIR_ERR_NO_MEMORY, "struct private_data");
+            return VIR_DRV_OPEN_ERROR;
+        }
+
+        flags |= VIR_DRV_OPEN_REMOTE_UNIX;
+        if (getuid() > 0)
+            flags |= VIR_DRV_OPEN_REMOTE_USER;
+
+        memset(priv, 0, sizeof(struct private_data));
+        priv->magic = DEAD;
+        priv->sock = -1;
+        ret = doRemoteOpen(conn, priv, uri_str, flags);
+        if (ret != VIR_DRV_OPEN_SUCCESS) {
+            conn->networkPrivateData = NULL;
+            free(priv);
+        } else {
+            priv->magic = MAGIC;
+            priv->networkOnly = 1;
+            conn->networkPrivateData = priv;
+        }
+        return ret;
+    }
 }
 
 static int
-remoteNetworkClose (virConnectPtr conn ATTRIBUTE_UNUSED)
+remoteNetworkClose (virConnectPtr conn)
 {
-    /* No need to pass this to the remote side, because
-     * libvirt.c will soon call remoteClose.
-     */
-    return 0;
+    int ret = 0;
+    GET_NETWORK_PRIVATE (conn, -1);
+    if (priv->networkOnly) {
+        ret = doRemoteClose(conn, priv);
+        free(priv);
+        conn->networkPrivateData = NULL;
+    }
+    return ret;
 }
 
 static int
 remoteNumOfNetworks (virConnectPtr conn)
 {
     remote_num_of_networks_ret ret;
-    GET_PRIVATE (conn, -1);
+    GET_NETWORK_PRIVATE (conn, -1);
 
     memset (&ret, 0, sizeof ret);
     if (call (conn, priv, 0, REMOTE_PROC_NUM_OF_NETWORKS,
@@ -1997,7 +2080,7 @@ remoteListNetworks (virConnectPtr conn, 
     int i;
     remote_list_networks_args args;
     remote_list_networks_ret ret;
-    GET_PRIVATE (conn, -1);
+    GET_NETWORK_PRIVATE (conn, -1);
 
     if (maxnames > REMOTE_NETWORK_NAME_LIST_MAX) {
         error (conn, VIR_ERR_RPC, "maxnames > REMOTE_NETWORK_NAME_LIST_MAX");
@@ -2034,7 +2117,7 @@ static int
 remoteNumOfDefinedNetworks (virConnectPtr conn)
 {
     remote_num_of_defined_networks_ret ret;
-    GET_PRIVATE (conn, -1);
+    GET_NETWORK_PRIVATE (conn, -1);
 
     memset (&ret, 0, sizeof ret);
     if (call (conn, priv, 0, REMOTE_PROC_NUM_OF_DEFINED_NETWORKS,
@@ -2052,7 +2135,7 @@ remoteListDefinedNetworks (virConnectPtr
     int i;
     remote_list_defined_networks_args args;
     remote_list_defined_networks_ret ret;
-    GET_PRIVATE (conn, -1);
+    GET_NETWORK_PRIVATE (conn, -1);
 
     if (maxnames > REMOTE_NETWORK_NAME_LIST_MAX) {
         error (conn, VIR_ERR_RPC, "maxnames > REMOTE_NETWORK_NAME_LIST_MAX");
@@ -2092,7 +2175,7 @@ remoteNetworkLookupByUUID (virConnectPtr
     virNetworkPtr net;
     remote_network_lookup_by_uuid_args args;
     remote_network_lookup_by_uuid_ret ret;
-    GET_PRIVATE (conn, NULL);
+    GET_NETWORK_PRIVATE (conn, NULL);
 
     memcpy (args.uuid, uuid, VIR_UUID_BUFLEN);
 
@@ -2118,7 +2201,7 @@ remoteNetworkLookupByName (virConnectPtr
     virNetworkPtr net;
     remote_network_lookup_by_name_args args;
     remote_network_lookup_by_name_ret ret;
-    GET_PRIVATE (conn, NULL);
+    GET_NETWORK_PRIVATE (conn, NULL);
 
     args.name = (char *) name;
 
@@ -2143,7 +2226,7 @@ remoteNetworkCreateXML (virConnectPtr co
     virNetworkPtr net;
     remote_network_create_xml_args args;
     remote_network_create_xml_ret ret;
-    GET_PRIVATE (conn, NULL);
+    GET_NETWORK_PRIVATE (conn, NULL);
 
     args.xml = (char *) xmlDesc;
 
@@ -2168,7 +2251,7 @@ remoteNetworkDefineXML (virConnectPtr co
     virNetworkPtr net;
     remote_network_define_xml_args args;
     remote_network_define_xml_ret ret;
-    GET_PRIVATE (conn, NULL);
+    GET_NETWORK_PRIVATE (conn, NULL);
 
     args.xml = (char *) xml;
 
@@ -2191,7 +2274,7 @@ static int
 remoteNetworkUndefine (virNetworkPtr network)
 {
     remote_network_undefine_args args;
-    GET_PRIVATE (network->conn, -1);
+    GET_NETWORK_PRIVATE (network->conn, -1);
 
     make_nonnull_network (&args.net, network);
 
@@ -2207,7 +2290,7 @@ static int
 remoteNetworkCreate (virNetworkPtr network)
 {
     remote_network_create_args args;
-    GET_PRIVATE (network->conn, -1);
+    GET_NETWORK_PRIVATE (network->conn, -1);
 
     make_nonnull_network (&args.net, network);
 
@@ -2223,7 +2306,7 @@ static int
 remoteNetworkDestroy (virNetworkPtr network)
 {
     remote_network_destroy_args args;
-    GET_PRIVATE (network->conn, -1);
+    GET_NETWORK_PRIVATE (network->conn, -1);
 
     make_nonnull_network (&args.net, network);
 
@@ -2240,7 +2323,7 @@ remoteNetworkDumpXML (virNetworkPtr netw
 {
     remote_network_dump_xml_args args;
     remote_network_dump_xml_ret ret;
-    GET_PRIVATE (network->conn, NULL);
+    GET_NETWORK_PRIVATE (network->conn, NULL);
 
     make_nonnull_network (&args.net, network);
     args.flags = flags;
@@ -2260,7 +2343,7 @@ remoteNetworkGetBridgeName (virNetworkPt
 {
     remote_network_get_bridge_name_args args;
     remote_network_get_bridge_name_ret ret;
-    GET_PRIVATE (network->conn, NULL);
+    GET_NETWORK_PRIVATE (network->conn, NULL);
 
     make_nonnull_network (&args.net, network);
 
@@ -2279,7 +2362,7 @@ remoteNetworkGetAutostart (virNetworkPtr
 {
     remote_network_get_autostart_args args;
     remote_network_get_autostart_ret ret;
-    GET_PRIVATE (network->conn, -1);
+    GET_NETWORK_PRIVATE (network->conn, -1);
 
     make_nonnull_network (&args.net, network);
 
@@ -2298,7 +2381,7 @@ static int
 remoteNetworkSetAutostart (virNetworkPtr network, int autostart)
 {
     remote_network_set_autostart_args args;
-    GET_PRIVATE (network->conn, -1);
+    GET_NETWORK_PRIVATE (network->conn, -1);
 
     make_nonnull_network (&args.net, network);
     args.autostart = autostart;


More information about the libvir-list mailing list