3:55 a.m.
I've noticed this problem when reviewing a patch on the list [1].
Long story short, dom->conn is not guaranteed to have all driver
pointers set (consider split daemons). I haven't identified any
other violation than what I'm fixing here. But something might have
slipped through my git grep.
1: https://www.redhat.com/archives/libvir-list/2019-December/msg00120.html
Michal Prívozník (9):
qemu_driver: Push qemuDomainInterfaceAddresses() a few lines down
qemu: Don't use dom->conn to lookup virNetwork
qemuGetDHCPInterfaces: Move some variables inside the loop
qemuGetDHCPInterfaces: Switch to GLib
libxl: Don't use dom->conn to lookup virNetwork
libxlGetDHCPInterfaces: Move some variables inside the loop
libxlGetDHCPInterfaces: Switch to GLib
lxc: Cleanup virConnectPtr usage
get_nonnull_domain: Drop useless comment
src/libxl/libxl_driver.c | 71 ++++------
src/lxc/lxc_driver.c | 12 +-
src/lxc/lxc_process.c | 40 +++---
src/lxc/lxc_process.h | 2 +-
src/qemu/qemu_driver.c | 193 ++++++++++++----------------
src/remote/remote_daemon_dispatch.c | 3 -
6 files changed, 139 insertions(+), 182 deletions(-)
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 1/9] qemu_driver: Push qemuDomainInterfaceAddresses() a few lines down
If we place qemuDomainInterfaceAddresses() a few lines below the
two functions its using then we can drop forward declarations of
those functions.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 125 ++++++++++++++++++++---------------------
1 file changed, 60 insertions(+), 65 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1911073f3e..1b9d6d16a1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -151,13 +151,6 @@ static int qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
const char *path, int oflags,
bool *needUnlink);
-static int qemuGetDHCPInterfaces(virDomainPtr dom,
- virDomainObjPtr vm,
- virDomainInterfacePtr **ifaces);
-
-static int qemuARPGetInterfaces(virDomainObjPtr vm,
- virDomainInterfacePtr **ifaces);
-
static virQEMUDriverPtr qemu_driver;
/* Looks up the domain object from snapshot and unlocks the
@@ -21554,64 +21547,6 @@ qemuDomainGetFSInfo(virDomainPtr dom,
return ret;
}
-static int
-qemuDomainInterfaceAddresses(virDomainPtr dom,
- virDomainInterfacePtr **ifaces,
- unsigned int source,
- unsigned int flags)
-{
- virQEMUDriverPtr driver = dom->conn->privateData;
- virDomainObjPtr vm = NULL;
- qemuAgentPtr agent;
- int ret = -1;
-
- virCheckFlags(0, -1);
-
- if (!(vm = qemuDomainObjFromDomain(dom)))
- goto cleanup;
-
- if (virDomainInterfaceAddressesEnsureACL(dom->conn, vm->def) < 0)
- goto cleanup;
-
- if (virDomainObjCheckActive(vm) < 0)
- goto cleanup;
-
- switch (source) {
- case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE:
- ret = qemuGetDHCPInterfaces(dom, vm, ifaces);
- break;
-
- case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT:
- if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
- goto cleanup;
-
- if (!qemuDomainAgentAvailable(vm, true))
- goto endjob;
-
- agent = qemuDomainObjEnterAgent(vm);
- ret = qemuAgentGetInterfaces(agent, ifaces);
- qemuDomainObjExitAgent(vm, agent);
-
- endjob:
- qemuDomainObjEndAgentJob(vm);
-
- break;
-
- case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP:
- ret = qemuARPGetInterfaces(vm, ifaces);
- break;
-
- default:
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
- _("Unknown IP address data source %d"),
- source);
- break;
- }
-
- cleanup:
- virDomainObjEndAPI(&vm);
- return ret;
-}
static int
qemuGetDHCPInterfaces(virDomainPtr dom,
@@ -21764,6 +21699,66 @@ qemuARPGetInterfaces(virDomainObjPtr vm,
}
+static int
+qemuDomainInterfaceAddresses(virDomainPtr dom,
+ virDomainInterfacePtr **ifaces,
+ unsigned int source,
+ unsigned int flags)
+{
+ virQEMUDriverPtr driver = dom->conn->privateData;
+ virDomainObjPtr vm = NULL;
+ qemuAgentPtr agent;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = qemuDomainObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainInterfaceAddressesEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ switch (source) {
+ case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE:
+ ret = qemuGetDHCPInterfaces(dom, vm, ifaces);
+ break;
+
+ case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT:
+ if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
+ goto cleanup;
+
+ if (!qemuDomainAgentAvailable(vm, true))
+ goto endjob;
+
+ agent = qemuDomainObjEnterAgent(vm);
+ ret = qemuAgentGetInterfaces(agent, ifaces);
+ qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+ qemuDomainObjEndAgentJob(vm);
+
+ break;
+
+ case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP:
+ ret = qemuARPGetInterfaces(vm, ifaces);
+ break;
+
+ default:
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("Unknown IP address data source %d"),
+ source);
+ break;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
+
static int
qemuDomainSetUserPassword(virDomainPtr dom,
const char *user,
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 2/9] qemu: Don't use dom->conn to lookup virNetwork
When using the monolithic daemon, then dom->conn has all driver
tables filled in properly and thus it's safe to call an API other
than virDomain*(). However, when using split daemons then
dom->conn has only hypervisor driver table set
(dom->conn->driver) and the rest is NULL. Therefore, if we want
to call a non-domain API (virNetworkLookupByName() in this case),
we have obtain the cached connection object accessible via
virGetConnectNetwork().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b9d6d16a1..e5be74c461 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21549,10 +21549,10 @@ qemuDomainGetFSInfo(virDomainPtr dom,
static int
-qemuGetDHCPInterfaces(virDomainPtr dom,
- virDomainObjPtr vm,
+qemuGetDHCPInterfaces(virDomainObjPtr vm,
virDomainInterfacePtr **ifaces)
{
+ g_autoptr(virConnect) conn = NULL;
int rv = -1;
int n_leases = 0;
size_t i, j;
@@ -21563,21 +21563,20 @@ qemuGetDHCPInterfaces(virDomainPtr dom,
virNetworkDHCPLeasePtr *leases = NULL;
virDomainInterfacePtr *ifaces_ret = NULL;
- if (!dom->conn->networkDriver ||
- !dom->conn->networkDriver->networkGetDHCPLeases) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Network driver does not support DHCP lease query"));
+ if (!(conn = virGetConnectNetwork()))
return -1;
- }
for (i = 0; i < vm->def->nnets; i++) {
if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
continue;
virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
+
virObjectUnref(network);
- network = virNetworkLookupByName(dom->conn,
+ network = virNetworkLookupByName(conn,
vm->def->nets[i]->data.network.name);
+ if (!network)
+ goto error;
if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
&leases, 0)) < 0)
@@ -21723,7 +21722,7 @@ qemuDomainInterfaceAddresses(virDomainPtr dom,
switch (source) {
case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE:
- ret = qemuGetDHCPInterfaces(dom, vm, ifaces);
+ ret = qemuGetDHCPInterfaces(vm, ifaces);
break;
case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT:
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 3/9] qemuGetDHCPInterfaces: Move some variables inside the loop
Some variables are not used outside of the for() loop. Move their
declaration to clean up the code a bit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e5be74c461..2bc2081d0b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21553,26 +21553,27 @@ qemuGetDHCPInterfaces(virDomainObjPtr vm,
virDomainInterfacePtr **ifaces)
{
g_autoptr(virConnect) conn = NULL;
+ virDomainInterfacePtr *ifaces_ret = NULL;
+ size_t ifaces_count = 0;
int rv = -1;
int n_leases = 0;
- size_t i, j;
- size_t ifaces_count = 0;
- g_autoptr(virNetwork) network = NULL;
- char macaddr[VIR_MAC_STRING_BUFLEN];
- virDomainInterfacePtr iface = NULL;
virNetworkDHCPLeasePtr *leases = NULL;
- virDomainInterfacePtr *ifaces_ret = NULL;
+ size_t i;
if (!(conn = virGetConnectNetwork()))
return -1;
for (i = 0; i < vm->def->nnets; i++) {
+ g_autoptr(virNetwork) network = NULL;
+ char macaddr[VIR_MAC_STRING_BUFLEN];
+ virDomainInterfacePtr iface = NULL;
+ size_t j;
+
if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
continue;
virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
- virObjectUnref(network);
network = virNetworkLookupByName(conn,
vm->def->nets[i]->data.network.name);
if (!network)
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 4/9] qemuGetDHCPInterfaces: Switch to GLib
If we use glib alloc functions, we can drop the 'cleanup' label
and @rv variable and also simplify the code a bit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 40 +++++++++++-----------------------------
1 file changed, 11 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2bc2081d0b..2d2a83cd8b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21555,9 +21555,6 @@ qemuGetDHCPInterfaces(virDomainObjPtr vm,
g_autoptr(virConnect) conn = NULL;
virDomainInterfacePtr *ifaces_ret = NULL;
size_t ifaces_count = 0;
- int rv = -1;
- int n_leases = 0;
- virNetworkDHCPLeasePtr *leases = NULL;
size_t i;
if (!(conn = virGetConnectNetwork()))
@@ -21566,6 +21563,8 @@ qemuGetDHCPInterfaces(virDomainObjPtr vm,
for (i = 0; i < vm->def->nnets; i++) {
g_autoptr(virNetwork) network = NULL;
char macaddr[VIR_MAC_STRING_BUFLEN];
+ virNetworkDHCPLeasePtr *leases = NULL;
+ int n_leases = 0;
virDomainInterfacePtr iface = NULL;
size_t j;
@@ -21584,21 +21583,15 @@ qemuGetDHCPInterfaces(virDomainObjPtr vm,
goto error;
if (n_leases) {
- if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
- goto error;
-
- if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
- goto error;
+ ifaces_ret = g_renew(typeof(*ifaces_ret), ifaces_ret, ifaces_count + 1);
+ ifaces_ret[ifaces_count] = g_new0(typeof(**ifaces_ret), 1);
+ iface = ifaces_ret[ifaces_count];
+ ifaces_count++;
- iface = ifaces_ret[ifaces_count - 1];
/* Assuming each lease corresponds to a separate IP */
iface->naddrs = n_leases;
-
- if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0)
- goto error;
-
+ iface->addrs = g_new0(typeof(*iface->addrs), iface->naddrs);
iface->name = g_strdup(vm->def->nets[i]->ifname);
-
iface->hwaddr = g_strdup(macaddr);
}
@@ -21607,28 +21600,17 @@ qemuGetDHCPInterfaces(virDomainObjPtr vm,
virDomainIPAddressPtr ip_addr = &iface->addrs[j];
ip_addr->addr = g_strdup(lease->ipaddr);
-
ip_addr->type = lease->type;
ip_addr->prefix = lease->prefix;
- }
- for (j = 0; j < n_leases; j++)
- virNetworkDHCPLeaseFree(leases[j]);
+ virNetworkDHCPLeaseFree(lease);
+ }
VIR_FREE(leases);
}
*ifaces = g_steal_pointer(&ifaces_ret);
- rv = ifaces_count;
-
- cleanup:
- if (leases) {
- for (i = 0; i < n_leases; i++)
- virNetworkDHCPLeaseFree(leases[i]);
- }
- VIR_FREE(leases);
-
- return rv;
+ return ifaces_count;
error:
if (ifaces_ret) {
@@ -21637,7 +21619,7 @@ qemuGetDHCPInterfaces(virDomainObjPtr vm,
}
VIR_FREE(ifaces_ret);
- goto cleanup;
+ return -1;
}
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 5/9] libxl: Don't use dom->conn to lookup virNetwork
When using the monolithic daemon, then dom->conn has all driver
tables filled in properly and thus it's safe to call an API other
than virDomain*(). However, when using split daemons then
dom->conn has only hypervisor driver table set
(dom->conn->driver) and the rest is NULL. Therefore, if we want
to call a non-domain API (virNetworkLookupByName() in this case),
we have obtain the cached connection object accessible via
virGetConnectNetwork().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libxl/libxl_driver.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 44a74e8779..ccef4f3955 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6235,10 +6235,10 @@ static int libxlNodeGetSecurityModel(virConnectPtr conn,
}
static int
-libxlGetDHCPInterfaces(virDomainPtr dom,
- virDomainObjPtr vm,
+libxlGetDHCPInterfaces(virDomainObjPtr vm,
virDomainInterfacePtr **ifaces)
{
+ g_autoptr(virConnect) conn = NULL;
int rv = -1;
int n_leases = 0;
size_t i, j;
@@ -6249,12 +6249,8 @@ libxlGetDHCPInterfaces(virDomainPtr dom,
virNetworkDHCPLeasePtr *leases = NULL;
virDomainInterfacePtr *ifaces_ret = NULL;
- if (!dom->conn->networkDriver ||
- !dom->conn->networkDriver->networkGetDHCPLeases) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Network driver does not support DHCP lease query"));
+ if (!(conn = virGetConnectNetwork()))
return -1;
- }
for (i = 0; i < vm->def->nnets; i++) {
if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
@@ -6262,8 +6258,10 @@ libxlGetDHCPInterfaces(virDomainPtr dom,
virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
virObjectUnref(network);
- network = virNetworkLookupByName(dom->conn,
+ network = virNetworkLookupByName(conn,
vm->def->nets[i]->data.network.name);
+ if (!network)
+ goto error;
if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
&leases, 0)) < 0)
@@ -6351,7 +6349,7 @@ libxlDomainInterfaceAddresses(virDomainPtr dom,
switch (source) {
case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE:
- ret = libxlGetDHCPInterfaces(dom, vm, ifaces);
+ ret = libxlGetDHCPInterfaces(vm, ifaces);
break;
default:
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 6/9] libxlGetDHCPInterfaces: Move some variables inside the loop
Some variables are not used outside of the for() loop. Move their
declaration to clean up the code a bit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libxl/libxl_driver.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index ccef4f3955..6064436681 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6239,25 +6239,27 @@ libxlGetDHCPInterfaces(virDomainObjPtr vm,
virDomainInterfacePtr **ifaces)
{
g_autoptr(virConnect) conn = NULL;
+ virDomainInterfacePtr *ifaces_ret = NULL;
+ size_t ifaces_count = 0;
int rv = -1;
int n_leases = 0;
- size_t i, j;
- size_t ifaces_count = 0;
- virNetworkPtr network = NULL;
- char macaddr[VIR_MAC_STRING_BUFLEN];
- virDomainInterfacePtr iface = NULL;
virNetworkDHCPLeasePtr *leases = NULL;
- virDomainInterfacePtr *ifaces_ret = NULL;
+ size_t i;
if (!(conn = virGetConnectNetwork()))
return -1;
for (i = 0; i < vm->def->nnets; i++) {
+ g_autoptr(virNetwork) network = NULL;
+ char macaddr[VIR_MAC_STRING_BUFLEN];
+ virDomainInterfacePtr iface = NULL;
+ size_t j;
+
if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
continue;
virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
- virObjectUnref(network);
+
network = virNetworkLookupByName(conn,
vm->def->nets[i]->data.network.name);
if (!network)
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 7/9] libxlGetDHCPInterfaces: Switch to GLib
If we use glib alloc functions, we can drop the 'cleanup' label
and @rv variable and also simplify the code a bit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libxl/libxl_driver.c | 41 +++++++++++-----------------------------
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6064436681..7467111618 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6241,9 +6241,6 @@ libxlGetDHCPInterfaces(virDomainObjPtr vm,
g_autoptr(virConnect) conn = NULL;
virDomainInterfacePtr *ifaces_ret = NULL;
size_t ifaces_count = 0;
- int rv = -1;
- int n_leases = 0;
- virNetworkDHCPLeasePtr *leases = NULL;
size_t i;
if (!(conn = virGetConnectNetwork()))
@@ -6252,6 +6249,8 @@ libxlGetDHCPInterfaces(virDomainObjPtr vm,
for (i = 0; i < vm->def->nnets; i++) {
g_autoptr(virNetwork) network = NULL;
char macaddr[VIR_MAC_STRING_BUFLEN];
+ virNetworkDHCPLeasePtr *leases = NULL;
+ int n_leases = 0;
virDomainInterfacePtr iface = NULL;
size_t j;
@@ -6270,21 +6269,16 @@ libxlGetDHCPInterfaces(virDomainObjPtr vm,
goto error;
if (n_leases) {
- if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
- goto error;
+ ifaces_ret = g_renew(typeof(*ifaces_ret), ifaces_ret, ifaces_count + 1);
+ ifaces_ret[ifaces_count] = g_new0(typeof(**ifaces_ret), 1);
+ iface = ifaces_ret[ifaces_count];
+ ifaces_count++;
- if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
- goto error;
-
- iface = ifaces_ret[ifaces_count - 1];
/* Assuming each lease corresponds to a separate IP */
iface->naddrs = n_leases;
- if (VIR_ALLOC_N(iface->addrs, iface->naddrs) < 0)
- goto error;
-
+ iface->addrs = g_new0(typeof(*iface->addrs), iface->naddrs);
iface->name = g_strdup(vm->def->nets[i]->ifname);
-
iface->hwaddr = g_strdup(macaddr);
}
@@ -6293,30 +6287,17 @@ libxlGetDHCPInterfaces(virDomainObjPtr vm,
virDomainIPAddressPtr ip_addr = &iface->addrs[j];
ip_addr->addr = g_strdup(lease->ipaddr);
-
ip_addr->type = lease->type;
ip_addr->prefix = lease->prefix;
- }
- for (j = 0; j < n_leases; j++)
virNetworkDHCPLeaseFree(leases[j]);
+ }
VIR_FREE(leases);
}
- *ifaces = ifaces_ret;
- ifaces_ret = NULL;
- rv = ifaces_count;
-
- cleanup:
- virObjectUnref(network);
- if (leases) {
- for (i = 0; i < n_leases; i++)
- virNetworkDHCPLeaseFree(leases[i]);
- }
- VIR_FREE(leases);
-
- return rv;
+ *ifaces = g_steal_pointer(&ifaces_ret);
+ return ifaces_count;
error:
if (ifaces_ret) {
@@ -6325,7 +6306,7 @@ libxlGetDHCPInterfaces(virDomainObjPtr vm,
}
VIR_FREE(ifaces_ret);
- goto cleanup;
+ return -1;
}
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 8/9] lxc: Cleanup virConnectPtr usage
There are some functions which pass virConnectPtr around for one
reason and one reason only: to obtain virLXCDriverPtr in the end.
Might replace the argument and pass a pointer to the driver right
from the start.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/lxc/lxc_driver.c | 12 +++++-------
src/lxc/lxc_process.c | 40 ++++++++++++++++++++++------------------
src/lxc/lxc_process.h | 2 +-
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 826bf074e3..e4be45fca2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3803,9 +3803,8 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver,
}
-/* XXX conn required for network -> bridge resolution */
static int
-lxcDomainAttachDeviceNetLive(virConnectPtr conn,
+lxcDomainAttachDeviceNetLive(virLXCDriverPtr driver,
virDomainObjPtr vm,
virDomainNetDefPtr net)
{
@@ -3866,7 +3865,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
goto cleanup;
break;
case VIR_DOMAIN_NET_TYPE_DIRECT: {
- if (!(veth = virLXCProcessSetupInterfaceDirect(conn, vm->def, net)))
+ if (!(veth = virLXCProcessSetupInterfaceDirect(driver, vm->def, net)))
goto cleanup;
} break;
case VIR_DOMAIN_NET_TYPE_USER:
@@ -4233,8 +4232,7 @@ lxcDomainAttachDeviceHostdevLive(virLXCDriverPtr driver,
static int
-lxcDomainAttachDeviceLive(virConnectPtr conn,
- virLXCDriverPtr driver,
+lxcDomainAttachDeviceLive(virLXCDriverPtr driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
@@ -4248,7 +4246,7 @@ lxcDomainAttachDeviceLive(virConnectPtr conn,
break;
case VIR_DOMAIN_DEVICE_NET:
- ret = lxcDomainAttachDeviceNetLive(conn, vm,
+ ret = lxcDomainAttachDeviceNetLive(driver, vm,
dev->data.net);
if (!ret)
dev->data.net = NULL;
@@ -4736,7 +4734,7 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
true) < 0)
goto endjob;
- if ((ret = lxcDomainAttachDeviceLive(dom->conn, driver, vm, dev_copy)) <
0)
+ if ((ret = lxcDomainAttachDeviceLive(driver, vm, dev_copy)) < 0)
goto endjob;
/*
* update domain status forcibly because the domain status may be
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 9b62a93096..f5d2fb0145 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -326,13 +326,13 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm,
}
-char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
- virDomainDefPtr def,
- virDomainNetDefPtr net)
+char *
+virLXCProcessSetupInterfaceDirect(virLXCDriverPtr driver,
+ virDomainDefPtr def,
+ virDomainNetDefPtr net)
{
char *ret = NULL;
char *res_ifname = NULL;
- virLXCDriverPtr driver = conn->privateData;
const virNetDevBandwidth *bw;
const virNetDevVPortProfile *prof;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
@@ -392,9 +392,10 @@ static const char *nsInfoLocal[VIR_LXC_DOMAIN_NAMESPACE_LAST] = {
[VIR_LXC_DOMAIN_NAMESPACE_SHAREUTS] = "uts",
};
-static int virLXCProcessSetupNamespaceName(virConnectPtr conn, int ns_type, const char
*name)
+static int virLXCProcessSetupNamespaceName(virLXCDriverPtr driver,
+ int ns_type,
+ const char *name)
{
- virLXCDriverPtr driver = conn->privateData;
int fd = -1;
virDomainObjPtr vm;
virLXCDomainObjPrivatePtr priv;
@@ -474,7 +475,7 @@ static int virLXCProcessSetupNamespaceNet(int ns_type, const char
*name)
/**
* virLXCProcessSetupNamespaces:
- * @conn: pointer to connection
+ * @driver: pointer to driver structure
* @def: pointer to virtual machines namespaceData
* @nsFDs: out parameter to store the namespace FD
*
@@ -483,9 +484,10 @@ static int virLXCProcessSetupNamespaceNet(int ns_type, const char
*name)
*
* Returns 0 on success or -1 in case of error
*/
-static int virLXCProcessSetupNamespaces(virConnectPtr conn,
- lxcDomainDefPtr lxcDef,
- int *nsFDs)
+static int
+virLXCProcessSetupNamespaces(virLXCDriverPtr driver,
+ lxcDomainDefPtr lxcDef,
+ int *nsFDs)
{
size_t i;
@@ -500,7 +502,8 @@ static int virLXCProcessSetupNamespaces(virConnectPtr conn,
case VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NONE:
continue;
case VIR_LXC_DOMAIN_NAMESPACE_SOURCE_NAME:
- if ((nsFDs[i] = virLXCProcessSetupNamespaceName(conn, i,
lxcDef->ns_val[i])) < 0)
+ if ((nsFDs[i] = virLXCProcessSetupNamespaceName(driver, i,
+ lxcDef->ns_val[i])) <
0)
return -1;
break;
case VIR_LXC_DOMAIN_NAMESPACE_SOURCE_PID:
@@ -519,7 +522,7 @@ static int virLXCProcessSetupNamespaces(virConnectPtr conn,
/**
* virLXCProcessSetupInterfaces:
- * @conn: pointer to connection
+ * @driver: pointer to driver structure
* @def: pointer to virtual machine structure
* @veths: string list of interface names
*
@@ -529,9 +532,10 @@ static int virLXCProcessSetupNamespaces(virConnectPtr conn,
*
* Returns 0 on success or -1 in case of error
*/
-static int virLXCProcessSetupInterfaces(virConnectPtr conn,
- virDomainDefPtr def,
- char ***veths)
+static int
+virLXCProcessSetupInterfaces(virLXCDriverPtr driver,
+ virDomainDefPtr def,
+ char ***veths)
{
int ret = -1;
size_t i;
@@ -585,7 +589,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
goto cleanup;
break;
case VIR_DOMAIN_NET_TYPE_DIRECT:
- if (!(veth = virLXCProcessSetupInterfaceDirect(conn, def, net)))
+ if (!(veth = virLXCProcessSetupInterfaceDirect(driver, def, net)))
goto cleanup;
break;
@@ -1344,11 +1348,11 @@ int virLXCProcessStart(virConnectPtr conn,
}
VIR_DEBUG("Setting up Interfaces");
- if (virLXCProcessSetupInterfaces(conn, vm->def, &veths) < 0)
+ if (virLXCProcessSetupInterfaces(driver, vm->def, &veths) < 0)
goto cleanup;
VIR_DEBUG("Setting up namespaces if any");
- if (virLXCProcessSetupNamespaces(conn, vm->def->namespaceData, nsInheritFDs)
< 0)
+ if (virLXCProcessSetupNamespaces(driver, vm->def->namespaceData, nsInheritFDs)
< 0)
goto cleanup;
VIR_DEBUG("Preparing to launch");
diff --git a/src/lxc/lxc_process.h b/src/lxc/lxc_process.h
index 1bf359b229..383f6f714d 100644
--- a/src/lxc/lxc_process.h
+++ b/src/lxc/lxc_process.h
@@ -50,6 +50,6 @@ int virLXCProcessValidateInterface(virDomainNetDefPtr net);
char *virLXCProcessSetupInterfaceTap(virDomainDefPtr vm,
virDomainNetDefPtr net,
const char *brname);
-char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
+char *virLXCProcessSetupInterfaceDirect(virLXCDriverPtr driver,
virDomainDefPtr def,
virDomainNetDefPtr net);
--
2.23.0
3:55 a.m.
New subject: [libvirt] [PATCH 9/9] get_nonnull_domain: Drop useless comment
The intent of get_nonnull_domain() is not to validate virDomain
as sent by the client but just to construct the virDomain
structure. The validation is then done in each API when looking
up the domain in our internal hash tables.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index f369ffb02a..a96e008242 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -7240,9 +7240,6 @@ remoteDispatchNetworkPortGetParameters(virNetServerPtr server
G_GNUC_UNUSED,
static virDomainPtr
get_nonnull_domain(virConnectPtr conn, remote_nonnull_domain domain)
{
- /* Should we believe the domain.id sent by the client? Maybe
- * this should be a check rather than an assignment? XXX
- */
return virGetDomain(conn, domain.name, BAD_CAST domain.uuid, domain.id);
}
--
2.23.0
9:24 a.m.
On 12/4/19 4:55 AM, Michal Privoznik wrote:
I've noticed this problem when reviewing a patch on the list
[1].
Long story short, dom->conn is not guaranteed to have all driver
pointers set (consider split daemons). I haven't identified any
other violation than what I'm fixing here. But something might have
slipped through my git grep.
1: https://www.redhat.com/archives/libvir-list/2019-December/msg00120.html
Michal Prívozník (9):
qemu_driver: Push qemuDomainInterfaceAddresses() a few lines down
qemu: Don't use dom->conn to lookup virNetwork
qemuGetDHCPInterfaces: Move some variables inside the loop
qemuGetDHCPInterfaces: Switch to GLib
libxl: Don't use dom->conn to lookup virNetwork
libxlGetDHCPInterfaces: Move some variables inside the loop
libxlGetDHCPInterfaces: Switch to GLib
lxc: Cleanup virConnectPtr usage
get_nonnull_domain: Drop useless comment
src/libxl/libxl_driver.c | 71 ++++------
src/lxc/lxc_driver.c | 12 +-
src/lxc/lxc_process.c | 40 +++---
src/lxc/lxc_process.h | 2 +-
src/qemu/qemu_driver.c | 193 ++++++++++++----------------
src/remote/remote_daemon_dispatch.c | 3 -
6 files changed, 139 insertions(+), 182 deletions(-)
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
- Cole
1805
days inactive
1818
days old
10 comments
2 participants
participants (2)
-
Cole Robinson -
Michal Privoznik