6:58 a.m.
When libvirtd is built without a driver we report a terrible error
message which doesn't really point to what's happening.
v2:
- propagate that we want to avoid the remote driver internally rather
than via a public API flag
Peter Krempa (9):
virConnectOpenInternal: Avoid double free() when alias is an invalid
URI
virConnectOpenInternal: Switch to automatic memory cleanup
virConnectOpenInternal: Remove 'failed' label
remote: remoteOpenConn: Use virConnectOpenAuth instead of
virConnectOpen(ReadOnly)
remoteConnectOpen: Refactor cleanup
remote: doRemoteOpen: Automatically clean up 'priv'
lxc: Remove unneeded forward declaration of 'lxcStateInitialize'
virStateInitialize: Propagate whether running in monolithic daemon
mode to stateful driver init
remote: Don't attempt remote connection from libvirtd
src/bhyve/bhyve_driver.c | 1 +
src/ch/ch_driver.c | 1 +
src/driver-state.h | 1 +
src/interface/interface_backend_netcf.c | 1 +
src/interface/interface_backend_udev.c | 1 +
src/libvirt.c | 67 +++++++++++--------------
src/libvirt_internal.h | 1 +
src/libxl/libxl_driver.c | 1 +
src/lxc/lxc_driver.c | 5 +-
src/network/bridge_driver.c | 1 +
src/node_device/node_device_udev.c | 1 +
src/nwfilter/nwfilter_driver.c | 1 +
src/qemu/qemu_driver.c | 1 +
src/remote/remote_daemon.c | 6 +++
src/remote/remote_daemon_dispatch.c | 7 ++-
src/remote/remote_driver.c | 59 +++++++++++-----------
src/secret/secret_driver.c | 1 +
src/storage/storage_driver.c | 1 +
src/vz/vz_driver.c | 1 +
19 files changed, 83 insertions(+), 75 deletions(-)
--
2.37.1
6:58 a.m.
New subject: [PATCH v2 1/9] virConnectOpenInternal: Avoid double free() when alias is an invalid URI
Configuring an URI alias such as
uri_aliases = [
"blah=qemu://invaliduri@@@",
]
Results in a double free when the alias is used:
$ virsh -c blah
free(): double free detected in tcache 2
Aborted (core dumped)
This happens as the 'alias' variable is first assigned to 'uristr' which
is cleared in the 'failed' label and then is explicitly freed again.
Fix this by switching to 'g_autofree' for alias and stealing it into
'uristr'.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index b78b49a632..7e7c0fc8e3 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -933,21 +933,19 @@ virConnectOpenInternal(const char *name,
}
if (uristr) {
- char *alias = NULL;
+ g_autofree char *alias = NULL;
if (!(flags & VIR_CONNECT_NO_ALIASES) &&
virURIResolveAlias(conf, uristr, &alias) < 0)
goto failed;
if (alias) {
- VIR_FREE(uristr);
- uristr = alias;
+ g_free(uristr);
+ uristr = g_steal_pointer(&alias);
}
- if (!(ret->uri = virURIParse(uristr))) {
- VIR_FREE(alias);
+ if (!(ret->uri = virURIParse(uristr)))
goto failed;
- }
/* Avoid need for drivers to worry about NULLs, as
* no one needs to distinguish "" vs NULL */
--
2.37.1
11:21 a.m.
New subject: [PATCH v2 1/9] virConnectOpenInternal: Avoid double free() when alias is an invalid URI
On a Friday in 2022, Peter Krempa wrote:
Configuring an URI alias such as
uri_aliases = [
"blah=qemu://invaliduri@@@",
]
Results in a double free when the alias is used:
$ virsh -c blah
free(): double free detected in tcache 2
Aborted (core dumped)
This happens as the 'alias' variable is first assigned to 'uristr' which
is cleared in the 'failed' label and then is explicitly freed again.
Fix this by switching to 'g_autofree' for alias and stealing it into
'uristr'.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index b78b49a632..7e7c0fc8e3 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -933,21 +933,19 @@ virConnectOpenInternal(const char *name,
}
if (uristr) {
- char *alias = NULL;
+ g_autofree char *alias = NULL;
Is this necessary? In the case 'alias' will be assigned a value,
we will steal the value right away.
if (!(flags & VIR_CONNECT_NO_ALIASES) &&
virURIResolveAlias(conf, uristr, &alias) < 0)
goto failed;
if (alias) {
- VIR_FREE(uristr);
- uristr = alias;
+ g_free(uristr);
+ uristr = g_steal_pointer(&alias);
}
- if (!(ret->uri = virURIParse(uristr))) {
- VIR_FREE(alias);
+ if (!(ret->uri = virURIParse(uristr)))
goto failed;
- }
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
6:58 a.m.
New subject: [PATCH v2 2/9] virConnectOpenInternal: Switch to automatic memory cleanup
Use automatic cleanup for 'ret' and 'uristr'.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 7e7c0fc8e3..fca38dba40 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -882,9 +882,9 @@ virConnectOpenInternal(const char *name,
{
size_t i;
int res;
- virConnectPtr ret;
+ g_autoptr(virConnect) ret = NULL;
g_autoptr(virConf) conf = NULL;
- char *uristr = NULL;
+ g_autofree char *uristr = NULL;
bool embed = false;
ret = virGetConnect();
@@ -1151,14 +1151,9 @@ virConnectOpenInternal(const char *name,
goto failed;
}
- VIR_FREE(uristr);
-
- return ret;
+ return g_steal_pointer(&ret);
failed:
- VIR_FREE(uristr);
- virObjectUnref(ret);
-
return NULL;
}
--
2.37.1
6:58 a.m.
New subject: [PATCH v2 3/9] virConnectOpenInternal: Remove 'failed' label
Jumping to the label would just return NULL.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt.c | 41 +++++++++++++++++++----------------------
1 file changed, 19 insertions(+), 22 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index fca38dba40..d5ae68d16f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -892,7 +892,7 @@ virConnectOpenInternal(const char *name,
return NULL;
if (virConfLoadConfig(&conf, "libvirt.conf") < 0)
- goto failed;
+ return NULL;
if (name && name[0] == '\0')
name = NULL;
@@ -916,14 +916,14 @@ virConnectOpenInternal(const char *name,
uristr = g_strdup(name);
} else {
if (virConnectGetDefaultURI(conf, &uristr) < 0)
- goto failed;
+ return NULL;
if (uristr == NULL) {
VIR_DEBUG("Trying to probe for default URI");
for (i = 0; i < virConnectDriverTabCount && uristr == NULL; i++)
{
if (virConnectDriverTab[i]->hypervisorDriver->connectURIProbe) {
if
(virConnectDriverTab[i]->hypervisorDriver->connectURIProbe(&uristr) < 0)
- goto failed;
+ return NULL;
VIR_DEBUG("%s driver URI probe returned '%s'",
virConnectDriverTab[i]->hypervisorDriver->name,
NULLSTR(uristr));
@@ -937,7 +937,7 @@ virConnectOpenInternal(const char *name,
if (!(flags & VIR_CONNECT_NO_ALIASES) &&
virURIResolveAlias(conf, uristr, &alias) < 0)
- goto failed;
+ return NULL;
if (alias) {
g_free(uristr);
@@ -945,7 +945,7 @@ virConnectOpenInternal(const char *name,
}
if (!(ret->uri = virURIParse(uristr)))
- goto failed;
+ return NULL;
/* Avoid need for drivers to worry about NULLs, as
* no one needs to distinguish "" vs NULL */
@@ -967,12 +967,12 @@ virConnectOpenInternal(const char *name,
virReportError(VIR_ERR_NO_CONNECT,
_("URI '%s' does not include a driver
name"),
name);
- goto failed;
+ return NULL;
}
if (virConnectCheckURIMissingSlash(uristr,
ret->uri) < 0) {
- goto failed;
+ return NULL;
}
if (STREQ(ret->uri->path, "/embed")) {
@@ -985,26 +985,26 @@ virConnectOpenInternal(const char *name,
virReportError(VIR_ERR_NO_CONNECT,
_("URI scheme '%s' for embedded driver is not
valid"),
ret->uri->scheme);
- goto failed;
+ return NULL;
}
root = virURIGetParam(ret->uri, "root");
if (!root)
- goto failed;
+ return NULL;
if (!g_path_is_absolute(root)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("root path must be absolute"));
- goto failed;
+ return NULL;
}
if (virEventRequireImpl() < 0)
- goto failed;
+ return NULL;
regMethod = g_strdup_printf("%sRegister", ret->uri->scheme);
if (virDriverLoadModule(ret->uri->scheme, regMethod, false) < 0)
- goto failed;
+ return NULL;
if (virAccessManagerGetDefault() == NULL) {
virAccessManager *acl;
@@ -1012,12 +1012,12 @@ virConnectOpenInternal(const char *name,
virResetLastError();
if (!(acl = virAccessManagerNew("none")))
- goto failed;
+ return NULL;
virAccessManagerSetDefault(acl);
}
if (virStateInitialize(geteuid() == 0, true, root, NULL, NULL) < 0)
- goto failed;
+ return NULL;
embed = true;
}
@@ -1055,7 +1055,7 @@ virConnectOpenInternal(const char *name,
__FILE__, __FUNCTION__, __LINE__,
_("libvirt was built without the '%s'
driver"),
ret->uri->scheme);
- goto failed;
+ return NULL;
}
VIR_DEBUG("trying driver %zu (%s) ...",
@@ -1105,13 +1105,13 @@ virConnectOpenInternal(const char *name,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Driver %s cannot be used in embedded mode"),
virConnectDriverTab[i]->hypervisorDriver->name);
- goto failed;
+ return NULL;
}
/* before starting the new connection, check if the driver only works
* with a server, and so return an error if the server is missing */
if (virConnectDriverTab[i]->remoteOnly && ret->uri &&
!ret->uri->server) {
virReportError(VIR_ERR_INVALID_ARG, "%s", _("URI is missing
the server part"));
- goto failed;
+ return NULL;
}
ret->driver = virConnectDriverTab[i]->hypervisorDriver;
@@ -1141,20 +1141,17 @@ virConnectOpenInternal(const char *name,
ret->storageDriver = NULL;
if (res == VIR_DRV_OPEN_ERROR)
- goto failed;
+ return NULL;
}
}
if (!ret->driver) {
/* If we reach here, then all drivers declined the connection. */
virReportError(VIR_ERR_NO_CONNECT, "%s", NULLSTR(name));
- goto failed;
+ return NULL;
}
return g_steal_pointer(&ret);
-
- failed:
- return NULL;
}
--
2.37.1
6:58 a.m.
New subject: [PATCH v2 4/9] remote: remoteOpenConn: Use virConnectOpenAuth instead of virConnectOpen(ReadOnly)
virConnectOpenAuth provides an unified interface with using 'flags' to
select the proper mode.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index dc5790f077..061e0f7811 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1790,6 +1790,7 @@ remoteOpenConn(const char *uri,
{
g_autoptr(virTypedParamList) identparams = NULL;
g_autoptr(virConnect) newconn = NULL;
+ unsigned int connectFlags = 0;
VIR_DEBUG("Getting secondary uri=%s readonly=%d preserveIdent=%d conn=%p",
NULLSTR(uri), readonly, preserveIdentity, conn);
@@ -1814,11 +1815,9 @@ remoteOpenConn(const char *uri,
VIR_DEBUG("Opening driver %s", uri);
if (readonly)
- newconn = virConnectOpenReadOnly(uri);
- else
- newconn = virConnectOpen(uri);
+ connectFlags |= VIR_CONNECT_RO;
- if (!newconn)
+ if (!(newconn = virConnectOpenAuth(uri, NULL, connectFlags)))
return -1;
VIR_DEBUG("Opened driver %p", newconn);
--
2.37.1
6:58 a.m.
New subject: [PATCH v2 5/9] remoteConnectOpen: Refactor cleanup
Use automatic memory freeing for 'driver' and return error right away to
avoid the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/remote/remote_driver.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 0ca365c4cc..b670284211 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1228,23 +1228,20 @@ remoteConnectOpen(virConnectPtr conn,
struct private_data *priv;
int ret = VIR_DRV_OPEN_ERROR;
unsigned int rflags = 0;
- char *driver = NULL;
+ g_autofree char *driver = NULL;
remoteDriverTransport transport;
if (conn->uri) {
if (remoteSplitURIScheme(conn->uri, &driver, &transport) < 0)
- goto cleanup;
+ return VIR_DRV_OPEN_ERROR;
} else {
/* No URI, then must be probing so use UNIX socket */
transport = REMOTE_DRIVER_TRANSPORT_UNIX;
}
-
if (inside_daemon) {
- if (!conn->uri) {
- ret = VIR_DRV_OPEN_DECLINED;
- goto cleanup;
- }
+ if (!conn->uri)
+ return VIR_DRV_OPEN_DECLINED;
/* If there's a driver registered we must defer to that.
* If there isn't a driver, we must connect in "direct"
@@ -1254,15 +1251,12 @@ remoteConnectOpen(virConnectPtr conn,
* host */
if (!conn->uri->server &&
virHasDriverForURIScheme(driver) &&
- !virURICheckUnixSocket(conn->uri)) {
- ret = VIR_DRV_OPEN_DECLINED;
- goto cleanup;
- }
+ !virURICheckUnixSocket(conn->uri))
+ return VIR_DRV_OPEN_DECLINED;
}
if (!(priv = remoteAllocPrivateData()))
- goto cleanup;
-
+ return VIR_DRV_OPEN_ERROR;
remoteGetURIDaemonInfo(conn->uri, transport, &rflags);
if (flags & VIR_CONNECT_RO)
@@ -1278,8 +1272,6 @@ remoteConnectOpen(virConnectPtr conn,
remoteDriverUnlock(priv);
}
- cleanup:
- VIR_FREE(driver);
return ret;
}
--
2.37.1
6:58 a.m.
New subject: [PATCH v2 6/9] remote: doRemoteOpen: Automatically clean up 'priv'
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/remote/remote_driver.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b670284211..25c80a09c7 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1225,7 +1225,7 @@ remoteConnectOpen(virConnectPtr conn,
virConf *conf,
unsigned int flags)
{
- struct private_data *priv;
+ g_autofree struct private_data *priv = NULL;
int ret = VIR_DRV_OPEN_ERROR;
unsigned int rflags = 0;
g_autofree char *driver = NULL;
@@ -1263,14 +1263,12 @@ remoteConnectOpen(virConnectPtr conn,
rflags |= REMOTE_DRIVER_OPEN_RO;
ret = doRemoteOpen(conn, priv, driver, transport, auth, conf, rflags);
- if (ret != VIR_DRV_OPEN_SUCCESS) {
+ remoteDriverUnlock(priv);
+
+ if (ret != VIR_DRV_OPEN_SUCCESS)
conn->privateData = NULL;
- remoteDriverUnlock(priv);
- VIR_FREE(priv);
- } else {
- conn->privateData = priv;
- remoteDriverUnlock(priv);
- }
+ else
+ conn->privateData = g_steal_pointer(&priv);
return ret;
}
--
2.37.1
6:58 a.m.
New subject: [PATCH v2 7/9] lxc: Remove unneeded forward declaration of 'lxcStateInitialize'
The function is used only after the definition.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/lxc/lxc_driver.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e7c6c4fbc4..f2a358805a 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -79,10 +79,6 @@ VIR_LOG_INIT("lxc.lxc_driver");
#define LXC_NB_DOMAIN_BLOCK_STAT_PARAM 4
-static int lxcStateInitialize(bool privileged,
- const char *root,
- virStateInhibitCallback callback,
- void *opaque);
static int lxcStateCleanup(void);
virLXCDriver *lxc_driver = NULL;
--
2.37.1
6:58 a.m.
New subject: [PATCH v2 8/9] virStateInitialize: Propagate whether running in monolithic daemon mode to stateful driver init
Upcoming patch which is fixing the opening of drivers in monolithic mode
needs to know whether we are inside 'libvirtd' but the code where the
decision needs to happen is not re-compiled per daemon. Thus we need to
pass this information to the stateful driver init function so that it
can be remebered.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/bhyve/bhyve_driver.c | 1 +
src/ch/ch_driver.c | 1 +
src/driver-state.h | 1 +
src/interface/interface_backend_netcf.c | 1 +
src/interface/interface_backend_udev.c | 1 +
src/libvirt.c | 5 ++++-
src/libvirt_internal.h | 1 +
src/libxl/libxl_driver.c | 1 +
src/lxc/lxc_driver.c | 1 +
src/network/bridge_driver.c | 1 +
src/node_device/node_device_udev.c | 1 +
src/nwfilter/nwfilter_driver.c | 1 +
src/qemu/qemu_driver.c | 1 +
src/remote/remote_daemon.c | 6 ++++++
src/remote/remote_driver.c | 1 +
src/secret/secret_driver.c | 1 +
src/storage/storage_driver.c | 1 +
src/vz/vz_driver.c | 1 +
18 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 09ba52483a..e0bf2a19a6 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1178,6 +1178,7 @@ bhyveStateCleanup(void)
static int
bhyveStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index c6e92efb2c..db2a66d131 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -861,6 +861,7 @@ static int chStateCleanup(void)
static int chStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/driver-state.h b/src/driver-state.h
index f3a0638e90..7f8b61fa1c 100644
--- a/src/driver-state.h
+++ b/src/driver-state.h
@@ -33,6 +33,7 @@ typedef enum {
typedef virDrvStateInitResult
(*virDrvStateInitialize)(bool privileged,
const char *root,
+ bool monolithic,
virStateInhibitCallback callback,
void *opaque);
diff --git a/src/interface/interface_backend_netcf.c
b/src/interface/interface_backend_netcf.c
index 90514692e2..5964720e0f 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -89,6 +89,7 @@ virNetcfDriverStateDispose(void *obj)
static int
netcfStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 32318d8d50..979f187d87 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -1110,6 +1110,7 @@ udevStateCleanup(void);
static int
udevStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/libvirt.c b/src/libvirt.c
index d5ae68d16f..1b4c90e110 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -604,6 +604,7 @@ virRegisterStateDriver(virStateDriver *driver)
* @privileged: set to true if running with root privilege, false otherwise
* @mandatory: set to true if all drivers must report success, not skipped
* @root: directory to use for embedded mode
+ * @monolithic: set to true if running in monolithic mode (daemon is libvirtd)
* @callback: callback to invoke to inhibit shutdown of the daemon
* @opaque: data to pass to @callback
*
@@ -633,6 +634,7 @@ int
virStateInitialize(bool privileged,
bool mandatory,
const char *root,
+ bool monolithic,
virStateInhibitCallback callback,
void *opaque)
{
@@ -650,6 +652,7 @@ virStateInitialize(bool privileged,
virStateDriverTab[i]->initialized = true;
ret = virStateDriverTab[i]->stateInitialize(privileged,
root,
+ monolithic,
callback,
opaque);
VIR_DEBUG("State init result %d (mandatory=%d)", ret, mandatory);
@@ -1016,7 +1019,7 @@ virConnectOpenInternal(const char *name,
virAccessManagerSetDefault(acl);
}
- if (virStateInitialize(geteuid() == 0, true, root, NULL, NULL) < 0)
+ if (virStateInitialize(geteuid() == 0, true, root, false, NULL, NULL) <
0)
return NULL;
embed = true;
diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
index f4e592922d..1ae3e2b2e0 100644
--- a/src/libvirt_internal.h
+++ b/src/libvirt_internal.h
@@ -32,6 +32,7 @@ typedef void (*virStateInhibitCallback)(bool inhibit,
int virStateInitialize(bool privileged,
bool mandatory,
const char *root,
+ bool monolithic,
virStateInhibitCallback inhibit,
void *opaque);
int virStateShutdownPrepare(void);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 79af2f4441..1b8b40e9e0 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -648,6 +648,7 @@ libxlAddDom0(libxlDriverPrivate *driver)
static int
libxlStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback,
void *opaque)
{
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index f2a358805a..d66c26221c 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1430,6 +1430,7 @@ lxcSecurityInit(virLXCDriverConfig *cfg)
static int lxcStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 7c7812e276..7c6430b4e3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -575,6 +575,7 @@ firewalld_dbus_signal_callback(GDBusConnection *connection
G_GNUC_UNUSED,
static int
networkStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index a06eaade5d..07c10f0d88 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -2226,6 +2226,7 @@ mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
static int
nodeStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index d1bdd806fc..9cb306909c 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -211,6 +211,7 @@ nwfilterStateCleanup(void)
static int
nwfilterStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6931f0670d..94b70872d4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -549,6 +549,7 @@ qemuDomainFindMaxID(virDomainObj *vm,
static int
qemuStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback,
void *opaque)
{
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 36d95de83d..f369d09d35 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -591,6 +591,11 @@ static void daemonRunStateInit(void *opaque)
#else /* ! MODULE_NAME */
bool mandatory = false;
#endif /* ! MODULE_NAME */
+#ifdef LIBVIRTD
+ bool monolithic = true;
+#else /* ! LIBVIRTD */
+ bool monolithic = false;
+#endif /* ! LIBVIRTD */
virIdentitySetCurrent(sysident);
@@ -605,6 +610,7 @@ static void daemonRunStateInit(void *opaque)
if (virStateInitialize(virNetDaemonIsPrivileged(dmn),
mandatory,
NULL,
+ monolithic,
daemonInhibitCallback,
dmn) < 0) {
VIR_ERROR(_("Driver state initialization failed"));
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 25c80a09c7..33cc6b1fce 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -168,6 +168,7 @@ static void
make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapsho
static int
remoteStateInitialize(bool privileged G_GNUC_UNUSED,
const char *root G_GNUC_UNUSED,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 080ed962a9..6328589fa4 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -462,6 +462,7 @@ secretStateCleanup(void)
static int
secretStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 8dc2cfa7c9..fccf0fcf52 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -242,6 +242,7 @@ storageDriverAutostart(void)
static int
storageStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 571d895167..4f5e340d53 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -4077,6 +4077,7 @@ vzStateCleanup(void)
static int
vzStateInitialize(bool privileged,
const char *root,
+ bool monolithic G_GNUC_UNUSED,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
--
2.37.1
6:58 a.m.
New subject: [PATCH v2 9/9] remote: Don't attempt remote connection from libvirtd
When a hypervisor driver is not compiled in and a user enables the
monolithic libvirtd, they get the following misleading error:
$ virsh -c qemu:///system
error: failed to connect to the hypervisor
error: Failed to connect socket to '/var/run/libvirt/virtqemud-sock': No such
file or directory
The issue is that the daemon side of the remote driver can't find the
appropriate driver, but the remote driver always accepts everything and
thus attempts to delegate further, which in case of libvirtd makes no
sense.
Refuse opening a connection for local URIS even when the requested
driver is not registered in case when we are inside 'libvirtd' as
libvirtd doesn't have anything to delegate to.
$ virsh -c qemu:///system
error: failed to connect to the hypervisor
error: no connection driver available for qemu:///system
Discovered when investigating https://gitlab.com/libvirt/libvirt/-/issues/370
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/remote/remote_driver.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 33cc6b1fce..a4efe101a3 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -73,6 +73,7 @@ VIR_LOG_INIT("remote.remote_driver");
#endif
static bool inside_daemon;
+static bool monolithic_daemon;
struct private_data {
virMutex lock;
@@ -168,7 +169,7 @@ static void
make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapsho
static int
remoteStateInitialize(bool privileged G_GNUC_UNUSED,
const char *root G_GNUC_UNUSED,
- bool monolithic G_GNUC_UNUSED,
+ bool monolithic,
virStateInhibitCallback callback G_GNUC_UNUSED,
void *opaque G_GNUC_UNUSED)
{
@@ -176,6 +177,7 @@ remoteStateInitialize(bool privileged G_GNUC_UNUSED,
* re-entering ourselves
*/
inside_daemon = true;
+ monolithic_daemon = monolithic;
return VIR_DRV_STATE_INIT_COMPLETE;
}
@@ -1244,16 +1246,22 @@ remoteConnectOpen(virConnectPtr conn,
if (!conn->uri)
return VIR_DRV_OPEN_DECLINED;
- /* If there's a driver registered we must defer to that.
- * If there isn't a driver, we must connect in "direct"
- * mode - see doRemoteOpen.
- * One exception is if we are trying to connect to an
- * unknown socket path as that might be proxied to remote
- * host */
- if (!conn->uri->server &&
- virHasDriverForURIScheme(driver) &&
- !virURICheckUnixSocket(conn->uri))
- return VIR_DRV_OPEN_DECLINED;
+ /* Handle deferring to local drivers if we are dealing with a default
+ * local URI. (Unknown local socket paths may be proxied to a remote
+ * host so they are treated as remote too).
+ *
+ * Deferring to a local driver is needed if:
+ * - the driver is registered in the current daemon
+ * - if we are running monolithic libvirtd, in which case we consider
+ * even un-registered drivers as local
+ */
+ if (!conn->uri->server && !virURICheckUnixSocket(conn->uri)) {
+ if (virHasDriverForURIScheme(driver))
+ return VIR_DRV_OPEN_DECLINED;
+
+ if (monolithic_daemon)
+ return VIR_DRV_OPEN_DECLINED;
+ }
}
if (!(priv = remoteAllocPrivateData()))
--
2.37.1
11:33 a.m.
On a Friday in 2022, Peter Krempa wrote:
When libvirtd is built without a driver we report a terrible error
message which doesn't really point to what's happening.
v2:
- propagate that we want to avoid the remote driver internally rather
than via a public API flag
Peter Krempa (9):
virConnectOpenInternal: Avoid double free() when alias is an invalid
URI
virConnectOpenInternal: Switch to automatic memory cleanup
virConnectOpenInternal: Remove 'failed' label
remote: remoteOpenConn: Use virConnectOpenAuth instead of
virConnectOpen(ReadOnly)
remoteConnectOpen: Refactor cleanup
remote: doRemoteOpen: Automatically clean up 'priv'
lxc: Remove unneeded forward declaration of 'lxcStateInitialize'
virStateInitialize: Propagate whether running in monolithic daemon
mode to stateful driver init
remote: Don't attempt remote connection from libvirtd
src/bhyve/bhyve_driver.c | 1 +
src/ch/ch_driver.c | 1 +
src/driver-state.h | 1 +
src/interface/interface_backend_netcf.c | 1 +
src/interface/interface_backend_udev.c | 1 +
src/libvirt.c | 67 +++++++++++--------------
src/libvirt_internal.h | 1 +
src/libxl/libxl_driver.c | 1 +
src/lxc/lxc_driver.c | 5 +-
src/network/bridge_driver.c | 1 +
src/node_device/node_device_udev.c | 1 +
src/nwfilter/nwfilter_driver.c | 1 +
src/qemu/qemu_driver.c | 1 +
src/remote/remote_daemon.c | 6 +++
src/remote/remote_daemon_dispatch.c | 7 ++-
src/remote/remote_driver.c | 59 +++++++++++-----------
src/secret/secret_driver.c | 1 +
src/storage/storage_driver.c | 1 +
src/vz/vz_driver.c | 1 +
19 files changed, 83 insertions(+), 75 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
837
days inactive
837
days old
11 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa