On 06/10/2013 08:44 AM, Peter Krempa wrote:
Can you include in the commit log a link to the BZ describing this
problem? It helps *immensely* when trying to trace things months/years
later.
> Without the socket path explicitly specified, the remote driver tried to
> connect to the "/system" instance socket even if "/session" was
> specified in the uri. With this patch this configuration now produces an
> error.
>
> It is still possible to initiate a session connection with specifying
> the path to the socket manually and also manually starting the session
> daemon. This was also possible prior to this patch,
>
> This is a minimal fix. We may decide to support remote session
> connections using ssh but this will require changes to the remote driver
> code so this fix shouldn't cause regressions in the case we decide to do
> that.
> ---
> src/remote/remote_driver.c | 40 ++++++++++++++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index fcf45d3..bd5646a 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -631,11 +631,21 @@ doRemoteOpen(virConnectPtr conn,
> break;
>
> case trans_libssh2:
> - if (!sockname &&
> - VIR_STRDUP(sockname,
> - flags & VIR_DRV_OPEN_REMOTE_RO ?
> - LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0)
> - goto failed;
> + if (!sockname) {
> + /* Right now we don't support default session connections */
> + if (STREQ(conn->uri->path, "/session")) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("Connecting to session instance without "
> + "socket path is not supported by the libssh2 "
> + "connection driver"));
> + goto failed;
> + }
A totally naive question: do we want to only allow "/system"? or
'anything except "/session"'?
> +
> + if (VIR_STRDUP(sockname,
> + flags & VIR_DRV_OPEN_REMOTE_RO ?
> + LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0)
> + goto failed;
> + }
>
> VIR_DEBUG("Starting LibSSH2 session");
>
> @@ -698,11 +708,21 @@ doRemoteOpen(virConnectPtr conn,
> if (!command && VIR_STRDUP(command, "ssh") < 0)
> goto failed;
>
> - if (!sockname &&
> - VIR_STRDUP(sockname,
> - flags & VIR_DRV_OPEN_REMOTE_RO ?
> - LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0)
> - goto failed;
> + if (!sockname) {
> + /* Right now we don't support default session connections */
> + if (STREQ(conn->uri->path, "/session")) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("Connecting to session instance without "
> + "socket path is not supported by the ssh "
> + "connection driver"));
> + goto failed;
> + }
> +
> + if (VIR_STRDUP(sockname,
> + flags & VIR_DRV_OPEN_REMOTE_RO ?
> + LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0)
> + goto failed;
> + }
>
> if (!(priv->client = virNetClientNewSSH(priv->hostname,
> port,
ACK once the BZ link is included in the commit log (and pending my
question about whether we want to check for == "/session" or != "/system").
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list