[PATCH] phyp: don't steal storage management from other drivers

[Eric Blake <eblake@xxxxxxxxxx>]

Fix regression introduced in commit a4a287242 - basically, the
phyp storage driver should only accept the same URIs that the
main phyp driver is willing to accept.  Blindly accepting all
URIs meant that the phyp storage driver was being consulted for
'virsh -c qemu:///session pool-list --all', rather than the
qemu storage driver, then since the URI was not for phyp, attempts
to then use the phyp driver crahsed because it was not initialized.

* src/phyp/phyp_driver.c (phypStorageOpen): Copy phypOpen on which
connections we are willing to accept.
---

> > Looks like the phyp storage driver is being used instead of the libvirtd
> > storage driver (and then segfaulting). Looked briefly for a fix, but it
> > wasn't obvious to me.
> The phypStorageOpen() method is totally bogus. It is ignoring the conn
> object and accepting all connections.

Sorry for not realizing that during my review; this is my first
time dealing with a storage driver patch.

> It must only return VIR_DRV_OPEN_SUCCESS if  conn->driver->name == VIR_DRV_PHYP

Agreed.  I think this patch does the right thing.

 src/phyp/phyp_driver.c |  127 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 126 insertions(+), 1 deletions(-)

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 3692f2c..62dfbcf 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -3875,13 +3875,138 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus)
 }

 static virDrvOpenStatus
-phypStorageOpen(virConnectPtr conn ATTRIBUTE_UNUSED,
+phypStorageOpen(virConnectPtr conn,
                 virConnectAuthPtr auth ATTRIBUTE_UNUSED,
                 int flags)
 {
+    LIBSSH2_SESSION *session = NULL;
+    ConnectionData *connection_data = NULL;
+    char *string = NULL;
+    size_t len = 0;
+    int internal_socket;
+    uuid_tablePtr uuid_table = NULL;
+    phyp_driverPtr phyp_driver = NULL;
+    char *char_ptr;
+    char *managed_system = NULL;
+
     virCheckFlags(0, VIR_DRV_OPEN_ERROR);

+    if (!conn || !conn->uri)
+        return VIR_DRV_OPEN_DECLINED;
+
+    if (conn->uri->scheme == NULL || STRNEQ(conn->uri->scheme, "phyp"))
+        return VIR_DRV_OPEN_DECLINED;
+
+    if (conn->uri->server == NULL) {
+        PHYP_ERROR(VIR_ERR_INTERNAL_ERROR,
+                   "%s", _("Missing server name in phyp:// URI"));
+        return VIR_DRV_OPEN_ERROR;
+    }
+
+    if (VIR_ALLOC(phyp_driver) < 0) {
+        virReportOOMError();
+        goto failure;
+    }
+
+    if (VIR_ALLOC(uuid_table) < 0) {
+        virReportOOMError();
+        goto failure;
+    }
+
+    if (VIR_ALLOC(connection_data) < 0) {
+        virReportOOMError();
+        goto failure;
+    }
+
+    if (conn->uri->path) {
+        len = strlen(conn->uri->path) + 1;
+
+        if (VIR_ALLOC_N(string, len) < 0) {
+            virReportOOMError();
+            goto failure;
+        }
+
+        /* need to shift one byte in order to remove the first "/" of URI component */
+        if (conn->uri->path[0] == '/')
+            managed_system = strdup(conn->uri->path + 1);
+        else
+            managed_system = strdup(conn->uri->path);
+
+        if (!managed_system) {
+            virReportOOMError();
+            goto failure;
+        }
+
+        /* here we are handling only the first component of the path,
+         * so skipping the second:
+         * */
+        char_ptr = strchr(managed_system, '/');
+
+        if (char_ptr)
+            *char_ptr = '\0';
+
+        if (escape_specialcharacters(conn->uri->path, string, len) == -1) {
+            PHYP_ERROR(VIR_ERR_INTERNAL_ERROR,
+                       "%s",
+                       _("Error parsing 'path'. Invalid characters."));
+            goto failure;
+        }
+    }
+
+    if ((session = openSSHSession(conn, auth, &internal_socket)) == NULL) {
+        PHYP_ERROR(VIR_ERR_INTERNAL_ERROR,
+                   "%s", _("Error while opening SSH session."));
+        goto failure;
+    }
+
+    connection_data->session = session;
+
+    uuid_table->nlpars = 0;
+    uuid_table->lpars = NULL;
+
+    if (conn->uri->path)
+        phyp_driver->managed_system = managed_system;
+
+    phyp_driver->uuid_table = uuid_table;
+    if ((phyp_driver->caps = phypCapsInit()) == NULL) {
+        virReportOOMError();
+        goto failure;
+    }
+
+    conn->privateData = phyp_driver;
+    conn->networkPrivateData = connection_data;
+
+    if ((phyp_driver->system_type = phypGetSystemType(conn)) == -1)
+        goto failure;
+
+    if (phypUUIDTable_Init(conn) == -1)
+        goto failure;
+
+    if (phyp_driver->system_type == HMC) {
+        if ((phyp_driver->vios_id = phypGetVIOSPartitionID(conn)) == -1)
+            goto failure;
+    }
+
     return VIR_DRV_OPEN_SUCCESS;
+
+  failure:
+    if (phyp_driver != NULL) {
+        virCapabilitiesFree(phyp_driver->caps);
+        VIR_FREE(phyp_driver->managed_system);
+        VIR_FREE(phyp_driver);
+    }
+
+    phypUUIDTable_Free(uuid_table);
+
+    if (session != NULL) {
+        libssh2_session_disconnect(session, "Disconnecting...");
+        libssh2_session_free(session);
+    }
+
+    VIR_FREE(connection_data);
+    VIR_FREE(string);
+
+    return VIR_DRV_OPEN_ERROR;
 }

 static int
-- 
1.7.0.1

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