9:19 a.m.
v2 of:
https://listman.redhat.com/archives/libvir-list/2023-February/237540.html
diff to v1:
- Dropped "remote_driver: Expose EXTRACT_URI_ARG_* macros" patch
- Dropped "src: Unify URI params parsing" patch
- Reworked couple of patches as a result of that
- Reworded couple of commit messages
Michal PrÃvoznÃk (8):
viruri: Search params case insensitively
Drop checks for virURIFormat() retval
doRemoteOpen(): Rename 'failed' label to 'error'
virt-ssh-helper: Accept ?mode= in connection URI
virt-ssh-helper: Accept ?socket= in connection URI
remote_driver: Move URI re-generation into a function
viruri: Introduce virURIParamsSetIgnore()
remote: Pass 'mode' and 'socket' URI parameters to virt-ssh-helper
src/admin/libvirt-admin.c | 6 +-
src/libvirt-host.c | 10 +--
src/libvirt_private.syms | 1 +
src/qemu/qemu_block.c | 3 +-
src/qemu/qemu_migration.c | 3 +-
src/remote/remote_driver.c | 114 ++++++++++++++++----------
src/remote/remote_ssh_helper.c | 32 ++++++--
src/storage/storage_backend_gluster.c | 6 +-
src/util/viruri.c | 37 ++++++++-
src/util/viruri.h | 2 +
tests/viruritest.c | 3 +-
11 files changed, 139 insertions(+), 78 deletions(-)
--
2.39.1
9:19 a.m.
New subject: [PATCH v2 1/8] viruri: Search params case insensitively
Our URI handling code (doRemoteOpen() specifically), uses case
insensitive parsing of query part of URI. For instance:
qemu:///system?socket=/some/path
qemu:///system?SoCkEt=/some/path
are the same URI. Even though the latter is probably not used
anywhere, let's switch to STRCASEEQ() instead of STREQ() at two
places: virURIGetParam() and virURICheckUnixSocket().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/viruri.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/util/viruri.c b/src/util/viruri.c
index 88bb0cc1f8..4afdd30c59 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -365,13 +365,24 @@ virURIResolveAlias(virConf *conf, const char *alias, char **uri)
}
+/**
+ * virURIGetParam:
+ * @uri: URI to get parameter from
+ * @name: name of the parameter
+ *
+ * For parsed @uri, find parameter with name @name and return its value. The
+ * string comparison is case insensitive, by design.
+ *
+ * Returns: a value on success, or
+ * NULL on error (with error reported)
+ */
const char *
virURIGetParam(virURI *uri, const char *name)
{
size_t i;
for (i = 0; i < uri->paramsCount; i++) {
- if (STREQ(uri->params[i].name, name))
+ if (STRCASEEQ(uri->params[i].name, name))
return uri->params[i].value;
}
@@ -389,6 +400,8 @@ virURIGetParam(virURI *uri, const char *name)
* scenario the socket might be proxied to a remote server even though the URI
* looks like it is only local.
*
+ * The "socket" parameter is looked for in case insensitive manner, by design.
+ *
* Returns: true if the URI might be proxied to a remote server
*/
bool
@@ -403,7 +416,7 @@ virURICheckUnixSocket(virURI *uri)
return false;
for (i = 0; i < uri->paramsCount; i++) {
- if (STREQ(uri->params[i].name, "socket"))
+ if (STRCASEEQ(uri->params[i].name, "socket"))
return true;
}
--
2.39.1
9:19 a.m.
New subject: [PATCH v2 2/8] Drop checks for virURIFormat() retval
The virURIFormat() function either returns a string, or aborts
(on OOM). There's no way this function can return NULL (as of
v7.2.0-rc1~277). Therefore, it doesn't make sense to check its
retval against NULL.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/admin/libvirt-admin.c | 6 +-----
src/libvirt-host.c | 10 +---------
src/qemu/qemu_block.c | 3 +--
src/qemu/qemu_migration.c | 3 +--
src/remote/remote_driver.c | 3 ---
src/storage/storage_backend_gluster.c | 6 +-----
src/util/viruri.c | 2 +-
tests/viruritest.c | 3 +--
8 files changed, 7 insertions(+), 29 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 6baaea4ff2..1786a283e5 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -427,17 +427,13 @@ virAdmConnectIsAlive(virAdmConnectPtr conn)
char *
virAdmConnectGetURI(virAdmConnectPtr conn)
{
- char *uri = NULL;
VIR_DEBUG("conn=%p", conn);
virResetLastError();
virCheckAdmConnectReturn(conn, NULL);
- if (!(uri = virURIFormat(conn->uri)))
- virDispatchError(NULL);
-
- return uri;
+ return virURIFormat(conn->uri);
}
/**
diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index c02222346c..a2ba347d54 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -313,21 +313,13 @@ virConnectGetHostname(virConnectPtr conn)
char *
virConnectGetURI(virConnectPtr conn)
{
- char *name;
VIR_DEBUG("conn=%p", conn);
virResetLastError();
virCheckConnectReturn(conn, NULL);
- if (!(name = virURIFormat(conn->uri)))
- goto error;
-
- return name;
-
- error:
- virDispatchError(conn);
- return NULL;
+ return virURIFormat(conn->uri);
}
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index c218262691..4c06565e0f 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -407,8 +407,7 @@ qemuBlockStorageSourceGetCURLProps(virStorageSource *src,
if (!(uri = qemuBlockStorageSourceGetURI(src)))
return NULL;
- if (!(uristr = virURIFormat(uri)))
- return NULL;
+ uristr = virURIFormat(uri);
if (!onlytarget) {
if (src->auth) {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index f36bb49be5..2720f0b083 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3777,8 +3777,7 @@ qemuMigrationDstPrepareDirect(virQEMUDriver *driver,
/* Send well-formed URI only if uri_in was well-formed */
if (well_formed_uri) {
uri->port = port;
- if (!(*uri_out = virURIFormat(uri)))
- goto cleanup;
+ *uri_out = virURIFormat(uri);
} else {
*uri_out = g_strdup_printf("%s:%d", uri_in, port);
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9fc73f6d88..2e08ff246f 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -843,9 +843,6 @@ doRemoteOpen(virConnectPtr conn,
name = virURIFormat(&tmpuri);
VIR_FREE(tmpuri.query);
-
- if (!name)
- goto failed;
}
}
} else {
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 1fe21b4a2b..bdf91e8bd5 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -195,11 +195,7 @@ virStorageBackendGlusterSetMetadata(virStorageBackendGlusterState
*state,
tmp = state->uri->path;
state->uri->path = g_strdup_printf("/%s", path);
- if (!(vol->target.path = virURIFormat(state->uri))) {
- VIR_FREE(state->uri->path);
- state->uri->path = tmp;
- return -1;
- }
+ vol->target.path = virURIFormat(state->uri);
VIR_FREE(state->uri->path);
state->uri->path = tmp;
diff --git a/src/util/viruri.c b/src/util/viruri.c
index 4afdd30c59..54db0bda06 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -194,7 +194,7 @@ virURIParse(const char *uri)
* Wrapper for xmlSaveUri
*
* This function constructs back everything that @ref virURIParse
- * changes after parsing
+ * changes after parsing. It aborts on error.
*
* @returns the constructed uri as a string
*/
diff --git a/tests/viruritest.c b/tests/viruritest.c
index cd6ce57371..705e0d5c6e 100644
--- a/tests/viruritest.c
+++ b/tests/viruritest.c
@@ -118,8 +118,7 @@ static int testURIParse(const void *args)
VIR_FREE(uri->query);
uri->query = virURIFormatParams(uri);
- if (!(uristr = virURIFormat(uri)))
- return -1;
+ uristr = virURIFormat(uri);
if (STRNEQ(uristr, data->uri_out)) {
VIR_TEST_DEBUG("URI did not roundtrip, expect '%s', actual
'%s'",
--
2.39.1
9:19 a.m.
New subject: [PATCH v2 3/8] doRemoteOpen(): Rename 'failed' label to 'error'
Our own coding style suggest not inventing new names for labels
and stick with 'cleanup' (when the path is used in both,
successful and unsuccessful returns), or 'error' (when the code
below the label is used only upon error). Well, 'failed' label
falls into the latter category. Rename it then.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/remote/remote_driver.c | 62 +++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 2e08ff246f..6a226999df 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -709,7 +709,7 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
virReportError(VIR_ERR_INVALID_ARG, \
_("Failed to parse value of URI component %s"), \
var->name); \
- goto failed; \
+ goto error; \
} \
ARG_VAR = tmp == 0; \
var->ignore = 1; \
@@ -852,13 +852,13 @@ doRemoteOpen(virConnectPtr conn,
if (conf && !mode_str &&
virConfGetValueString(conf, "remote_mode", &mode_str) < 0)
- goto failed;
+ goto error;
if (mode_str) {
if ((mode = remoteDriverModeTypeFromString(mode_str)) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
_("Unknown remote mode '%s'"), mode_str);
- goto failed;
+ goto error;
}
} else {
if (inside_daemon && !conn->uri->server) {
@@ -870,13 +870,13 @@ doRemoteOpen(virConnectPtr conn,
if (conf && !proxy_str &&
virConfGetValueString(conf, "remote_proxy", &proxy_str) < 0)
- goto failed;
+ goto error;
if (proxy_str) {
if ((proxy = virNetClientProxyTypeFromString(proxy_str)) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
_("Unnkown proxy type '%s'"), proxy_str);
- goto failed;
+ goto error;
}
} else {
/*
@@ -924,7 +924,7 @@ doRemoteOpen(virConnectPtr conn,
if (transport == REMOTE_DRIVER_TRANSPORT_EXT && !command) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("remote_open: for 'ext' transport, command is
required"));
- goto failed;
+ goto error;
}
VIR_DEBUG("Connecting with transport %d", transport);
@@ -937,7 +937,7 @@ doRemoteOpen(virConnectPtr conn,
if (!sockname &&
!(sockname = remoteGetUNIXSocket(transport, mode, driver_str,
flags, &daemon_path)))
- goto failed;
+ goto error;
break;
case REMOTE_DRIVER_TRANSPORT_TCP:
@@ -948,7 +948,7 @@ doRemoteOpen(virConnectPtr conn,
case REMOTE_DRIVER_TRANSPORT_LAST:
default:
virReportEnumRangeError(remoteDriverTransport, transport);
- goto failed;
+ goto error;
}
VIR_DEBUG("Chosen UNIX socket %s", NULLSTR(sockname));
@@ -958,26 +958,26 @@ doRemoteOpen(virConnectPtr conn,
case REMOTE_DRIVER_TRANSPORT_TLS:
if (conf && !tls_priority &&
virConfGetValueString(conf, "tls_priority", &tls_priority) <
0)
- goto failed;
+ goto error;
priv->tls = virNetTLSContextNewClientPath(pkipath,
geteuid() != 0,
tls_priority,
sanity, verify);
if (!priv->tls)
- goto failed;
+ goto error;
priv->is_secure = 1;
G_GNUC_FALLTHROUGH;
case REMOTE_DRIVER_TRANSPORT_TCP:
priv->client = virNetClientNewTCP(priv->hostname, port, AF_UNSPEC);
if (!priv->client)
- goto failed;
+ goto error;
if (priv->tls) {
VIR_DEBUG("Starting TLS session");
if (virNetClientSetTLSSession(priv->client, priv->tls) < 0)
- goto failed;
+ goto error;
}
break;
@@ -1001,7 +1001,7 @@ doRemoteOpen(virConnectPtr conn,
auth,
conn->uri);
if (!priv->client)
- goto failed;
+ goto error;
priv->is_secure = 1;
break;
@@ -1025,7 +1025,7 @@ doRemoteOpen(virConnectPtr conn,
auth,
conn->uri);
if (!priv->client)
- goto failed;
+ goto error;
priv->is_secure = 1;
break;
@@ -1034,7 +1034,7 @@ doRemoteOpen(virConnectPtr conn,
case REMOTE_DRIVER_TRANSPORT_UNIX:
if (!(priv->client = virNetClientNewUNIX(sockname,
daemon_path)))
- goto failed;
+ goto error;
priv->is_secure = 1;
break;
@@ -1055,7 +1055,7 @@ doRemoteOpen(virConnectPtr conn,
sockname,
name,
flags & REMOTE_DRIVER_OPEN_RO)))
- goto failed;
+ goto error;
priv->is_secure = 1;
break;
@@ -1063,7 +1063,7 @@ doRemoteOpen(virConnectPtr conn,
case REMOTE_DRIVER_TRANSPORT_EXT: {
char const *cmd_argv[] = { command, NULL };
if (!(priv->client = virNetClientNewExternal(cmd_argv)))
- goto failed;
+ goto error;
/* Do not set 'is_secure' flag since we can't guarantee
* an external program is secure, and this flag must be
@@ -1078,14 +1078,14 @@ doRemoteOpen(virConnectPtr conn,
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("transport methods unix, ssh and ext are not supported
"
"under Windows"));
- goto failed;
+ goto error;
#endif /* WIN32 */
case REMOTE_DRIVER_TRANSPORT_LAST:
default:
virReportEnumRangeError(remoteDriverTransport, transport);
- goto failed;
+ goto error;
} /* switch (transport) */
@@ -1095,11 +1095,11 @@ doRemoteOpen(virConnectPtr conn,
virResetLastError();
} else {
if (virNetClientRegisterKeepAlive(priv->client) < 0)
- goto failed;
+ goto error;
}
if (!(priv->closeCallback = virNewConnectCloseCallbackData()))
- goto failed;
+ goto error;
/* ref on behalf of netclient */
virObjectRef(priv->closeCallback);
virNetClientSetCloseCallback(priv->client,
@@ -1111,29 +1111,29 @@ doRemoteOpen(virConnectPtr conn,
remoteEvents,
G_N_ELEMENTS(remoteEvents),
conn)))
- goto failed;
+ goto error;
if (!(priv->lxcProgram = virNetClientProgramNew(LXC_PROGRAM,
LXC_PROTOCOL_VERSION,
NULL,
0,
NULL)))
- goto failed;
+ goto error;
if (!(priv->qemuProgram = virNetClientProgramNew(QEMU_PROGRAM,
QEMU_PROTOCOL_VERSION,
qemuEvents,
G_N_ELEMENTS(qemuEvents),
conn)))
- goto failed;
+ goto error;
if (virNetClientAddProgram(priv->client, priv->remoteProgram) < 0 ||
virNetClientAddProgram(priv->client, priv->lxcProgram) < 0 ||
virNetClientAddProgram(priv->client, priv->qemuProgram) < 0)
- goto failed;
+ goto error;
/* Try and authenticate with server */
VIR_DEBUG("Trying authentication");
if (remoteAuthenticate(conn, priv, auth, authtype) == -1)
- goto failed;
+ goto error;
if (virNetClientKeepAliveIsSupported(priv->client)) {
priv->serverKeepAlive = remoteConnectSupportsFeatureUnlocked(conn,
@@ -1152,7 +1152,7 @@ doRemoteOpen(virConnectPtr conn,
if (call(conn, priv, 0, REMOTE_PROC_CONNECT_OPEN,
(xdrproc_t) xdr_remote_connect_open_args, (char *) &args,
(xdrproc_t) xdr_void, (char *) NULL) == -1)
- goto failed;
+ goto error;
}
/* Now try and find out what URI the daemon used */
@@ -1165,18 +1165,18 @@ doRemoteOpen(virConnectPtr conn,
REMOTE_PROC_CONNECT_GET_URI,
(xdrproc_t) xdr_void, (char *) NULL,
(xdrproc_t) xdr_remote_connect_get_uri_ret, (char *) &uriret) <
0)
- goto failed;
+ goto error;
VIR_DEBUG("Auto-probed URI is %s", uriret.uri);
conn->uri = virURIParse(uriret.uri);
VIR_FREE(uriret.uri);
if (!conn->uri)
- goto failed;
+ goto error;
}
/* Set up events */
if (!(priv->eventState = virObjectEventStateNew()))
- goto failed;
+ goto error;
priv->serverEventFilter = remoteConnectSupportsFeatureUnlocked(conn,
priv, VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK);
@@ -1194,7 +1194,7 @@ doRemoteOpen(virConnectPtr conn,
return VIR_DRV_OPEN_SUCCESS;
- failed:
+ error:
virObjectUnref(priv->remoteProgram);
virObjectUnref(priv->lxcProgram);
virObjectUnref(priv->qemuProgram);
--
2.39.1
9:19 a.m.
New subject: [PATCH v2 4/8] virt-ssh-helper: Accept ?mode= in connection URI
When split daemons were introduced, we also made connection URI
accept new parameter: mode={auto,legacy,direct} so that a client
can force connecting to either old, monolithic daemon, or to
split daemon (see v5.7.0-rc1~257 for more info).
Now, the change was done to the remote driver, but not to
virt-ssh-helper. True, our remote driver code still does not pass
the 'mode' parameter, but that will be addressed in next commits.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_ssh_helper.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_ssh_helper.c b/src/remote/remote_ssh_helper.c
index b4735027be..8adc62acf2 100644
--- a/src/remote/remote_ssh_helper.c
+++ b/src/remote/remote_ssh_helper.c
@@ -354,6 +354,8 @@ int main(int argc, char **argv)
g_autoptr(virURI) uri = NULL;
g_autofree char *driver = NULL;
remoteDriverTransport transport;
+ int mode = REMOTE_DRIVER_MODE_AUTO;
+ const char *mode_str = NULL;
gboolean version = false;
gboolean readonly = false;
g_autofree char *sock_path = NULL;
@@ -367,6 +369,7 @@ int main(int argc, char **argv)
{ NULL, '\0', 0, 0, NULL, NULL, NULL }
};
unsigned int flags;
+ size_t i;
context = g_option_context_new("URI - libvirt socket proxy");
g_option_context_set_summary(context,
@@ -429,8 +432,23 @@ int main(int argc, char **argv)
if (readonly)
flags |= REMOTE_DRIVER_OPEN_RO;
+ for (i = 0; i < uri->paramsCount; i++) {
+ virURIParam *var = &uri->params[i];
+
+ if (STRCASEEQ(var->name, "mode")) {
+ mode_str = var->value;
+ continue;
+ }
+ }
+
+ if (mode_str &&
+ (mode = remoteDriverModeTypeFromString(mode_str)) < 0) {
+ g_printerr(_("%s: unknown remote mode '%s'"), argv[0],
mode_str);
+ exit(EXIT_FAILURE);
+ }
+
sock_path = remoteGetUNIXSocket(transport,
- REMOTE_DRIVER_MODE_AUTO,
+ mode,
driver,
flags,
&daemon_path);
--
2.39.1
9:35 a.m.
New subject: [PATCH v2 4/8] virt-ssh-helper: Accept ?mode= in connection URI
On Wed, Feb 08, 2023 at 16:19:10 +0100, Michal Privoznik wrote:
When split daemons were introduced, we also made connection URI
accept new parameter: mode={auto,legacy,direct} so that a client
can force connecting to either old, monolithic daemon, or to
split daemon (see v5.7.0-rc1~257 for more info).
Now, the change was done to the remote driver, but not to
virt-ssh-helper. True, our remote driver code still does not pass
the 'mode' parameter, but that will be addressed in next commits.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_ssh_helper.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
9:19 a.m.
New subject: [PATCH v2 5/8] virt-ssh-helper: Accept ?socket= in connection URI
Similarly to the previous commit, let's accept "socket" parameter
in the connection URI. This change will allow us to use
virt-ssh-helper instead of 'nc' in all cases (done in one of
future commits).
Please note, when the parameter is used it effectively disables
automatic daemon spawning and an error is reported. But this is
intentional - so that the helper behaves just like regular
virConnectOpen() with different transport than ssh, e.g. unix.
But this 'change' is acceptable - there's no way for users to
make our remote code pass the argument to virt-ssh-helper, yet.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_ssh_helper.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/remote/remote_ssh_helper.c b/src/remote/remote_ssh_helper.c
index 8adc62acf2..29f5889533 100644
--- a/src/remote/remote_ssh_helper.c
+++ b/src/remote/remote_ssh_helper.c
@@ -438,6 +438,9 @@ int main(int argc, char **argv)
if (STRCASEEQ(var->name, "mode")) {
mode_str = var->value;
continue;
+ } else if (STRCASEEQ(var->name, "socket")) {
+ sock_path = g_strdup(var->value);
+ continue;
}
}
@@ -447,11 +450,12 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
- sock_path = remoteGetUNIXSocket(transport,
- mode,
- driver,
- flags,
- &daemon_path);
+ if (!sock_path &&
+ !(sock_path = remoteGetUNIXSocket(transport, mode,
+ driver, flags, &daemon_path))) {
+ g_printerr(_("%s: failed to generate UNIX socket path"), argv[0]);
+ exit(EXIT_FAILURE);
+ }
if (virNetSocketNewConnectUNIX(sock_path, daemon_path, &sock) < 0) {
g_printerr(_("%s: cannot connect to '%s': %s\n"),
--
2.39.1
9:46 a.m.
New subject: [PATCH v2 5/8] virt-ssh-helper: Accept ?socket= in connection URI
On Wed, Feb 08, 2023 at 16:19:11 +0100, Michal Privoznik wrote:
Similarly to the previous commit, let's accept "socket"
parameter
in the connection URI. This change will allow us to use
virt-ssh-helper instead of 'nc' in all cases (done in one of
future commits).
Please note, when the parameter is used it effectively disables
automatic daemon spawning and an error is reported. But this is
intentional - so that the helper behaves just like regular
virConnectOpen() with different transport than ssh, e.g. unix.
But this 'change' is acceptable - there's no way for users to
make our remote code pass the argument to virt-ssh-helper, yet.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_ssh_helper.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
[....]
@@ -447,11 +450,12 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
- sock_path = remoteGetUNIXSocket(transport,
- mode,
- driver,
- flags,
- &daemon_path);
+ if (!sock_path &&
+ !(sock_path = remoteGetUNIXSocket(transport, mode,
+ driver, flags, &daemon_path))) {
+ g_printerr(_("%s: failed to generate UNIX socket path"), argv[0]);
This made me wonder how errors were reported until now, but I didn't
really manage to make it report an error.
Anyways:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
9:19 a.m.
New subject: [PATCH v2 6/8] remote_driver: Move URI re-generation into a function
There's a piece of code in doRemoteOpen() that is going to be
called twice. Instead of duplicating the code, move it into a
function that will be called twice, later on.
Signee-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_driver.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 6a226999df..cce9e7ddaf 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -693,6 +693,24 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
return rc != -1 && ret.supported;
}
+
+static char *
+remoteConnectFormatURI(virURI *uri,
+ const char *driver_str)
+{
+ g_autofree char *query = NULL;
+ virURI tmpuri = {
+ .scheme = (char *)driver_str,
+ .path = uri->path,
+ .fragment = uri->fragment,
+ };
+
+ query = tmpuri.query = virURIFormatParams(uri);
+
+ return virURIFormat(&tmpuri);
+}
+
+
/* helper macro to ease extraction of arguments from the URI */
#define EXTRACT_URI_ARG_STR(ARG_NAME, ARG_VAR) \
if (STRCASEEQ(var->name, ARG_NAME)) { \
@@ -833,16 +851,8 @@ doRemoteOpen(virConnectPtr conn,
/* Allow remote serve to probe */
name = g_strdup("");
} else {
- virURI tmpuri = {
- .scheme = (char *)driver_str,
- .query = virURIFormatParams(conn->uri),
- .path = conn->uri->path,
- .fragment = conn->uri->fragment,
- };
+ name = remoteConnectFormatURI(conn->uri, driver_str);
- name = virURIFormat(&tmpuri);
-
- VIR_FREE(tmpuri.query);
}
}
} else {
--
2.39.1
9:47 a.m.
New subject: [PATCH v2 6/8] remote_driver: Move URI re-generation into a function
On Wed, Feb 08, 2023 at 16:19:12 +0100, Michal Privoznik wrote:
There's a piece of code in doRemoteOpen() that is going to be
called twice. Instead of duplicating the code, move it into a
function that will be called twice, later on.
Signee-off-by: Michal Privoznik <mprivozn(a)redhat.com>
This is misspelled
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
A bit too much signoffage, most likely caused by the misspelled one
above.
9:19 a.m.
New subject: [PATCH v2 7/8] viruri: Introduce virURIParamsSetIgnore()
The aim of this helper is to manipulate the .ignore value for
given list of parameters. For instance:
virURIParamsSetIgnore(uri, false, {"mode", "socket", NULL});
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/viruri.c | 18 ++++++++++++++++++
src/util/viruri.h | 2 ++
3 files changed, 21 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7ca8b472be..97c3d86217 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3560,6 +3560,7 @@ virURIFormat;
virURIFormatParams;
virURIFree;
virURIGetParam;
+virURIParamsSetIgnore;
virURIParse;
virURIResolveAlias;
diff --git a/src/util/viruri.c b/src/util/viruri.c
index 54db0bda06..3af2e54565 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -422,3 +422,21 @@ virURICheckUnixSocket(virURI *uri)
return false;
}
+
+
+void
+virURIParamsSetIgnore(virURI *uri,
+ bool ignore,
+ const char *names[])
+{
+ size_t i;
+
+ for (i = 0; i < uri->paramsCount; i++) {
+ size_t j;
+
+ for (j = 0; names[j]; j++) {
+ if (STRCASEEQ(uri->params[i].name, names[j]))
+ uri->params[i].ignore = ignore;
+ }
+ }
+}
diff --git a/src/util/viruri.h b/src/util/viruri.h
index 4f27fa26d2..ad00570b7f 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -61,4 +61,6 @@ const char *virURIGetParam(virURI *uri, const char *name);
bool virURICheckUnixSocket(virURI *uri);
+void virURIParamsSetIgnore(virURI *uri, bool ignore, const char *names[]);
+
#define VIR_URI_SERVER(uri) ((uri) && (uri)->server ? (uri)->server :
"localhost")
--
2.39.1
9:19 a.m.
New subject: [PATCH v2 8/8] remote: Pass 'mode' and 'socket' URI parameters to virt-ssh-helper
When handling virConnectOpen(), we parse given URI, specifically
all those parameters we know, like ?mode, ?socket, ?name, etc.
ignoring those we don't recognize yet. Then, we reconstruct the
URI back, but ignoring all parameters we've parsed. In other
words:
qemu:///system?mode=legacy&foo=bar
becomes:
qemu:///system?foo=bar
The reconstructed URI is then passed to the corresponding driver
(QEMU in our example) with intent of it parsing parameters
further (or just ignoring them). But for some transport modes,
where virt-ssh-helper is ran on the remote host (libssh, libssh2,
ssh) we need to pass ?mode and ?socket parameters, so that it can
do the right thing, e.g. for 'mode=legacy' start the monolithic
daemon, or for 'socket=' connect to the given socket.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/433
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/remote/remote_driver.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index cce9e7ddaf..58cd0abe8c 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -696,18 +696,31 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
static char *
remoteConnectFormatURI(virURI *uri,
- const char *driver_str)
+ const char *driver_str,
+ bool unmask)
{
+ const char *names[] = {"mode", "socket", NULL};
g_autofree char *query = NULL;
+ char *ret = NULL;
virURI tmpuri = {
.scheme = (char *)driver_str,
.path = uri->path,
.fragment = uri->fragment,
};
+ if (unmask) {
+ virURIParamsSetIgnore(uri, false, names);
+ }
+
query = tmpuri.query = virURIFormatParams(uri);
- return virURIFormat(&tmpuri);
+ ret = virURIFormat(&tmpuri);
+
+ if (unmask) {
+ virURIParamsSetIgnore(uri, true, names);
+ }
+
+ return ret;
}
@@ -780,6 +793,7 @@ doRemoteOpen(virConnectPtr conn,
g_autofree char *mode_str = NULL;
g_autofree char *daemon_path = NULL;
g_autofree char *proxy_str = NULL;
+ g_autofree char *virtSshURI = NULL;
bool sanity = true;
bool verify = true;
#ifndef WIN32
@@ -851,7 +865,10 @@ doRemoteOpen(virConnectPtr conn,
/* Allow remote serve to probe */
name = g_strdup("");
} else {
- name = remoteConnectFormatURI(conn->uri, driver_str);
+ name = remoteConnectFormatURI(conn->uri, driver_str, false);
+
+ /* Preserve mode and socket parameters. */
+ virtSshURI = remoteConnectFormatURI(conn->uri, driver_str, true);
}
}
@@ -1006,7 +1023,7 @@ doRemoteOpen(virConnectPtr conn,
proxy,
netcat,
sockname,
- name,
+ virtSshURI ? virtSshURI : name,
flags & REMOTE_DRIVER_OPEN_RO,
auth,
conn->uri);
@@ -1030,7 +1047,7 @@ doRemoteOpen(virConnectPtr conn,
proxy,
netcat,
sockname,
- name,
+ virtSshURI ? virtSshURI : name,
flags & REMOTE_DRIVER_OPEN_RO,
auth,
conn->uri);
@@ -1063,7 +1080,7 @@ doRemoteOpen(virConnectPtr conn,
proxy,
netcat,
sockname,
- name,
+ virtSshURI ? virtSshURI : name,
flags & REMOTE_DRIVER_OPEN_RO)))
goto error;
--
2.39.1
653
days inactive
653
days old
11 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa