3:16 a.m.
The first couple of patches are cleanups, mostly. The last 5 patches are
the important ones. Now, the fix I went with in the 10/10 is to format
URI anew, just for the virt-ssh-helper's sake. I did not want to touch
@name as it's passed to sub-daemon's .open() method. If desired, I can
change the @name variable instead as it seems that no driver relies on
?mode or ?socket (they couldn't anyway). Thoughts?
Michal Prívozník (10):
viruri: Search params case insensitively
Drop checks for virURIFormat() retval
doRemoteOpen(): Rename 'failed' label to 'error'
remote_driver: Expose EXTRACT_URI_ARG_* macros
src: Unify URI params parsing
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
po/POTFILES | 1 +
src/admin/libvirt-admin.c | 21 ++--
src/esx/esx_util.c | 96 ++++++++--------
src/hyperv/hyperv_util.c | 30 +++--
src/libvirt-host.c | 10 +-
src/libvirt_private.syms | 1 +
src/qemu/qemu_block.c | 3 +-
src/qemu/qemu_migration.c | 24 ++--
src/remote/remote_driver.c | 158 +++++++++++++-------------
src/remote/remote_ssh_helper.c | 27 ++++-
src/storage/storage_backend_gluster.c | 6 +-
src/util/virauth.c | 12 +-
src/util/viruri.c | 24 +++-
src/util/viruri.h | 37 ++++++
tests/viruritest.c | 3 +-
15 files changed, 245 insertions(+), 208 deletions(-)
--
2.39.1
3:16 a.m.
New subject: [PATCH 01/10] 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>
---
src/util/viruri.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/viruri.c b/src/util/viruri.c
index 88bb0cc1f8..53f85ed705 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -371,7 +371,7 @@ 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;
}
@@ -403,7 +403,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:02 a.m.
New subject: [PATCH 01/10] viruri: Search params case insensitively
On Mon, Feb 06, 2023 at 10:16:49 +0100, Michal Privoznik wrote:
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().
rfc3986 (Uniform Resource Identifier (URI): Generic Syntax) states:
6.2.2.1. Case Normalization
For all URIs, the hexadecimal digits within a percent-encoding
triplet (e.g., "%3a" versus "%3A") are case-insensitive and
therefore
should be normalized to use uppercase letters for the digits A-F.
When a URI uses components of the generic syntax, the component
syntax equivalence rules always apply; namely, that the scheme and
host are case-insensitive and therefore should be normalized to
lowercase. For example, the URI <HTTP://www.EXAMPLE.com/> is
equivalent to <http://www.example.com/>;. The other generic syntax
components are assumed to be case-sensitive unless specifically
defined otherwise by the scheme (see Section 6.2.3).
Thus in our scheme we can assume that the query parameters are indeed
case in-sensitive, but others such as HTTP may disagree:
rfc7230 (Hypertext Transfer Protocol (HTTP/1.1): Message Syntax and Routing)
2.7.3. http and https URI Normalization and Comparison
Since the "http" and "https" schemes conform to the URI generic
syntax, such URIs are normalized and compared according to the
algorithm defined in Section 6 of [RFC3986], using the defaults
described above for each scheme.
If the port is equal to the default port for a scheme, the normal
form is to omit the port subcomponent. When not being used in
absolute form as the request target of an OPTIONS request, an empty
path component is equivalent to an absolute path of "/", so the
normal form is to provide a path of "/" instead. The scheme and host
are case-insensitive and normally provided in lowercase; all other
components are compared in a case-sensitive manner. Characters other
^^^^^^^^^^^^^^^^^^^^^
than those in the "reserved" set are equivalent to their
percent-encoded octets: the normal form is to not encode them (see
Sections 2.1 and 2.2 of [RFC3986]).
The functions you are modifying are used (for now) exclusively for
handling of the URIs based on libvirt-defined schemes, so this does not
create problems for current usage.
Nevertheless the code is placed in a common helper module I suggest you
add a comment describing that it's case sensitive by design.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/viruri.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/viruri.c b/src/util/viruri.c
index 88bb0cc1f8..53f85ed705 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -371,7 +371,7 @@ 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;
}
@@ -403,7 +403,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;
}
If you add a warning into the comment:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
P.S: virURICheckUnixSocket really doesn't look like a helper that should
reside in the generic URI module.
1:33 a.m.
New subject: [PATCH 01/10] viruri: Search params case insensitively
On 2/6/23 16:02, Peter Krempa wrote:
On Mon, Feb 06, 2023 at 10:16:49 +0100, Michal Privoznik wrote:
> 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().
rfc3986 (Uniform Resource Identifier (URI): Generic Syntax) states:
6.2.2.1. Case Normalization
For all URIs, the hexadecimal digits within a percent-encoding
triplet (e.g., "%3a" versus "%3A") are case-insensitive and
therefore
should be normalized to use uppercase letters for the digits A-F.
When a URI uses components of the generic syntax, the component
syntax equivalence rules always apply; namely, that the scheme and
host are case-insensitive and therefore should be normalized to
lowercase. For example, the URI <HTTP://www.EXAMPLE.com/> is
equivalent to <http://www.example.com/>;. The other generic syntax
components are assumed to be case-sensitive unless specifically
defined otherwise by the scheme (see Section 6.2.3).
Thus in our scheme we can assume that the query parameters are indeed
case in-sensitive, but others such as HTTP may disagree:
rfc7230 (Hypertext Transfer Protocol (HTTP/1.1): Message Syntax and Routing)
2.7.3. http and https URI Normalization and Comparison
Since the "http" and "https" schemes conform to the URI generic
syntax, such URIs are normalized and compared according to the
algorithm defined in Section 6 of [RFC3986], using the defaults
described above for each scheme.
If the port is equal to the default port for a scheme, the normal
form is to omit the port subcomponent. When not being used in
absolute form as the request target of an OPTIONS request, an empty
path component is equivalent to an absolute path of "/", so the
normal form is to provide a path of "/" instead. The scheme and host
are case-insensitive and normally provided in lowercase; all other
components are compared in a case-sensitive manner. Characters other
^^^^^^^^^^^^^^^^^^^^^
than those in the "reserved" set are equivalent to their
percent-encoded octets: the normal form is to not encode them (see
Sections 2.1 and 2.2 of [RFC3986]).
The functions you are modifying are used (for now) exclusively for
handling of the URIs based on libvirt-defined schemes, so this does not
create problems for current usage.
Nevertheless the code is placed in a common helper module I suggest you
add a comment describing that it's case sensitive by design.
you mean case
IN-sensitive, right?
For HTTP - I don't think we ever parse that.
But for me, the deciding factor to do this change was consistency. We
already have case insensitive lookup, so maybe there's somebody using
that (although, we don't really document that, so it's not written in
stone, yet).
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/util/viruri.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/viruri.c b/src/util/viruri.c
> index 88bb0cc1f8..53f85ed705 100644
> --- a/src/util/viruri.c
> +++ b/src/util/viruri.c
> @@ -371,7 +371,7 @@ 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;
> }
>
> @@ -403,7 +403,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;
> }
If you add a warning into the comment:
Fair enough. Consider this squashed in:
diff --git i/src/util/viruri.c w/src/util/viruri.c
index 53f85ed705..4afdd30c59 100644
--- i/src/util/viruri.c
+++ w/src/util/viruri.c
@@ -365,6 +365,17 @@ 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)
{
@@ -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
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
Thanks.
P.S: virURICheckUnixSocket really doesn't look like a helper that should
reside in the generic URI module.
I can see where you're coming from, but since this function is used in
two distinct modules (src/libvirt-domain.c and
src/remote/remote_driver.c) I can't think of a better place.
Michal
3:16 a.m.
New subject: [PATCH 02/10] 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>
---
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 53f85ed705..79492e87ce 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:05 a.m.
New subject: [PATCH 02/10] Drop checks for virURIFormat() retval
On Mon, Feb 06, 2023 at 10:16:50 +0100, Michal Privoznik wrote:
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>
---
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(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
3:16 a.m.
New subject: [PATCH 03/10] 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>
---
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:13 a.m.
New subject: [PATCH 03/10] doRemoteOpen(): Rename 'failed' label to 'error'
On Mon, Feb 06, 2023 at 10:16:51 +0100, Michal Privoznik wrote:
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>
---
src/remote/remote_driver.c | 62 +++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 31 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
3:16 a.m.
New subject: [PATCH 04/10] remote_driver: Expose EXTRACT_URI_ARG_* macros
Almost in all places where an URI is parsed we look for
additional argument(s). The remote driver's parsing uses two
macros EXTRACT_URI_ARG_STR() and EXTRACT_URI_ARG_BOOL() for that
purpose. Expose these so that other places can be rewritten using
those macros.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
po/POTFILES | 1 +
src/remote/remote_driver.c | 58 +++++++++++---------------------------
src/util/viruri.h | 23 +++++++++++++++
3 files changed, 40 insertions(+), 42 deletions(-)
diff --git a/po/POTFILES b/po/POTFILES
index 4e446aaf40..2d35def639 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -332,6 +332,7 @@ src/util/virtpm.c
src/util/virtypedparam-public.c
src/util/virtypedparam.c
src/util/viruri.c
+src/util/viruri.h
src/util/virusb.c
src/util/virutil.c
src/util/virvhba.c
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 6a226999df..c41d5b414f 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -693,30 +693,6 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
return rc != -1 && ret.supported;
}
-/* 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)) { \
- VIR_FREE(ARG_VAR); \
- ARG_VAR = g_strdup(var->value); \
- var->ignore = 1; \
- continue; \
- }
-
-#define EXTRACT_URI_ARG_BOOL(ARG_NAME, ARG_VAR) \
- if (STRCASEEQ(var->name, ARG_NAME)) { \
- int tmp; \
- if (virStrToLong_i(var->value, NULL, 10, &tmp) < 0) { \
- virReportError(VIR_ERR_INVALID_ARG, \
- _("Failed to parse value of URI component %s"), \
- var->name); \
- goto error; \
- } \
- ARG_VAR = tmp == 0; \
- var->ignore = 1; \
- continue; \
- }
-
-
/*
* URIs that this driver needs to handle:
*
@@ -796,23 +772,23 @@ doRemoteOpen(virConnectPtr conn,
if (conn->uri) {
for (i = 0; i < conn->uri->paramsCount; i++) {
virURIParam *var = &conn->uri->params[i];
- EXTRACT_URI_ARG_STR("name", name);
- EXTRACT_URI_ARG_STR("command", command);
- EXTRACT_URI_ARG_STR("socket", sockname);
- EXTRACT_URI_ARG_STR("auth", authtype);
- EXTRACT_URI_ARG_STR("sshauth", sshauth);
- EXTRACT_URI_ARG_STR("netcat", netcat);
- EXTRACT_URI_ARG_STR("keyfile", keyfile);
- EXTRACT_URI_ARG_STR("pkipath", pkipath);
- EXTRACT_URI_ARG_STR("known_hosts", knownHosts);
- EXTRACT_URI_ARG_STR("known_hosts_verify", knownHostsVerify);
- EXTRACT_URI_ARG_STR("tls_priority", tls_priority);
- EXTRACT_URI_ARG_STR("mode", mode_str);
- EXTRACT_URI_ARG_STR("proxy", proxy_str);
- EXTRACT_URI_ARG_BOOL("no_sanity", sanity);
- EXTRACT_URI_ARG_BOOL("no_verify", verify);
+ VIR_EXTRACT_URI_ARG_STR("name", name);
+ VIR_EXTRACT_URI_ARG_STR("command", command);
+ VIR_EXTRACT_URI_ARG_STR("socket", sockname);
+ VIR_EXTRACT_URI_ARG_STR("auth", authtype);
+ VIR_EXTRACT_URI_ARG_STR("sshauth", sshauth);
+ VIR_EXTRACT_URI_ARG_STR("netcat", netcat);
+ VIR_EXTRACT_URI_ARG_STR("keyfile", keyfile);
+ VIR_EXTRACT_URI_ARG_STR("pkipath", pkipath);
+ VIR_EXTRACT_URI_ARG_STR("known_hosts", knownHosts);
+ VIR_EXTRACT_URI_ARG_STR("known_hosts_verify", knownHostsVerify);
+ VIR_EXTRACT_URI_ARG_STR("tls_priority", tls_priority);
+ VIR_EXTRACT_URI_ARG_STR("mode", mode_str);
+ VIR_EXTRACT_URI_ARG_STR("proxy", proxy_str);
+ VIR_EXTRACT_URI_ARG_BOOL("no_sanity", sanity, error);
+ VIR_EXTRACT_URI_ARG_BOOL("no_verify", verify, error);
#ifndef WIN32
- EXTRACT_URI_ARG_BOOL("no_tty", tty);
+ VIR_EXTRACT_URI_ARG_BOOL("no_tty", tty, error);
#endif
if (STRCASEEQ(var->name, "authfile")) {
@@ -1206,8 +1182,6 @@ doRemoteOpen(virConnectPtr conn,
VIR_FREE(priv->hostname);
return VIR_DRV_OPEN_ERROR;
}
-#undef EXTRACT_URI_ARG_STR
-#undef EXTRACT_URI_ARG_BOOL
static struct private_data *
remoteAllocPrivateData(void)
diff --git a/src/util/viruri.h b/src/util/viruri.h
index 4f27fa26d2..0e4176c037 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -62,3 +62,26 @@ const char *virURIGetParam(virURI *uri, const char *name);
bool virURICheckUnixSocket(virURI *uri);
#define VIR_URI_SERVER(uri) ((uri) && (uri)->server ? (uri)->server :
"localhost")
+
+/* helper macros to ease extraction of arguments from the URI */
+#define VIR_EXTRACT_URI_ARG_STR(ARG_NAME, ARG_VAR) \
+ if (STRCASEEQ(var->name, ARG_NAME)) { \
+ g_free(ARG_VAR); \
+ ARG_VAR = g_strdup(var->value); \
+ var->ignore = 1; \
+ continue; \
+ }
+
+#define VIR_EXTRACT_URI_ARG_BOOL(ARG_NAME, ARG_VAR, LABEL) \
+ if (STRCASEEQ(var->name, ARG_NAME)) { \
+ int tmp; \
+ if (virStrToLong_i(var->value, NULL, 10, &tmp) < 0) { \
+ virReportError(VIR_ERR_INVALID_ARG, \
+ _("Failed to parse value of URI component %s"), \
+ var->name); \
+ goto LABEL; \
+ } \
+ ARG_VAR = tmp == 0; \
+ var->ignore = 1; \
+ continue; \
+ }
--
2.39.1
2:05 a.m.
New subject: [PATCH 04/10] remote_driver: Expose EXTRACT_URI_ARG_* macros
On Mon, Feb 06, 2023 at 10:16:52 +0100, Michal Privoznik wrote:
Almost in all places where an URI is parsed we look for
additional argument(s). The remote driver's parsing uses two
macros EXTRACT_URI_ARG_STR() and EXTRACT_URI_ARG_BOOL() for that
purpose. Expose these so that other places can be rewritten using
those macros.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
po/POTFILES | 1 +
src/remote/remote_driver.c | 58 +++++++++++---------------------------
src/util/viruri.h | 23 +++++++++++++++
3 files changed, 40 insertions(+), 42 deletions(-)
[...]
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 6a226999df..c41d5b414f 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -693,30 +693,6 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
return rc != -1 && ret.supported;
}
-/* 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)) { \
- VIR_FREE(ARG_VAR); \
- ARG_VAR = g_strdup(var->value); \
- var->ignore = 1; \
- continue; \
- }
This style of macros, which apart from arguments also accesses variables
directly from the place where it's expanded or influence control flow
...
@@ -796,23 +772,23 @@ doRemoteOpen(virConnectPtr conn,
if (conn->uri) {
for (i = 0; i < conn->uri->paramsCount; i++) {
virURIParam *var = &conn->uri->params[i];
- EXTRACT_URI_ARG_STR("name", name);
- EXTRACT_URI_ARG_STR("command", command);
- EXTRACT_URI_ARG_STR("socket", sockname);
- EXTRACT_URI_ARG_STR("auth", authtype);
... are really acceptable only in ad-hoc situations as here ...
@@ -1206,8 +1182,6 @@ doRemoteOpen(virConnectPtr conn,
VIR_FREE(priv->hostname);
return VIR_DRV_OPEN_ERROR;
}
-#undef EXTRACT_URI_ARG_STR
-#undef EXTRACT_URI_ARG_BOOL
... where we dispose of it right away.
static struct private_data *
remoteAllocPrivateData(void)
diff --git a/src/util/viruri.h b/src/util/viruri.h
index 4f27fa26d2..0e4176c037 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -62,3 +62,26 @@ const char *virURIGetParam(virURI *uri, const char *name);
bool virURICheckUnixSocket(virURI *uri);
#define VIR_URI_SERVER(uri) ((uri) && (uri)->server ? (uri)->server :
"localhost")
+
+/* helper macros to ease extraction of arguments from the URI */
+#define VIR_EXTRACT_URI_ARG_STR(ARG_NAME, ARG_VAR) \
Exporting it as such is not acceptable. Less so with non-existent docs
outlining the "side effects" not visible from the place where it's
called.
I suggest you turn it into a function (keeping proper naming scheme)
such as:
int
virURIParamExtractStr(virURIParam *par,
const char *name,
char **val)
{
if (STRCASEEQ(par->name, name)) {
g_free(*val);
*val = g_strdup(par->value);
par->ignore = true;
return 1;
}
return 0;
}
Alternatively return a bool.
And use it as:
if (virURIParamExtractStr(var, "name", &name) == 1)
continue;
+ if (STRCASEEQ(var->name, ARG_NAME)) { \
+ g_free(ARG_VAR); \
+ ARG_VAR = g_strdup(var->value); \
+ var->ignore = 1; \
+ continue; \
+ }
10:43 a.m.
New subject: [PATCH 04/10] remote_driver: Expose EXTRACT_URI_ARG_* macros
On 2/7/23 09:05, Peter Krempa wrote:
On Mon, Feb 06, 2023 at 10:16:52 +0100, Michal Privoznik wrote:
> Almost in all places where an URI is parsed we look for
> additional argument(s). The remote driver's parsing uses two
> macros EXTRACT_URI_ARG_STR() and EXTRACT_URI_ARG_BOOL() for that
> purpose. Expose these so that other places can be rewritten using
> those macros.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> po/POTFILES | 1 +
> src/remote/remote_driver.c | 58 +++++++++++---------------------------
> src/util/viruri.h | 23 +++++++++++++++
> 3 files changed, 40 insertions(+), 42 deletions(-)
[...]
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 6a226999df..c41d5b414f 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -693,30 +693,6 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
> return rc != -1 && ret.supported;
> }
>
> -/* 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)) { \
> - VIR_FREE(ARG_VAR); \
> - ARG_VAR = g_strdup(var->value); \
> - var->ignore = 1; \
> - continue; \
> - }
This style of macros, which apart from arguments also accesses variables
directly from the place where it's expanded or influence control flow
...
Fair enough. I can do without this patch though. So let me just drop it
from v2.
Michal
10:48 a.m.
New subject: [PATCH 04/10] remote_driver: Expose EXTRACT_URI_ARG_* macros
On Tue, Feb 07, 2023 at 09:05:36AM +0100, Peter Krempa wrote:
On Mon, Feb 06, 2023 at 10:16:52 +0100, Michal Privoznik wrote:
> Almost in all places where an URI is parsed we look for
> additional argument(s). The remote driver's parsing uses two
> macros EXTRACT_URI_ARG_STR() and EXTRACT_URI_ARG_BOOL() for that
> purpose. Expose these so that other places can be rewritten using
> those macros.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> po/POTFILES | 1 +
> src/remote/remote_driver.c | 58 +++++++++++---------------------------
> src/util/viruri.h | 23 +++++++++++++++
> 3 files changed, 40 insertions(+), 42 deletions(-)
[...]
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 6a226999df..c41d5b414f 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -693,30 +693,6 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn,
> return rc != -1 && ret.supported;
> }
>
> -/* 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)) { \
> - VIR_FREE(ARG_VAR); \
> - ARG_VAR = g_strdup(var->value); \
> - var->ignore = 1; \
> - continue; \
> - }
This style of macros, which apart from arguments also accesses variables
directly from the place where it's expanded or influence control flow
...
> @@ -796,23 +772,23 @@ doRemoteOpen(virConnectPtr conn,
> if (conn->uri) {
> for (i = 0; i < conn->uri->paramsCount; i++) {
> virURIParam *var = &conn->uri->params[i];
> - EXTRACT_URI_ARG_STR("name", name);
> - EXTRACT_URI_ARG_STR("command", command);
> - EXTRACT_URI_ARG_STR("socket", sockname);
> - EXTRACT_URI_ARG_STR("auth", authtype);
... are really acceptable only in ad-hoc situations as here ...
> @@ -1206,8 +1182,6 @@ doRemoteOpen(virConnectPtr conn,
> VIR_FREE(priv->hostname);
> return VIR_DRV_OPEN_ERROR;
> }
> -#undef EXTRACT_URI_ARG_STR
> -#undef EXTRACT_URI_ARG_BOOL
... where we dispose of it right away.
>
> static struct private_data *
> remoteAllocPrivateData(void)
> diff --git a/src/util/viruri.h b/src/util/viruri.h
> index 4f27fa26d2..0e4176c037 100644
> --- a/src/util/viruri.h
> +++ b/src/util/viruri.h
> @@ -62,3 +62,26 @@ const char *virURIGetParam(virURI *uri, const char *name);
> bool virURICheckUnixSocket(virURI *uri);
>
> #define VIR_URI_SERVER(uri) ((uri) && (uri)->server ? (uri)->server :
"localhost")
> +
> +/* helper macros to ease extraction of arguments from the URI */
> +#define VIR_EXTRACT_URI_ARG_STR(ARG_NAME, ARG_VAR) \
Exporting it as such is not acceptable. Less so with non-existent docs
outlining the "side effects" not visible from the place where it's
called.
I suggest you turn it into a function (keeping proper naming scheme)
such as:
int
virURIParamExtractStr(virURIParam *par,
const char *name,
char **val)
{
if (STRCASEEQ(par->name, name)) {
g_free(*val);
*val = g_strdup(par->value);
par->ignore = true;
return 1;
}
return 0;
}
Alternatively return a bool.
And use it as:
if (virURIParamExtractStr(var, "name", &name) == 1)
continue;
Alternatively declare this code all redundant optimizatio and
just use the existing virURIGetParam instead. It'll iterate
over the parameters many times, but that's irrelevant when
we normally have zero or one parameters :-)
With 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 :|
3:16 a.m.
New subject: [PATCH 05/10] src: Unify URI params parsing
Now that we have VIR_EXTRACT_URI_ARG_* macros, we can use them in
the rest of the places where an URI is parsed. This also unifies
behavior wrt to query arguments handling. For instance, our
virAdmConnectOpen() accepts "?socket=..." but not "?SOCKET="
whereas plain virConnectOpen() does accept both.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/admin/libvirt-admin.c | 15 +++---
src/esx/esx_util.c | 96 +++++++++++++++++++--------------------
src/hyperv/hyperv_util.c | 30 ++++++------
src/qemu/qemu_migration.c | 21 ++++-----
src/util/virauth.c | 12 ++---
src/util/viruri.h | 12 +++++
6 files changed, 92 insertions(+), 94 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 1786a283e5..0f2410d18d 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -109,16 +109,13 @@ getSocketPath(virURI *uri)
for (i = 0; i < uri->paramsCount; i++) {
- virURIParam *param = &uri->params[i];
+ virURIParam *var = &uri->params[i];
- if (STREQ(param->name, "socket")) {
- g_free(sock_path);
- sock_path = g_strdup(param->value);
- } else {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown URI parameter '%s'"),
param->name);
- return NULL;
- }
+ VIR_EXTRACT_URI_ARG_STR("socket", sock_path);
+
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown URI parameter '%s'"), var->name);
+ return NULL;
}
if (!sock_path) {
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 89d5517262..8c0f39ecc9 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -40,8 +40,8 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURI *uri)
{
int result = -1;
size_t i;
- int noVerify;
- int autoAnswer;
+ int noVerify = -1;
+ int autoAnswer = -1;
char *tmp;
ESX_VI_CHECK_ARG_LIST(parsedUri);
@@ -49,71 +49,39 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURI *uri)
*parsedUri = g_new0(esxUtil_ParsedUri, 1);
for (i = 0; i < uri->paramsCount; i++) {
- virURIParam *queryParam = &uri->params[i];
+ virURIParam *var = &uri->params[i];
- if (STRCASEEQ(queryParam->name, "transport")) {
- g_free((*parsedUri)->transport);
+ VIR_EXTRACT_URI_ARG_STR("transport", (*parsedUri)->transport);
+ VIR_EXTRACT_URI_ARG_STR("vcenter", (*parsedUri)->vCenter);
+ VIR_EXTRACT_URI_ARG_INT("no_verify", noVerify, cleanup);
+ VIR_EXTRACT_URI_ARG_INT("auto_answer", autoAnswer, cleanup);
- (*parsedUri)->transport = g_strdup(queryParam->value);
-
- if (STRNEQ((*parsedUri)->transport, "http") &&
- STRNEQ((*parsedUri)->transport, "https")) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("Query parameter 'transport' has unexpected
value "
- "'%s' (should be http|https)"),
- (*parsedUri)->transport);
- goto cleanup;
- }
- } else if (STRCASEEQ(queryParam->name, "vcenter")) {
- g_free((*parsedUri)->vCenter);
-
- (*parsedUri)->vCenter = g_strdup(queryParam->value);
- } else if (STRCASEEQ(queryParam->name, "no_verify")) {
- if (virStrToLong_i(queryParam->value, NULL, 10, &noVerify) < 0 ||
- (noVerify != 0 && noVerify != 1)) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("Query parameter 'no_verify' has unexpected
value "
- "'%s' (should be 0 or 1)"),
queryParam->value);
- goto cleanup;
- }
-
- (*parsedUri)->noVerify = noVerify != 0;
- } else if (STRCASEEQ(queryParam->name, "auto_answer")) {
- if (virStrToLong_i(queryParam->value, NULL, 10, &autoAnswer) < 0
||
- (autoAnswer != 0 && autoAnswer != 1)) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("Query parameter 'auto_answer' has
unexpected "
- "value '%s' (should be 0 or 1)"),
queryParam->value);
- goto cleanup;
- }
-
- (*parsedUri)->autoAnswer = autoAnswer != 0;
- } else if (STRCASEEQ(queryParam->name, "proxy")) {
+ if (STRCASEEQ(var->name, "proxy")) {
/* Expected format: [<type>://]<hostname>[:<port>] */
(*parsedUri)->proxy = true;
(*parsedUri)->proxy_type = CURLPROXY_HTTP;
g_clear_pointer(&(*parsedUri)->proxy_hostname, g_free);
(*parsedUri)->proxy_port = 1080;
- if ((tmp = STRSKIP(queryParam->value, "http://"))) {
+ if ((tmp = STRSKIP(var->value, "http://"))) {
(*parsedUri)->proxy_type = CURLPROXY_HTTP;
- } else if ((tmp = STRSKIP(queryParam->value, "socks://")) ||
- (tmp = STRSKIP(queryParam->value, "socks5://"))) {
+ } else if ((tmp = STRSKIP(var->value, "socks://")) ||
+ (tmp = STRSKIP(var->value, "socks5://"))) {
(*parsedUri)->proxy_type = CURLPROXY_SOCKS5;
- } else if ((tmp = STRSKIP(queryParam->value, "socks4://"))) {
+ } else if ((tmp = STRSKIP(var->value, "socks4://"))) {
(*parsedUri)->proxy_type = CURLPROXY_SOCKS4;
- } else if ((tmp = STRSKIP(queryParam->value, "socks4a://"))) {
+ } else if ((tmp = STRSKIP(var->value, "socks4a://"))) {
(*parsedUri)->proxy_type = CURLPROXY_SOCKS4A;
- } else if ((tmp = strstr(queryParam->value, "://"))) {
+ } else if ((tmp = strstr(var->value, "://"))) {
*tmp = '\0';
virReportError(VIR_ERR_INVALID_ARG,
_("Query parameter 'proxy' contains
unexpected "
"type '%s' (should be
(http|socks(|4|4a|5))"),
- queryParam->value);
+ var->value);
goto cleanup;
} else {
- tmp = queryParam->value;
+ tmp = var->value;
}
(*parsedUri)->proxy_hostname = g_strdup(tmp);
@@ -141,7 +109,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURI *uri)
}
} else {
VIR_WARN("Ignoring unexpected query parameter '%s'",
- queryParam->name);
+ var->name);
}
}
@@ -150,6 +118,36 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURI *uri)
if (!(*parsedUri)->transport)
(*parsedUri)->transport = g_strdup("https");
+ if (STRNEQ((*parsedUri)->transport, "http") &&
+ STRNEQ((*parsedUri)->transport, "https")) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("Query parameter 'transport' has unexpected value
'%s' (should be http|https)"),
+ (*parsedUri)->transport);
+ goto cleanup;
+ }
+
+ if (noVerify >= 0) {
+ if (noVerify != 0 && noVerify != 1) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("Query parameter 'no_verify' has unexpected
value '%d' (should be 0 or 1)"),
+ noVerify);
+ goto cleanup;
+ }
+
+ (*parsedUri)->noVerify = noVerify != 0;
+ }
+
+ if (autoAnswer >= 0) {
+ if (autoAnswer != 0 && autoAnswer != 1) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("Query parameter 'auto_answer' has unexpected
value '%d' (should be 0 or 1)"),
+ autoAnswer);
+ goto cleanup;
+ }
+
+ (*parsedUri)->autoAnswer = autoAnswer != 0;
+ }
+
result = 0;
cleanup:
diff --git a/src/hyperv/hyperv_util.c b/src/hyperv/hyperv_util.c
index fe71e47285..38daa27743 100644
--- a/src/hyperv/hyperv_util.c
+++ b/src/hyperv/hyperv_util.c
@@ -45,30 +45,26 @@ hypervParseUri(hypervParsedUri **parsedUri, virURI *uri)
*parsedUri = g_new0(hypervParsedUri, 1);
for (i = 0; i < uri->paramsCount; i++) {
- virURIParam *queryParam = &uri->params[i];
+ virURIParam *var = &uri->params[i];
- if (STRCASEEQ(queryParam->name, "transport")) {
- VIR_FREE((*parsedUri)->transport);
+ VIR_EXTRACT_URI_ARG_STR("transport", (*parsedUri)->transport);
- (*parsedUri)->transport = g_strdup(queryParam->value);
-
- if (STRNEQ((*parsedUri)->transport, "http") &&
- STRNEQ((*parsedUri)->transport, "https")) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("Query parameter 'transport' has unexpected
value "
- "'%s' (should be http|https)"),
- (*parsedUri)->transport);
- goto cleanup;
- }
- } else {
- VIR_WARN("Ignoring unexpected query parameter '%s'",
- queryParam->name);
- }
+ VIR_WARN("Ignoring unexpected query parameter '%s'",
+ var->name);
}
if (!(*parsedUri)->transport)
(*parsedUri)->transport = g_strdup("https");
+ if (STRNEQ((*parsedUri)->transport, "http") &&
+ STRNEQ((*parsedUri)->transport, "https")) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("Query parameter 'transport' has unexpected value
"
+ "'%s' (should be http|https)"),
+ (*parsedUri)->transport);
+ goto cleanup;
+ }
+
result = 0;
cleanup:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2720f0b083..3c38be46f7 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2080,7 +2080,7 @@ qemuMigrationSrcGraphicsRelocate(virDomainObj *vm,
int type = -1;
int port = -1;
int tlsPort = -1;
- const char *tlsSubject = NULL;
+ g_autofree char *tlsSubject = NULL;
int rc = -1;
if (!cookie || (!cookie->graphics && !graphicsuri))
@@ -2101,7 +2101,7 @@ qemuMigrationSrcGraphicsRelocate(virDomainObj *vm,
port = cookie->graphics->port;
tlsPort = cookie->graphics->tlsPort;
- tlsSubject = cookie->graphics->tlsSubject;
+ tlsSubject = g_strdup(cookie->graphics->tlsSubject);
}
if (uri) {
@@ -2119,18 +2119,10 @@ qemuMigrationSrcGraphicsRelocate(virDomainObj *vm,
port = uri->port;
for (i = 0; i < uri->paramsCount; i++) {
- virURIParam *param = uri->params + i;
+ virURIParam *var = &uri->params[i];
- if (STRCASEEQ(param->name, "tlsPort")) {
- if (virStrToLong_i(param->value, NULL, 10, &tlsPort) < 0) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("invalid tlsPort number: %s"),
- param->value);
- return -1;
- }
- } else if (STRCASEEQ(param->name, "tlsSubject")) {
- tlsSubject = param->value;
- }
+ VIR_EXTRACT_URI_ARG_INT("tlsPort", tlsPort, error);
+ VIR_EXTRACT_URI_ARG_STR("tlsSubject", tlsSubject);
}
}
@@ -2157,6 +2149,9 @@ qemuMigrationSrcGraphicsRelocate(virDomainObj *vm,
}
return rc;
+
+ error:
+ return -1;
}
diff --git a/src/util/virauth.c b/src/util/virauth.c
index 7b4a1bd8a5..afebb609a7 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -57,13 +57,13 @@ virAuthGetConfigFilePathURI(virURI *uri,
if (uri) {
for (i = 0; i < uri->paramsCount; i++) {
- if (STREQ_NULLABLE(uri->params[i].name, "authfile") &&
- uri->params[i].value) {
- VIR_DEBUG("Using path from URI '%s'",
uri->params[i].value);
- *path = g_strdup(uri->params[i].value);
- return 0;
- }
+ virURIParam *var = &uri->params[i];
+
+ VIR_EXTRACT_URI_ARG_STR("authfile", *path);
}
+
+ if (*path)
+ return 0;
}
userdir = virGetUserConfigDirectory();
diff --git a/src/util/viruri.h b/src/util/viruri.h
index 0e4176c037..7e4f95a2b1 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -72,6 +72,18 @@ bool virURICheckUnixSocket(virURI *uri);
continue; \
}
+#define VIR_EXTRACT_URI_ARG_INT(ARG_NAME, ARG_VAR, LABEL) \
+ if (STRCASEEQ(var->name, ARG_NAME)) { \
+ if (virStrToLong_i(var->value, NULL, 10, &(ARG_VAR)) < 0) { \
+ virReportError(VIR_ERR_INVALID_ARG, \
+ _("Failed to parse value of URI component %s"), \
+ var->name); \
+ goto LABEL; \
+ } \
+ var->ignore = 1; \
+ continue; \
+ }
+
#define VIR_EXTRACT_URI_ARG_BOOL(ARG_NAME, ARG_VAR, LABEL) \
if (STRCASEEQ(var->name, ARG_NAME)) { \
int tmp; \
--
2.39.1
3:16 a.m.
New subject: [PATCH 06/10] 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 | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_ssh_helper.c b/src/remote/remote_ssh_helper.c
index b4735027be..3b4de7f214 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;
+ g_autofree 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,20 @@ 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];
+
+ VIR_EXTRACT_URI_ARG_STR("mode", mode_str);
+ }
+
+ 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
2:16 a.m.
New subject: [PATCH 06/10] virt-ssh-helper: Accept ?mode= in connection URI
On Mon, Feb 06, 2023 at 10:16:54 +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 | 17 ++++++++++++++++-
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
3:16 a.m.
New subject: [PATCH 07/10] 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 | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/remote/remote_ssh_helper.c b/src/remote/remote_ssh_helper.c
index 3b4de7f214..0eafc70d16 100644
--- a/src/remote/remote_ssh_helper.c
+++ b/src/remote/remote_ssh_helper.c
@@ -436,6 +436,7 @@ int main(int argc, char **argv)
virURIParam *var = &uri->params[i];
VIR_EXTRACT_URI_ARG_STR("mode", mode_str);
+ VIR_EXTRACT_URI_ARG_STR("socket", sock_path);
}
if (mode_str &&
@@ -444,11 +445,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
2:17 a.m.
New subject: [PATCH 07/10] virt-ssh-helper: Accept ?socket= in connection URI
On Mon, Feb 06, 2023 at 10:16:55 +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 | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
3:16 a.m.
New subject: [PATCH 08/10] remote_driver: Move URI re-generation into a function
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).
Now, this URI reconstruction is currently implemented in an else
branch. Move it into a separate function so that it can be
re-used.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_driver.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index c41d5b414f..7e1a31a5a0 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -693,6 +693,22 @@ 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);
+}
+
/*
* URIs that this driver needs to handle:
*
@@ -809,16 +825,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
2:21 a.m.
New subject: [PATCH 08/10] remote_driver: Move URI re-generation into a function
On Mon, Feb 06, 2023 at 10:16:56 +0100, Michal Privoznik wrote:
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).
Did you mean to put all the explanation above into the patch that is
going to modify it? Here it doesn't make much sense as ....
Now, this URI reconstruction is currently implemented in an else
branch. Move it into a separate function so that it can be
re-used.
... this patch simply moves the code doing the URI construction, where
you can't se what's actually happening to the parameters into a new
helper.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_driver.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
3:16 a.m.
New subject: [PATCH 09/10] 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>
---
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 79492e87ce..85a9355918 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -409,3 +409,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 7e4f95a2b1..de59e7f0f8 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -61,6 +61,8 @@ 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")
/* helper macros to ease extraction of arguments from the URI */
--
2.39.1
2:55 a.m.
New subject: [PATCH 09/10] viruri: Introduce virURIParamsSetIgnore()
On Mon, Feb 06, 2023 at 10:16:57 +0100, Michal Privoznik wrote:
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>
---
src/libvirt_private.syms | 1 +
src/util/viruri.c | 18 ++++++++++++++++++
src/util/viruri.h | 2 ++
3 files changed, 21 insertions(+)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
3:16 a.m.
New subject: [PATCH 10/10] remote: Pass 'mode' and 'socket' URI parameters to virt-ssh-helper
Even though we have split daemons for some time now, they are not
the default by far. We've made the change ~1.5 year ago (in
v7.5.0-rc1~35).
Therefore, we have some users that use 'mode=legacy' parameter in
their connection URI. But this parameter is not propagated to
virt-ssh-helper (and neither is the 'socket=...' parameter).
But now that virt-ssh-helper parses those URI parameters, we can
pass them onto the remote host.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/433
Signed-off-by: Michal Privoznik <mprivozn(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 7e1a31a5a0..f8f2dc0636 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -695,18 +695,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;
}
/*
@@ -754,6 +767,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
@@ -825,7 +839,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);
}
}
@@ -980,7 +997,7 @@ doRemoteOpen(virConnectPtr conn,
proxy,
netcat,
sockname,
- name,
+ virtSshURI ? virtSshURI : name,
flags & REMOTE_DRIVER_OPEN_RO,
auth,
conn->uri);
@@ -1004,7 +1021,7 @@ doRemoteOpen(virConnectPtr conn,
proxy,
netcat,
sockname,
- name,
+ virtSshURI ? virtSshURI : name,
flags & REMOTE_DRIVER_OPEN_RO,
auth,
conn->uri);
@@ -1037,7 +1054,7 @@ doRemoteOpen(virConnectPtr conn,
proxy,
netcat,
sockname,
- name,
+ virtSshURI ? virtSshURI : name,
flags & REMOTE_DRIVER_OPEN_RO)))
goto error;
--
2.39.1
2:58 a.m.
New subject: [PATCH 10/10] remote: Pass 'mode' and 'socket' URI parameters to virt-ssh-helper
On Mon, Feb 06, 2023 at 10:16:58 +0100, Michal Privoznik wrote:
Even though we have split daemons for some time now, they are not
the default by far. We've made the change ~1.5 year ago (in
v7.5.0-rc1~35).
Therefore, we have some users that use 'mode=legacy' parameter in
their connection URI. But this parameter is not propagated to
virt-ssh-helper (and neither is the 'socket=...' parameter).
But now that virt-ssh-helper parses those URI parameters, we can
pass them onto the remote host.
This commit message would benefit more from the explanation of how the
parameters are stripped, so you can explain why it's needed to be
skipped.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/433
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_driver.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
654
days inactive
655
days old
22 comments
4 participants
participants (4)
-
Daniel P. Berrangé -
Michal Privoznik -
Michal Prívozník
-
Peter Krempa